From 966d36a36250a89ebc86b3e9c4012fbd00ad45d3 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Fri, 21 Apr 2023 14:26:56 -0700 Subject: [PATCH] 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 --- .../Common/JsonSeparatorNamingPolicy.cs | 5 ++++ .../System.Text.Json/Common/ThrowHelper.cs | 16 ++++++++++ .../System.Text.Json.SourceGeneration.targets | 1 + .../src/System.Text.Json.csproj | 1 + .../src/System/Text/Json/ThrowHelper.cs | 7 ----- .../Serialization/PropertyNameTests.cs | 30 +++++++++++++++++++ 6 files changed, 53 insertions(+), 7 deletions(-) create mode 100644 src/libraries/System.Text.Json/Common/ThrowHelper.cs diff --git a/src/libraries/System.Text.Json/Common/JsonSeparatorNamingPolicy.cs b/src/libraries/System.Text.Json/Common/JsonSeparatorNamingPolicy.cs index 9e54127c65ad9..77eb1d8af32c3 100644 --- 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 new file mode 100644 index 0000000000000..3f381ce3be681 --- /dev/null +++ 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 index 6c977db11d6bb..42094702dcc3a 100644 --- 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 @@ + diff --git a/src/libraries/System.Text.Json/src/System.Text.Json.csproj b/src/libraries/System.Text.Json/src/System.Text.Json.csproj index 261abe0130e79..682fa0aba8fc8 100644 --- 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 + 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 index f1b20d3edd79d..7537f720abf11 100644 --- 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/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PropertyNameTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PropertyNameTests.cs index 6da381d144dab..09fb2c3128757 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.Tests/Serialization/PropertyNameTests.cs +++ b/src/libraries/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(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(() => policy.ConvertName(null)); + } + + [Theory] + [MemberData(nameof(JsonSeparatorNamingPolicyInstances))] + public void InboxSeparatorNamingPolicies_EmptyInput(JsonNamingPolicy policy) + { + Assert.Equal(string.Empty, policy.ConvertName(string.Empty)); + } + + public static IEnumerable 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 }; + } } }