From 8e399fb9b28e6bf90e5a3efb56089b9f96b74df3 Mon Sep 17 00:00:00 2001 From: Christophe Chevalier Date: Fri, 5 Jul 2024 11:06:06 +0200 Subject: [PATCH] json: fix JsonObject would not be properly escaped in some scenarios - WritePropertyName(...., bool knownSafe) must be explicitly declared by the caller - WriteNameEscaped(...) is used when the caller DOESN'T KNOW - WriteName(...) is used when the classer IS SURE it is safe - also fix issue when 0/false/"" could still be written even if DiscardDefault is true --- .../Serialization/JSON/CrystalJson.cs | 17 ++- .../JSON/CrystalJsonFormatter.cs | 20 ++- .../JSON/CrystalJsonStreamWriter.cs | 6 +- .../Serialization/JSON/CrystalJsonWriter.cs | 120 +++++++++++++++--- .../JSON/ObjectModel/JsonObject.cs | 8 +- 5 files changed, 144 insertions(+), 27 deletions(-) diff --git a/Doxense.Core.Tests/Serialization/JSON/CrystalJson.cs b/Doxense.Core.Tests/Serialization/JSON/CrystalJson.cs index 2a80d16aa..ba425ec45 100644 --- a/Doxense.Core.Tests/Serialization/JSON/CrystalJson.cs +++ b/Doxense.Core.Tests/Serialization/JSON/CrystalJson.cs @@ -5979,7 +5979,7 @@ public void Test_JsonObject() Assert.That(obj.ToJson(), Is.EqualTo("{ \"Hello\": \"World\", \"Foo\": 456, \"Bar\": true }")); Assert.That(SerializeToSlice(obj), Is.EqualTo(Slice.FromString("{\"Hello\":\"World\",\"Foo\":456,\"Bar\":true}"))); - // case sensitive! ('Bar' != 'BAR' + // case sensitive! ('Bar' != 'BAR') var sub = JsonObject.Create("Alpha", 111, "Omega", 999); obj.Add("BAR", sub); Assert.That(obj.Count, Is.EqualTo(4)); @@ -5989,6 +5989,21 @@ public void Test_JsonObject() Assert.That(obj.ToJson(), Is.EqualTo("{ \"Hello\": \"World\", \"Foo\": 456, \"Bar\": true, \"BAR\": { \"Alpha\": 111, \"Omega\": 999 } }")); Assert.That(SerializeToSlice(obj), Is.EqualTo(Slice.FromString("{\"Hello\":\"World\",\"Foo\":456,\"Bar\":true,\"BAR\":{\"Alpha\":111,\"Omega\":999}}"))); + // property names that require escaping ("..\.." => "..\\..", "\\?\..." => "\\\\?\\....") + obj = new JsonObject(); + obj["""Hello\World"""] = 123; + obj["""Hello"World"""] = 456; + obj["""\\?\GLOBALROOT\Device\Foo\Bar"""] = 789; + Assert.That(obj.Count, Is.EqualTo(3)); + Assert.That(obj.ContainsKey("""Hello\World"""), Is.True); + Assert.That(obj.ContainsKey("""Hello"World"""), Is.True); + Assert.That(obj.ContainsKey("""\\?\GLOBALROOT\Device\Foo\Bar"""), Is.True); + Assert.That(obj.Get("""Hello\World"""), Is.EqualTo(123)); + Assert.That(obj.Get("""Hello"World"""), Is.EqualTo(456)); + Assert.That(obj.Get("""\\?\GLOBALROOT\Device\Foo\Bar"""), Is.EqualTo(789)); + Assert.That(obj.ToJson(), Is.EqualTo("""{ "Hello\\World": 123, "Hello\"World": 456, "\\\\?\\GLOBALROOT\\Device\\Foo\\Bar": 789 }""")); + Assert.That(SerializeToSlice(obj), Is.EqualTo(Slice.FromString("""{"Hello\\World":123,"Hello\"World":456,"\\\\?\\GLOBALROOT\\Device\\Foo\\Bar":789}"""))); + //note: on ne sérialise pas les JsonNull "Missing"/"Error" par défaut! obj = JsonObject.Create("Foo", JsonNull.Null, "Bar", JsonNull.Missing, "Baz", JsonNull.Error); Assert.That(obj.ToJson(), Is.EqualTo("{ \"Foo\": null }")); diff --git a/Doxense.Core/Serialization/JSON/CrystalJsonFormatter.cs b/Doxense.Core/Serialization/JSON/CrystalJsonFormatter.cs index b8ed73a60..e951623b4 100644 --- a/Doxense.Core/Serialization/JSON/CrystalJsonFormatter.cs +++ b/Doxense.Core/Serialization/JSON/CrystalJsonFormatter.cs @@ -88,9 +88,19 @@ public static void WriteJsonString(TextWriter writer, string? text) { // null -> "null" writer.Write(JsonTokens.Null); } + else if (text.Length == 0) + { // empty string -> "" + writer.Write(JsonTokens.EmptyString); + } + else if (!JsonEncoding.NeedsEscaping(text)) + { + writer.Write('"'); + writer.Write(text); + writer.Write('"'); + } else { - WriteJsonString(writer, text.AsSpan()); + WriteJsonStringSlow(writer, text.AsSpan()); } } @@ -109,10 +119,16 @@ public static void WriteJsonString(TextWriter writer, ReadOnlySpan text) } else { // string requires encoding (slow path) - writer.Write(JsonEncoding.AppendSlow(new StringBuilder(), text, true).ToString()); + WriteJsonStringSlow(writer, text); } } + internal static void WriteJsonStringSlow(TextWriter writer, ReadOnlySpan text) + { + //TODO: PERF: OPTIMIZE: optimize this! + writer.Write(JsonEncoding.AppendSlow(new StringBuilder(), text, true).ToString()); + } + public static void WriteJavaScriptString(TextWriter writer, string? text) { if (text == null) diff --git a/Doxense.Core/Serialization/JSON/CrystalJsonStreamWriter.cs b/Doxense.Core/Serialization/JSON/CrystalJsonStreamWriter.cs index a90c818ad..3aae643e0 100644 --- a/Doxense.Core/Serialization/JSON/CrystalJsonStreamWriter.cs +++ b/Doxense.Core/Serialization/JSON/CrystalJsonStreamWriter.cs @@ -181,7 +181,7 @@ private void VisitProperty(string name, CrystalJsonTypeVisitor? visitor, Type ty { writer.Buffer.Write(' '); } - writer.WritePropertyName(name); + writer.WritePropertyName(name, knownSafe: true); if (item == null || visitor == null) { @@ -233,7 +233,7 @@ public ArrayStream BeginArrayStream(string name) { writer.Buffer.Write(' '); } - writer.WritePropertyName(name); + writer.WritePropertyName(name, knownSafe: false); return m_parent.BeginArrayFragment(m_ct); } @@ -252,7 +252,7 @@ public ObjectStream BeginObjectStream(string name) { writer.Buffer.Write(' '); } - writer.WritePropertyName(name); + writer.WritePropertyName(name, knownSafe: false); return m_parent.BeginObjectFragment(m_ct); } diff --git a/Doxense.Core/Serialization/JSON/CrystalJsonWriter.cs b/Doxense.Core/Serialization/JSON/CrystalJsonWriter.cs index 02ce6ee97..435fc109c 100644 --- a/Doxense.Core/Serialization/JSON/CrystalJsonWriter.cs +++ b/Doxense.Core/Serialization/JSON/CrystalJsonWriter.cs @@ -906,7 +906,7 @@ public void WriteRaw(string? rawJson) public void WriteName(string name) { WriteFieldSeparator(); - WritePropertyName(name); + WritePropertyName(name, knownSafe: true); } /// Write a property name that is KNOWN to not require any escaping. @@ -915,17 +915,48 @@ public void WriteName(string name) public void WriteName(ReadOnlySpan name) { WriteFieldSeparator(); - WritePropertyName(name); + WritePropertyName(name, knownSafe: true); + } + + /// Write a property name that MAY require escaping. + /// Name of the property that will be escaped if necessary + /// + /// This method should be used whenever the origin of key is not controlled, and may contains any character that would required escaping ('\', '"', ...). + /// + public void WriteNameEscaped(string name) + { + WriteFieldSeparator(); + WritePropertyName(name, knownSafe: false); + } + + /// Write a property name that MAY require escaping. + /// Name of the property that will be escaped if necessary + /// + /// This method should be used whenever the origin of key is not controlled, and may contains any character that would required escaping ('\', '"', ...). + /// + public void WriteNameEscaped(ReadOnlySpan name) + { + WriteFieldSeparator(); + WritePropertyName(name, knownSafe: false); } - internal void WritePropertyName(string name) + internal void WritePropertyName(string name, bool knownSafe) { if (!m_javascript) { var buffer = m_buffer; - buffer.Write('"'); - buffer.Write(FormatName(name)); - buffer.Write(m_formatted ? JsonTokens.QuoteColonFormatted : JsonTokens.QuoteColonCompact); + string formattedName = FormatName(name); + if (knownSafe || !JsonEncoding.NeedsEscaping(formattedName)) + { + buffer.Write('"'); + buffer.Write(formattedName); + buffer.Write(m_formatted ? JsonTokens.QuoteColonFormatted : JsonTokens.QuoteColonCompact); + } + else + { + CrystalJsonFormatter.WriteJsonStringSlow(buffer, name); + buffer.Write(m_formatted ? JsonTokens.ColonFormatted : JsonTokens.ColonCompact); + } } else { @@ -940,7 +971,7 @@ internal void WriteJavaScriptName(string name) buffer.Write(m_formatted ? JsonTokens.ColonFormatted : JsonTokens.ColonCompact); } - internal void WritePropertyName(ReadOnlySpan name) + internal void WritePropertyName(ReadOnlySpan name, bool knownSafe) { if (m_javascript) { @@ -949,18 +980,28 @@ internal void WritePropertyName(ReadOnlySpan name) } var buffer = m_buffer; - buffer.Write('"'); - if (!m_camelCase || (name[0] is '_' or (>= 'a' and <= 'z'))) + if (knownSafe || !JsonEncoding.NeedsEscaping(name)) { - buffer.Write(name); + buffer.Write('"'); + if (!m_camelCase || (name[0] is '_' or (>= 'a' and <= 'z'))) + { + buffer.Write(name); + } + else + { + buffer.Write(char.ToLowerInvariant(name[0])); + if (name.Length > 1) + { + buffer.Write(name[1..]); + } + } + buffer.Write(m_formatted ? JsonTokens.QuoteColonFormatted : JsonTokens.QuoteColonCompact); } else { - buffer.Write(char.ToLowerInvariant(name[0])); - if (name.Length > 1) buffer.Write(name[1..]); + CrystalJsonFormatter.WriteJsonStringSlow(buffer, name); + buffer.Write(m_formatted ? JsonTokens.ColonFormatted : JsonTokens.ColonCompact); } - - buffer.Write(m_formatted ? JsonTokens.QuoteColonFormatted : JsonTokens.QuoteColonCompact); } internal void WriteJavaScriptName(ReadOnlySpan name) @@ -2626,19 +2667,50 @@ public void WriteField(string name, NodaTime.DateTimeZone? value) #endregion - public void WriteField(string name, JsonValue value) + /// Tests if the specified value would have been discarded when calling + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public bool WillBeDiscarded(JsonValue? value) => value switch + { + null => m_discardNulls, + JsonNull => !ReferenceEquals(value, JsonNull.Null) && m_discardNulls, // note: JsonNull.Null is NEVER discarded + JsonBoolean b => m_discardDefaults && !b.Value, + JsonString or JsonNumber or JsonDateTime => m_discardDefaults && value.IsDefault, + _ => false // arrays and objects are NEVER discarded + }; + + public void WriteField(string name, JsonValue? value) + { + value ??= JsonNull.Null; + if (!WillBeDiscarded(value)) + { + WriteName(name); + value.JsonSerialize(this); + } + } + + public void WriteField(ReadOnlySpan name, JsonValue? value) { - //note: on écrit JsonNull.Null, mais pas JsonNull.Missing! - if (!value.IsNullOrMissing() || !m_discardNulls || object.ReferenceEquals(value, JsonNull.Null)) + value ??= JsonNull.Null; + if (!WillBeDiscarded(value)) { WriteName(name); value.JsonSerialize(this); } } + //note: these overloads only exist to prevent "WriteField(..., new JsonObject())" to call WriteField(..., ...), instead of WriteField(..., JsonValue) + + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void WriteField(string name, JsonNull? value) => WriteField(name, (JsonValue?) value); + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void WriteField(string name, JsonObject? value) => WriteField(name, (JsonValue?) value); + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void WriteField(string name, JsonArray? value) => WriteField(name, (JsonValue?) value); + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void WriteField(string name, JsonString? value) => WriteField(name, (JsonValue?) value); + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void WriteField(string name, JsonNumber? value) => WriteField(name, (JsonValue?) value); + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void WriteField(string name, JsonBoolean? value) => WriteField(name, (JsonValue?) value); + [MethodImpl(MethodImplOptions.AggressiveInlining)] public void WriteField(string name, JsonDateTime? value) => WriteField(name, (JsonValue?) value); + public void WriteField(string name, object? value, Type declaredType) { - if (value != null || !m_discardNulls) + if (value is not null || !m_discardNulls) { WriteName(name); CrystalJsonVisitor.VisitValue(value, declaredType, this); @@ -2647,8 +2719,16 @@ public void WriteField(string name, object? value, Type declaredType) public void WriteField(string name, T value) { - WriteName(name); - VisitValue(value); + if (value is not null) + { + WriteName(name); + VisitValue(value); + } + else if (!m_discardNulls) + { + WriteName(name); + WriteNull(); + } } public void WriteField(string name, T? value) diff --git a/Doxense.Core/Serialization/JSON/ObjectModel/JsonObject.cs b/Doxense.Core/Serialization/JSON/ObjectModel/JsonObject.cs index b34bd00b7..365482dab 100644 --- a/Doxense.Core/Serialization/JSON/ObjectModel/JsonObject.cs +++ b/Doxense.Core/Serialization/JSON/ObjectModel/JsonObject.cs @@ -3197,7 +3197,13 @@ public override void JsonSerialize(CrystalJsonWriter writer) var state = writer.BeginObject(); foreach (var item in this) { - writer.WriteField(item.Key, item.Value); + // first check if the value is not a discarded null or default + if (!writer.WillBeDiscarded(item.Value)) + { + //note: the key may require escaping! + writer.WriteNameEscaped(item.Key); + item.Value.JsonSerialize(writer); + } } writer.EndObject(state); }