Fallback to 0.0.0 when runtime version is invalid (#5541)

Author: stevejgordon

src/Elasticsearch.Net/Connection/MetaData/MetaDataHeader.cs

@@ -8,36 +8,38 @@ namespace Elasticsearch.Net
 {
 	internal sealed class MetaDataHeader
 	{
-		private const char _separator = ',';
+		private const char Separator = ',';
 
 		private readonly string _headerValue;
 
+		private static readonly string RuntimeVersionString = new RuntimeVersionInfo().ToString();
+
 		public MetaDataHeader(VersionInfo version, string serviceIdentifier, bool isAsync)
 		{
 			ClientVersion = version.ToString();
-			RuntimeVersion = new RuntimeVersionInfo().ToString();
 			ServiceIdentifier = serviceIdentifier;
+			RuntimeVersion = RuntimeVersionString;
 
 			// This code is expected to be called infrequently so we're not concerns with over optimising this
 
 			_headerValue = new StringBuilder(64)
-				.Append(serviceIdentifier).Append("=").Append(ClientVersion).Append(_separator)
-				.Append("a=").Append(isAsync ? "1" : "0").Append(_separator)
-				.Append("net=").Append(RuntimeVersion).Append(_separator)
-				.Append(_httpClientIdentifier).Append("=").Append(RuntimeVersion)
+				.Append(serviceIdentifier).Append("=").Append(ClientVersion).Append(Separator)
+				.Append("a=").Append(isAsync ? "1" : "0").Append(Separator)
+				.Append("net=").Append(RuntimeVersion).Append(Separator)
+				.Append(HttpClientIdentifier).Append("=").Append(RuntimeVersion)
 				.ToString();
 		}
 
-		private static readonly string _httpClientIdentifier =
+		private static readonly string HttpClientIdentifier =
 #if DOTNETCORE
 			ConnectionInfo.UsingCurlHandler ? "cu" : "so";
 #else
 			"wr";
 #endif
 
-		public string ServiceIdentifier { get; private set; }
-		public string ClientVersion { get; private set; }
-		public string RuntimeVersion { get; private set; }
+		public string ServiceIdentifier { get; }
+		public string ClientVersion { get; }
+		public string RuntimeVersion { get; }
 
 		public override string ToString() => _headerValue;
 	}

src/Elasticsearch.Net/Connection/MetaData/RuntimeVersionInfo.cs

@@ -41,7 +41,7 @@ namespace Elasticsearch.Net
 	/// </summary>
 	internal sealed class RuntimeVersionInfo : VersionInfo
 	{
-		public static readonly RuntimeVersionInfo Default = new RuntimeVersionInfo { Version = new Version(0, 0, 0), IsPrerelease = false };
+		public static readonly RuntimeVersionInfo Default = new() { Version = new Version(0, 0, 0), IsPrerelease = false };
 
 		public RuntimeVersionInfo() => StoreVersion(GetRuntimeVersion());
 
@@ -79,7 +79,7 @@ private static string GetNetCoreVersion()
 				return runtimeVersion;
 			}
 
-			//At this point, we can't identify whether this is a prerelease, but a version is better than nothing!
+			//At this point, we can't identify whether this is a pre-release, but a version is better than nothing!
 
 			var frameworkName = Assembly.GetEntryAssembly()?.GetCustomAttribute<TargetFrameworkAttribute>()?.FrameworkName;
 			if (TryGetVersionFromFrameworkName(frameworkName, out runtimeVersion))
@@ -182,7 +182,7 @@ private static string GetFullFrameworkRuntime()
 				{
 					var version = CheckFor45PlusVersion((int)ndpKey.GetValue("Release"));
 
-					if (!string.IsNullOrEmpty(version) )
+					if (!string.IsNullOrEmpty(version))
 						return version;
 				}
 			}

src/Elasticsearch.Net/Connection/MetaData/VersionInfo.cs

@@ -20,9 +20,9 @@ protected void StoreVersion(string fullVersion)
 				fullVersion = EmptyVersion;
 
 			var clientVersion = GetParsableVersionPart(fullVersion);
-
+			
 			if (!Version.TryParse(clientVersion, out var parsedVersion))
-				throw new ArgumentException("Invalid version string", nameof(fullVersion));
+				parsedVersion = new Version(EmptyVersion);
 
 			var finalVersion = parsedVersion;
 
@@ -34,14 +34,16 @@ protected void StoreVersion(string fullVersion)
 						: 0);
 
 			Version = finalVersion;
-			IsPrerelease = ContainsPrerelease(fullVersion);
+			IsPrerelease = !IsEmpty(parsedVersion) && ContainsPrerelease(fullVersion);
 		}
 
+		private bool IsEmpty(Version version) => version.Major == 0 && version.Minor == 0 && version.Build == 0;
+
 		protected virtual bool ContainsPrerelease(string version) => version.Contains("-");
 
 		private static string GetParsableVersionPart(string fullVersionName) =>
-			new string(fullVersionName.TakeWhile(c => char.IsDigit(c) || c == '.').ToArray());
+			new(fullVersionName.TakeWhile(c => char.IsDigit(c) || c == '.').ToArray());
 
-		public override string ToString() => IsPrerelease ? Version.ToString() + "p" : Version.ToString();
+		public override string ToString() => IsPrerelease ? $"{Version}p" : Version.ToString();
 	}
 }

tests/Tests/Connection/MetaData/MetaHeaderProviderTests.cs

@@ -8,7 +8,7 @@
 using FluentAssertions;
 using Nest;
 
-namespace Tests.Core.Connection.MetaData
+namespace Tests.Connection.MetaData
 {
 	public class MetaHeaderProviderTests
 	{

tests/Tests/Connection/MetaData/VersionInfoTests.cs

@@ -7,22 +7,36 @@
 using Elasticsearch.Net;
 using FluentAssertions;
 
-namespace Tests.Core.Connection.MetaData
+namespace Tests.Connection.MetaData
 {
 	public class VersionInfoTests
 	{
 		[U] public void ToString_ReturnsExpectedValue_ForNonPrerelease()
 		{
-			var sut = new TestVersionInfo("1.2.3", false);
+			var sut = new TestableVersionInfo("1.2.3");
 			sut.ToString().Should().Be("1.2.3");
 		}
 
 		[U] public void ToString_ReturnsExpectedValue_ForPrerelease()
 		{
-			var sut = new TestVersionInfo("1.2.3", true);
+			var sut = new TestableVersionInfo("1.2.3-beta-1");
 			sut.ToString().Should().Be("1.2.3p");
 		}
 
+		[U]
+		public void ToString_ReturnsExpectedValue_ForNullVersion()
+		{
+			var sut = new TestableVersionInfo(null);
+			sut.ToString().Should().Be("0.0.0");
+		}
+
+		[U]
+		public void ToString_ReturnsExpectedValue_ForInvalidVersion()
+		{
+			var sut = new TestableVersionInfo("NOT-A-VERSION-NUMBER");
+			sut.ToString().Should().Be("0.0.0");
+		}
+
 		private class TestVersionInfo : VersionInfo
 		{
 			public TestVersionInfo(string version, bool isPrerelease)
@@ -31,5 +45,10 @@ public TestVersionInfo(string version, bool isPrerelease)
 				IsPrerelease = isPrerelease;
 			}
 		}
+
+		private class TestableVersionInfo : VersionInfo
+		{
+			public TestableVersionInfo(string version) => StoreVersion(version);
+		}
 	}
 }