Skip to content

Commit

Permalink
json: fix JsonObject would not be properly escaped in some scenarios
Browse files Browse the repository at this point in the history 
- 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
  • Loading branch information
@KrzysFR
KrzysFR committed Jul 5, 2024
1 parent 15408d7 commit 8e399fb
Show file tree
Hide file tree
Showing 5 changed files with 144 additions and 27 deletions.
17 changes: 16 additions & 1 deletion Doxense.Core.Tests/Serialization/JSON/CrystalJson.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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<int>("""Hello\World"""), Is.EqualTo(123));
Assert.That(obj.Get<int>("""Hello"World"""), Is.EqualTo(456));
Assert.That(obj.Get<int>("""\\?\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 }"));
Expand Down
20 changes: 18 additions & 2 deletions Doxense.Core/Serialization/JSON/CrystalJsonFormatter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
}

Expand All @@ -109,10 +119,16 @@ public static void WriteJsonString(TextWriter writer, ReadOnlySpan<char> 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<char> 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)
Expand Down
6 changes: 3 additions & 3 deletions Doxense.Core/Serialization/JSON/CrystalJsonStreamWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand Down
120 changes: 100 additions & 20 deletions Doxense.Core/Serialization/JSON/CrystalJsonWriter.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ public void WriteRaw(string? rawJson)
public void WriteName(string name)
{
WriteFieldSeparator();
WritePropertyName(name);
WritePropertyName(name, knownSafe: true);
}

/// <summary>Write a property name that is KNOWN to not require any escaping.</summary>
Expand All @@ -915,17 +915,48 @@ public void WriteName(string name)
public void WriteName(ReadOnlySpan<char> name)
{
WriteFieldSeparator();
WritePropertyName(name);
WritePropertyName(name, knownSafe: true);
}

/// <summary>Write a property name that MAY require escaping.</summary>
/// <param name="name">Name of the property that will be escaped if necessary</param>
/// <remarks>
/// <para>This method should be used whenever the origin of key is not controlled, and may contains any character that would required escaping ('\', '"', ...).</para>
/// </remarks>
public void WriteNameEscaped(string name)
{
WriteFieldSeparator();
WritePropertyName(name, knownSafe: false);
}

/// <summary>Write a property name that MAY require escaping.</summary>
/// <param name="name">Name of the property that will be escaped if necessary</param>
/// <remarks>
/// <para>This method should be used whenever the origin of key is not controlled, and may contains any character that would required escaping ('\', '"', ...).</para>
/// </remarks>
public void WriteNameEscaped(ReadOnlySpan<char> 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
{
Expand All @@ -940,7 +971,7 @@ internal void WriteJavaScriptName(string name)
buffer.Write(m_formatted ? JsonTokens.ColonFormatted : JsonTokens.ColonCompact);
}

internal void WritePropertyName(ReadOnlySpan<char> name)
internal void WritePropertyName(ReadOnlySpan<char> name, bool knownSafe)
{
if (m_javascript)
{
Expand All @@ -949,18 +980,28 @@ internal void WritePropertyName(ReadOnlySpan<char> 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<char> name)
Expand Down Expand Up @@ -2626,19 +2667,50 @@ public void WriteField(string name, NodaTime.DateTimeZone? value)

#endregion

public void WriteField(string name, JsonValue value)
/// <summary>Tests if the specified value would have been discarded when calling <see cref="WriteField(string,JsonValue)"/></summary>
[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<char> 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<JsonObject>(..., ...), 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);
Expand All @@ -2647,8 +2719,16 @@ public void WriteField(string name, object? value, Type declaredType)

public void WriteField<T>(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<T>(string name, T? value)
Expand Down
8 changes: 7 additions & 1 deletion Doxense.Core/Serialization/JSON/ObjectModel/JsonObject.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down

0 comments on commit 8e399fb

Please sign in to comment.