Fix WriteEscapedJsonString for Special Control Characters. #6665
Conversation
|
cc: @cyberphone |
| @@ -209,10 +209,62 @@ public static void DCJS_SbyteAsRoot() | |||
| [Fact] | |||
| public static void DCJS_StringAsRoot() | |||
| { | |||
| foreach (string value in new string[] { "abc", " a b ", null, "", " ", "Hello World! 漢 ñ" }) | |||
| foreach (string value in new string[] { "abc", " a b ", null, "", " ", "Hello World! 漢 ñ"}) | |||
There was a problem hiding this comment.
I will revert this line.
|
It's good that the change is largely compatible with desktop .NET, still it changes serialization payload so it could possibly be a breaking change depending on how it's being used by apps. If an app always uses deserialized value, then everything is fine, but in the rare case where an app uses the serialization payload, then it will fail. |
|
@SGuyGe It is of course not my decision but I believe the .NET serializer may very well be the only serializer in the world that does not produce \n for 000a values. A much worse problem is the ordering of properties which (I guess) is incompatible with ES6. |
|
@cyberphone That's kind of unfortunate and that's why we're trying to get it fixed. It's just we need to do it in a safe way without breaking existing apps. |
|
@SGuyGe I'm not a .NET expert but is not one of the points that you can have multiple versions installed? That is, if you absolutely must use features that only existed in an earlier version you can still do that? From my experience you cannot automatically assume that new versions of anything will work flawlessly with your old software, be it operating systems, tools, or libraries. |
|
@cyberphone The concern here is not about .NET Core per se, but about .NET on desktop, as we want these 2 implementations to have the same behavior and can interop. If we change the existing behavior on desktop .NET, it might cause existing apps to fail, which is what we want to avoid. |
|
@SGuyGe I fully understand that although I'm pretty sure we are not talking about very important SW that would break. Anyway, a specific ES6/V8 option would be cool, I just don't know exactly how to add that to .NET. Ideally it should be a "decoration" like [ES6]. Note that such an option would address more things than just \n serialization. Property ordering and Number serialization would also be affected. |
| @@ -1425,6 +1426,33 @@ private unsafe void WriteEscapedJsonString(string str) | |||
| } | |||
| } | |||
|
|
|||
| private bool ProcessSpecialControlCharacter(char ch, out char abbrev) | |||
There was a problem hiding this comment.
I feel TryEscapeControlCharacter() would be a better name.
|
Just one comment on the naming, otherwise LGTM. |
|
LGTM |
|
Updated the PR per feedback. I will merge it after tests passed. |
|
@dotnet-bot test this please |
1 similar comment
|
@dotnet-bot test this please |
|
@dotnet-bot test Innerloop Windows_NT Release Build and Test please |
Fix WriteEscapedJsonString for Special Control Characters.
Fix WriteEscapedJsonString for Special Control Characters. Commit migrated from dotnet/corefx@e5c1682
For some special control characters, the escaped strings generated by DataContractJsonSerializer are not compatible with ECMAScript V6. The PR is to fix this issue. The changes are based on http://www.ecma-international.org/ecma-262/6.0/#sec-quotejsonstring.
Ref #5647.