Skip to content

Option to disallow duplicate JSON properties #115856

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/libraries/System.Text.Json/Common/JsonHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,20 @@ public static bool TryAdd<TKey, TValue>(this Dictionary<TKey, TValue> dictionary
return false;
}

/// <summary>
/// netstandard/netfx polyfill for IDictionary.TryAdd
/// </summary>
public static bool TryAdd<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key, TValue value) where TKey : notnull
{
if (!dictionary.ContainsKey(key))
{
dictionary[key] = value;
return true;
}

return false;
}

/// <summary>
/// netstandard/netfx polyfill for Queue.TryDequeue
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,5 +170,10 @@ public JsonSourceGenerationOptionsAttribute(JsonSerializerDefaults defaults)
/// Specifies the default value of <see cref="JsonSerializerOptions.NewLine"/> when set.
/// </summary>
public string? NewLine { get; set; }

/// <summary>
/// Specifies the default value of <see cref="JsonSerializerOptions.AllowDuplicateProperties"/> when set.
/// </summary>
public bool AllowDuplicateProperties { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1183,6 +1183,9 @@ private static void GetLogicForDefaultSerializerOptionsInit(SourceGenerationOpti
writer.WriteLine('{');
writer.Indentation++;

if (optionsSpec.AllowDuplicateProperties is bool allowDuplicateProperties)
writer.WriteLine($"AllowDuplicateProperties = {FormatBoolLiteral(allowDuplicateProperties)},");

if (optionsSpec.AllowOutOfOrderMetadataProperties is bool allowOutOfOrderMetadataProperties)
writer.WriteLine($"AllowOutOfOrderMetadataProperties = {FormatBoolLiteral(allowOutOfOrderMetadataProperties)},");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
bool? writeIndented = null;
char? indentCharacter = null;
int? indentSize = null;
bool? allowDuplicateProperties = null;

if (attributeData.ConstructorArguments.Length > 0)
{
Expand Down Expand Up @@ -412,6 +413,10 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
generationMode = (JsonSourceGenerationMode)namedArg.Value.Value!;
break;

case nameof(JsonSourceGenerationOptionsAttribute.AllowDuplicateProperties):
allowDuplicateProperties = (bool)namedArg.Value.Value!;
break;

default:
throw new InvalidOperationException();
}
Expand Down Expand Up @@ -446,6 +451,7 @@ private SourceGenerationOptionsSpec ParseJsonSourceGenerationOptionsAttribute(IN
WriteIndented = writeIndented,
IndentCharacter = indentCharacter,
IndentSize = indentSize,
AllowDuplicateProperties = allowDuplicateProperties,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,8 @@ public sealed record SourceGenerationOptionsSpec

public required int? IndentSize { get; init; }

public required bool? AllowDuplicateProperties { get; init; }

public JsonKnownNamingPolicy? GetEffectivePropertyNamingPolicy()
=> PropertyNamingPolicy ?? (Defaults is JsonSerializerDefaults.Web ? JsonKnownNamingPolicy.CamelCase : null);
}
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Text.Json/ref/System.Text.Json.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ public void WriteTo(System.Text.Json.Utf8JsonWriter writer) { }
public partial struct JsonDocumentOptions
{
private int _dummyPrimitive;
public bool AllowDuplicateProperties { get { throw null; } set { } }
public bool AllowTrailingCommas { readonly get { throw null; } set { } }
public System.Text.Json.JsonCommentHandling CommentHandling { readonly get { throw null; } set { } }
public int MaxDepth { readonly get { throw null; } set { } }
Expand Down Expand Up @@ -393,6 +394,7 @@ public sealed partial class JsonSerializerOptions
public JsonSerializerOptions() { }
public JsonSerializerOptions(System.Text.Json.JsonSerializerDefaults defaults) { }
public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
public bool AllowDuplicateProperties { get { throw null; } set { } }
public bool AllowOutOfOrderMetadataProperties { get { throw null; } set { } }
public bool AllowTrailingCommas { get { throw null; } set { } }
public System.Collections.Generic.IList<System.Text.Json.Serialization.JsonConverter> Converters { get { throw null; } }
Expand Down Expand Up @@ -1139,6 +1141,7 @@ public sealed partial class JsonSourceGenerationOptionsAttribute : System.Text.J
{
public JsonSourceGenerationOptionsAttribute() { }
public JsonSourceGenerationOptionsAttribute(System.Text.Json.JsonSerializerDefaults defaults) { }
public bool AllowDuplicateProperties { get { throw null; } set { } }
public bool AllowOutOfOrderMetadataProperties { get { throw null; } set { } }
public bool AllowTrailingCommas { get { throw null; } set { } }
public System.Type[]? Converters { get { throw null; } set { } }
Expand Down
9 changes: 9 additions & 0 deletions src/libraries/System.Text.Json/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -827,4 +827,13 @@
<data name="CannotMixEncodings" xml:space="preserve">
<value>Mixing UTF encodings in a single multi-segment JSON string is not supported. The previous segment's encoding was '{0}' and the current segment's encoding is '{1}'.</value>
</data>
<data name="DuplicatePropertiesNotAllowed_JsonPropertyInfo" xml:space="preserve">
<value>Duplicate property '{0}' encountered during deserialization of type '{1}'.</value>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we've settled on a design where we don't just throw on duplicate JSON properties in the strictly syntactic sense (case insensitivity, dictionary key conflicts) I'm starting to wonder if the term "duplicate" is fit for purpose. Is the term "conflicting property" more appropriate perhaps?

</data>
<data name="DuplicatePropertiesNotAllowed_NameSpan" xml:space="preserve">
<value>Duplicate property '{0}' encountered during deserialization.</value>
</data>
<data name="DuplicatePropertiesNotAllowed" xml:space="preserve">
<value>Duplicate properties not allowed during deserialization.</value>
</data>
</root>
1 change: 1 addition & 0 deletions src/libraries/System.Text.Json/src/System.Text.Json.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ The System.Text.Json library is built-in as part of the shared framework in .NET
<Compile Include="System\Runtime\InteropServices\JsonMarshal.cs" />
<Compile Include="System\Text\Json\AppContextSwitchHelper.cs" />
<Compile Include="System\Text\Json\BitStack.cs" />
<Compile Include="System\Text\Json\Document\JsonDocument.PropertyNameSet.cs" />
<Compile Include="System\Text\Json\Document\JsonDocument.cs" />
<Compile Include="System\Text\Json\Document\JsonDocument.DbRow.cs" />
<Compile Include="System\Text\Json\Document\JsonDocument.MetadataDb.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public sealed partial class JsonDocument
/// </exception>
public static JsonDocument Parse(ReadOnlyMemory<byte> utf8Json, JsonDocumentOptions options = default)
{
return Parse(utf8Json, options.GetReaderOptions());
return Parse(utf8Json, options.GetReaderOptions(), allowDuplicateProperties: options.AllowDuplicateProperties);
}

/// <summary>
Expand Down Expand Up @@ -80,7 +80,7 @@ public static JsonDocument Parse(ReadOnlySequence<byte> utf8Json, JsonDocumentOp

if (utf8Json.IsSingleSegment)
{
return Parse(utf8Json.First, readerOptions);
return Parse(utf8Json.First, readerOptions, allowDuplicateProperties: options.AllowDuplicateProperties);
}

int length = checked((int)utf8Json.Length);
Expand All @@ -89,7 +89,11 @@ public static JsonDocument Parse(ReadOnlySequence<byte> utf8Json, JsonDocumentOp
try
{
utf8Json.CopyTo(utf8Bytes.AsSpan());
return Parse(utf8Bytes.AsMemory(0, length), readerOptions, utf8Bytes);
return Parse(
utf8Bytes.AsMemory(0, length),
readerOptions,
utf8Bytes,
allowDuplicateProperties: options.AllowDuplicateProperties);
}
catch
{
Expand Down Expand Up @@ -123,7 +127,7 @@ public static JsonDocument Parse(Stream utf8Json, JsonDocumentOptions options =
Debug.Assert(drained.Array != null);
try
{
return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array);
return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array, allowDuplicateProperties: options.AllowDuplicateProperties);
}
catch
{
Expand All @@ -140,7 +144,8 @@ internal static JsonDocument ParseRented(PooledByteBufferWriter utf8Json, JsonDo
utf8Json.WrittenMemory,
options.GetReaderOptions(),
extraRentedArrayPoolBytes: null,
extraPooledByteBufferWriter: utf8Json);
extraPooledByteBufferWriter: utf8Json,
allowDuplicateProperties: options.AllowDuplicateProperties);
}

internal static JsonDocument ParseValue(Stream utf8Json, JsonDocumentOptions options)
Expand All @@ -157,15 +162,21 @@ internal static JsonDocument ParseValue(Stream utf8Json, JsonDocumentOptions opt
drained.AsSpan().Clear();
ArrayPool<byte>.Shared.Return(drained.Array);

return ParseUnrented(owned.AsMemory(), options.GetReaderOptions());
return ParseUnrented(
owned.AsMemory(),
options.GetReaderOptions(),
allowDuplicateProperties: options.AllowDuplicateProperties);
}

internal static JsonDocument ParseValue(ReadOnlySpan<byte> utf8Json, JsonDocumentOptions options)
{
byte[] owned = new byte[utf8Json.Length];
utf8Json.CopyTo(owned);

return ParseUnrented(owned.AsMemory(), options.GetReaderOptions());
return ParseUnrented(
owned.AsMemory(),
options.GetReaderOptions(),
allowDuplicateProperties: options.AllowDuplicateProperties);
}

internal static JsonDocument ParseValue(string json, JsonDocumentOptions options)
Expand Down Expand Up @@ -209,7 +220,11 @@ private static async Task<JsonDocument> ParseAsyncCore(
Debug.Assert(drained.Array != null);
try
{
return Parse(drained.AsMemory(), options.GetReaderOptions(), drained.Array);
return Parse(
drained.AsMemory(),
options.GetReaderOptions(),
drained.Array,
allowDuplicateProperties: options.AllowDuplicateProperties);
}
catch
{
Expand All @@ -235,7 +250,10 @@ internal static async Task<JsonDocument> ParseAsyncCoreUnrented(
drained.AsSpan().Clear();
ArrayPool<byte>.Shared.Return(drained.Array);

return ParseUnrented(owned.AsMemory(), options.GetReaderOptions());
return ParseUnrented(
owned.AsMemory(),
options.GetReaderOptions(),
allowDuplicateProperties: options.AllowDuplicateProperties);
}

/// <summary>
Expand Down Expand Up @@ -271,7 +289,8 @@ public static JsonDocument Parse([StringSyntax(StringSyntaxAttribute.Json)] Read
return Parse(
utf8Bytes.AsMemory(0, actualByteCount),
options.GetReaderOptions(),
utf8Bytes);
utf8Bytes,
allowDuplicateProperties: options.AllowDuplicateProperties);
}
catch
{
Expand Down Expand Up @@ -304,7 +323,10 @@ internal static JsonDocument ParseValue(ReadOnlyMemory<char> json, JsonDocumentO
ArrayPool<byte>.Shared.Return(utf8Bytes);
}

return ParseUnrented(owned.AsMemory(), options.GetReaderOptions());
return ParseUnrented(
owned.AsMemory(),
options.GetReaderOptions(),
allowDuplicateProperties: options.AllowDuplicateProperties);
}

/// <summary>
Expand Down Expand Up @@ -406,9 +428,12 @@ public static bool TryParseValue(ref Utf8JsonReader reader, [NotNullWhen(true)]
/// <exception cref="JsonException">
/// A value could not be read from the reader.
/// </exception>
public static JsonDocument ParseValue(ref Utf8JsonReader reader)
public static JsonDocument ParseValue(ref Utf8JsonReader reader) =>
ParseValue(ref reader, allowDuplicateProperties: true);

internal static JsonDocument ParseValue(ref Utf8JsonReader reader, bool allowDuplicateProperties)
{
bool ret = TryParseValue(ref reader, out JsonDocument? document, shouldThrow: true, useArrayPools: true);
bool ret = TryParseValue(ref reader, out JsonDocument? document, shouldThrow: true, useArrayPools: true, allowDuplicateProperties);

Debug.Assert(ret, "TryParseValue returned false with shouldThrow: true.");
Debug.Assert(document != null, "null document returned with shouldThrow: true.");
Expand All @@ -419,7 +444,8 @@ internal static bool TryParseValue(
ref Utf8JsonReader reader,
[NotNullWhen(true)] out JsonDocument? document,
bool shouldThrow,
bool useArrayPools)
bool useArrayPools,
bool allowDuplicateProperties = true)
{
JsonReaderState state = reader.CurrentState;
CheckSupportedOptions(state.Options, nameof(reader));
Expand Down Expand Up @@ -629,7 +655,7 @@ internal static bool TryParseValue(
valueSpan.CopyTo(rentedSpan);
}

document = Parse(rented.AsMemory(0, length), state.Options, rented);
document = Parse(rented.AsMemory(0, length), state.Options, rented, allowDuplicateProperties: allowDuplicateProperties);
}
catch
{
Expand All @@ -654,7 +680,7 @@ internal static bool TryParseValue(
owned = valueSpan.ToArray();
}

document = ParseUnrented(owned, state.Options, reader.TokenType);
document = ParseUnrented(owned, state.Options, reader.TokenType, allowDuplicateProperties: allowDuplicateProperties);
}

return true;
Expand Down Expand Up @@ -688,7 +714,8 @@ private static JsonDocument Parse(
ReadOnlyMemory<byte> utf8Json,
JsonReaderOptions readerOptions,
byte[]? extraRentedArrayPoolBytes = null,
PooledByteBufferWriter? extraPooledByteBufferWriter = null)
PooledByteBufferWriter? extraPooledByteBufferWriter = null,
bool allowDuplicateProperties = true)
{
ReadOnlySpan<byte> utf8JsonSpan = utf8Json.Span;
var database = MetadataDb.CreateRented(utf8Json.Length, convertToAlloc: false);
Expand All @@ -708,13 +735,16 @@ private static JsonDocument Parse(
stack.Dispose();
}

return new JsonDocument(utf8Json, database, extraRentedArrayPoolBytes, extraPooledByteBufferWriter);
JsonDocument document = new JsonDocument(utf8Json, database, extraRentedArrayPoolBytes, extraPooledByteBufferWriter);
ValidateDocument(document, allowDuplicateProperties);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if ValidateDocument throws? Shouldn't we be disposing the database or the JsonDocument?

return document;
}

private static JsonDocument ParseUnrented(
ReadOnlyMemory<byte> utf8Json,
JsonReaderOptions readerOptions,
JsonTokenType tokenType = JsonTokenType.None)
JsonTokenType tokenType = JsonTokenType.None,
bool allowDuplicateProperties = true)
{
// These tokens should already have been processed.
Debug.Assert(
Expand Down Expand Up @@ -746,7 +776,9 @@ private static JsonDocument ParseUnrented(
}
}

return new JsonDocument(utf8Json, database, isDisposable: false);
JsonDocument document = new JsonDocument(utf8Json, database, isDisposable: false);
ValidateDocument(document, allowDuplicateProperties);
return document;
}

private static ArraySegment<byte> ReadToEnd(Stream stream)
Expand Down
Loading