Skip to content
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.

Sign up for GitHub

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

fix + in json #3663

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from
+18 −6
Draft

fix + in json #3663

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0917647
Try fix
shargon Jan 8, 2025
6a77c33
Merge branch 'master' into json-plus
Jim8y Jan 9, 2025
83b1394
Merge branch 'master' into json-plus
Jim8y Jan 11, 2025
3c15c6f
Fixed unsafe relaxing
cschuchardt88 Jan 17, 2025
ebb00e7
Merge branch 'master' into json-plus
cschuchardt88 Jan 17, 2025
9bb0fe7
Allow plus sign `+`
cschuchardt88 Jan 17, 2025
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
13 changes: 12 additions & 1 deletion src/Neo.Json/JToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using System.Text.Encodings.Web;
using System.Text.Json;
using System.Text.RegularExpressions;
using System.Text.Unicode;
using static Neo.Json.Utility;

namespace Neo.Json
Expand All @@ -19,6 +22,13 @@ namespace Neo.Json
/// </summary>
public abstract class JToken
{
public readonly static JavaScriptEncoder DefaultEncoder;

static JToken()
{
DefaultEncoder = JavaScriptEncoder.Create(UnicodeRanges.BasicLatin);
}

/// <summary>
/// Represents a <see langword="null"/> token.
/// </summary>
Expand Down Expand Up @@ -239,6 +249,7 @@ public byte[] ToByteArray(bool indented)
using Utf8JsonWriter writer = new(ms, new JsonWriterOptions
{
Indented = indented,
Encoder = DefaultEncoder,
SkipValidation = true
});
Write(writer);
Expand All @@ -262,7 +273,7 @@ public override string ToString()
/// <returns>The encoded JSON token.</returns>
public string ToString(bool indented)
{
return StrictUTF8.GetString(ToByteArray(indented));
return Regex.Unescape(StrictUTF8.GetString(ToByteArray(indented)));
cschuchardt88 marked this conversation as resolved.
Show resolved Hide resolved
}

internal abstract void Write(Utf8JsonWriter writer);
Expand Down
4 changes: 1 addition & 3 deletions src/Neo.Json/Utility.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ static class Utility

static Utility()
{
StrictUTF8 = (Encoding)Encoding.UTF8.Clone();
StrictUTF8.DecoderFallback = DecoderFallback.ExceptionFallback;
StrictUTF8.EncoderFallback = EncoderFallback.ExceptionFallback;
StrictUTF8 = new UTF8Encoding(false, true);
}
}
}
1 change: 1 addition & 0 deletions src/Neo/SmartContract/JsonSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ public static byte[] SerializeToByteArray(StackItem item, uint maxSize)
using Utf8JsonWriter writer = new(ms, new JsonWriterOptions
{
Indented = false,
Encoder = JToken.DefaultEncoder,
SkipValidation = false
});
Stack stack = new();
Expand Down
7 changes: 5 additions & 2 deletions tests/Neo.UnitTests/SmartContract/UT_JsonSerializer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,13 @@ public void JsonTest_Object()

Assert.AreEqual(@"{""test"":true}", parsed.ToString());

json = @" {""\uAAAA"": true}";
json = @" {""test"":""+""} ";
Copy link
Member

Choose a reason for hiding this comment

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

Can we keep this test-case with \uAAAA? It's a nice compatibility test for NeoGo.

Copy link
Member Author

Choose a reason for hiding this comment

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

In fact I think this will be dropped, no way to do it without unsafe relaxing if we use native json

Copy link
Member

@cschuchardt88 cschuchardt88 Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Doesnt use unsafe relaxing currently. And its working fine now.

Copy link
Member Author

@shargon shargon Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Replace is a dirty fix, performance is affected always, not only when it contains +

Copy link
Member

@cschuchardt88 cschuchardt88 Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

RegEx.UnEscape? https://learn.microsoft.com/en-us/dotnet/api/system.text.regularexpressions.regex.unescape?view=net-9.0

Could use: https://github.com/dotnet/runtime/blob/4c224ab16d287c17dbc54236638db91266071465/src/libraries/System.Text.Json/src/System/Text/Json/Reader/JsonReaderHelper.Unescaping.cs#L69

parsed = JObject.Parse(json);
Assert.AreEqual(@"{""test"":""+""}", parsed.ToString());
cschuchardt88 marked this conversation as resolved.
Show resolved Hide resolved

Assert.AreEqual(@"{""\uAAAA"":true}", parsed.ToString());
json = @" {""测试"": true}";
Copy link
Member

Choose a reason for hiding this comment

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

Had to allow UnicodeRanges.CjkUnifiedIdeographs for these characters for json DefaultEncoder.

parsed = JObject.Parse(json);
Assert.AreEqual(@"{""测试"":true}", parsed.ToString());

json = @"{""a"":}";
Assert.ThrowsException<FormatException>(() => JObject.Parse(json));
Expand Down
Loading