Avoid null ref in JsonSeparatorNamingPolicy (#85002)

* Avoid null ref in JsonSeparatorNamingPolicy * Implement desired null-handling behavior * Use throw helper for exception * Create shared throw helper and fix CI issues
dotnet · Apr 21, 2023 · 966d36a · 966d36a
1 parent 8744ae6
commit 966d36a
Show file tree

Hide file tree

Showing 6 changed files with 53 additions and 7 deletions.
diff --git a/src/libraries/System.Text.Json/Common/JsonSeparatorNamingPolicy.cs b/src/libraries/System.Text.Json/Common/JsonSeparatorNamingPolicy.cs
@@ -16,6 +16,11 @@ internal JsonSeparatorNamingPolicy(bool lowercase, char separator) =>
 
         public sealed override string ConvertName(string name)
         {
+            if (name is null)
+            {
+                ThrowHelper.ThrowArgumentNullException(nameof(name));
+            }
+
             // Rented buffer 20% longer that the input.
             int rentedBufferLength = (12 * name.Length) / 10;
             char[]? rentedBuffer = rentedBufferLength > JsonConstants.StackallocCharThreshold

diff --git a/src/libraries/System.Text.Json/Common/ThrowHelper.cs b/src/libraries/System.Text.Json/Common/ThrowHelper.cs
@@ -0,0 +1,16 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics.CodeAnalysis;
+
+namespace System.Text.Json
+{
+    internal static partial class ThrowHelper
+    {
+        [DoesNotReturn]
+        public static void ThrowArgumentNullException(string parameterName)
+        {
+            throw new ArgumentNullException(parameterName);
+        }
+    }
+}
diff --git a/src/libraries/System.Text.Json/gen/System.Text.Json.SourceGeneration.targets b/src/libraries/System.Text.Json/gen/System.Text.Json.SourceGeneration.targets
@@ -43,6 +43,7 @@
     <Compile Include="..\Common\JsonSourceGenerationOptionsAttribute.cs" Link="Common\System\Text\Json\Serialization\JsonSourceGenerationOptionsAttribute.cs" />
     <Compile Include="..\Common\ReflectionExtensions.cs" Link="Common\System\Text\Json\Serialization\ReflectionExtensions.cs" />
     <Compile Include="..\Common\JsonUnmappedMemberHandling.cs" Link="Common\System\Text\Json\Serialization\JsonUnmappedMemberHandling.cs" />
+    <Compile Include="..\Common\ThrowHelper.cs" Link="Common\System\Text\Json\ThrowHelper.cs" />
     <Compile Include="$(CommonPath)\Roslyn\GetBestTypeByMetadataName.cs" Link="Common\Roslyn\GetBestTypeByMetadataName.cs" />
     <Compile Include="ClassType.cs" />
     <Compile Include="CollectionType.cs" />

diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj
@@ -43,6 +43,7 @@ The System.Text.Json library is built-in as part of the shared framework in .NET
     <Compile Include="..\Common\JsonSourceGenerationMode.cs" Link="Common\System\Text\Json\Serialization\JsonSourceGenerationMode.cs" />
     <Compile Include="..\Common\JsonSourceGenerationOptionsAttribute.cs" Link="Common\System\Text\Json\Serialization\JsonSourceGenerationOptionsAttribute.cs" />
     <Compile Include="..\Common\ReflectionExtensions.cs" Link="Common\System\Text\Json\Serialization\ReflectionExtensions.cs" />
+    <Compile Include="..\Common\ThrowHelper.cs" Link="Common\System\Text\Json\ThrowHelper.cs" />
     <Compile Include="System\Text\Json\AppContextSwitchHelper.cs" />
     <Compile Include="System\Text\Json\BitStack.cs" />
     <Compile Include="System\Text\Json\Document\JsonDocument.cs" />

diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs b/src/libraries/System.Text.Json/src/System/Text/Json/ThrowHelper.cs
@@ -4,7 +4,6 @@
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Runtime.CompilerServices;
-using System.Text.Json.Serialization.Metadata;
 
 namespace System.Text.Json
 {
@@ -13,12 +12,6 @@ internal static partial class ThrowHelper
         // If the exception source is this value, the serializer will re-throw as JsonException.
         public const string ExceptionSourceValueToRethrowAsJsonException = "System.Text.Json.Rethrowable";
 
-        [DoesNotReturn]
-        public static void ThrowArgumentNullException(string parameterName)
-        {
-            throw new ArgumentNullException(parameterName);
-        }
-
         [DoesNotReturn]
         public static void ThrowArgumentOutOfRangeException_MaxDepthMustBePositive(string parameterName)
         {

diff --git a/...ibraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PropertyNameTests.cs b/...ibraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PropertyNameTests.cs
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using Xunit;
 
@@ -34,5 +35,34 @@ public async Task JsonNameConflictOnCaseInsensitiveFail()
                 await Assert.ThrowsAsync<InvalidOperationException>(async () => await Serializer.SerializeWrapper(new IntPropertyNamesDifferentByCaseOnly_TestClass(), options));
             }
         }
+
+        [Fact]
+        public void CamelCasePolicyToleratesNullOrEmpty()
+        {
+            Assert.Null(JsonNamingPolicy.CamelCase.ConvertName(null));
+            Assert.Equal(string.Empty, JsonNamingPolicy.CamelCase.ConvertName(string.Empty));
+        }
+
+        [Theory]
+        [MemberData(nameof(JsonSeparatorNamingPolicyInstances))]
+        public void InboxSeparatorNamingPolicies_ThrowsOnNullInput(JsonNamingPolicy policy)
+        {
+            Assert.Throws<ArgumentNullException>(() => policy.ConvertName(null));
+        }
+
+        [Theory]
+        [MemberData(nameof(JsonSeparatorNamingPolicyInstances))]
+        public void InboxSeparatorNamingPolicies_EmptyInput(JsonNamingPolicy policy)
+        {
+            Assert.Equal(string.Empty, policy.ConvertName(string.Empty));
+        }
+
+        public static IEnumerable<object[]> JsonSeparatorNamingPolicyInstances()
+        {
+            yield return new object[] { JsonNamingPolicy.SnakeCaseLower };
+            yield return new object[] { JsonNamingPolicy.SnakeCaseUpper };
+            yield return new object[] { JsonNamingPolicy.KebabCaseLower };
+            yield return new object[] { JsonNamingPolicy.KebabCaseUpper };
+        }
     }
 }