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.

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

DataContractJsonSerializer incompatible with ES6 (and V8) #16204

Closed
cyberphone opened this issue Jan 23, 2016 · 16 comments
Closed

DataContractJsonSerializer incompatible with ES6 (and V8) #16204

cyberphone opened this issue Jan 23, 2016 · 16 comments
Assignees
Milestone

Comments

@cyberphone
Copy link

Bug report with code:
https://github.com/cyberphone/jsondotnet

@JamesNK
Copy link
Member

JamesNK commented Jan 25, 2016

I think an ES6 compatible way to ToString double and floats would be useful beyond just DataContractJsonSerializer.

@SGuyGe SGuyGe assigned shmao and unassigned khdang Jan 28, 2016
@shmao
Copy link
Contributor

shmao commented Feb 25, 2016

Thanks for the suggestion, @JamesNK . That seems reasonable to me. I opened https://github.com/dotnet/corefx/issues/6405 for tracking this.

DataContractJsonSerializer converts a double value to string by calling,

value.ToString("R", NumberFormatInfo.InvariantInfo)

@JamesNK
Copy link
Member

JamesNK commented Feb 25, 2016

That is what Json.NET does as well.

@cyberphone
Copy link
Author

If you look into the published .NET code in the initial bug report, there are other incompatibility issues as well. My wish is to be able to use the following with .NET:
https://cyberphone.github.io/openkeystore/resources/docs/jsonsignatures.html

@shmao
Copy link
Contributor

shmao commented Feb 29, 2016

@cyberphone can you please direct me to the spec for the following point? Thanks.

DataContractJsonSerializer:
\u000a should be serialized as \n

@cyberphone
Copy link
Author

Hi @shmao
When reading section 24.3.2.2 paragraph 2 b of
http://www.ecma-international.org/ecma-262/6.0/ECMA-262.pdf
I do this interpretation.

All browsers do this so it can't be completely wrong :-)

Anyway, it is a prerequisite for "predictable serialization" which is what you need for interoperable in-object JSON signatures.

@shmao
Copy link
Contributor

shmao commented Mar 1, 2016

Thanks for the info, @cyberphone . According to Note 3 in section 24.3.2,

String values are wrapped in QUOTATION MARK (") code units. The code units " and \ are escaped with \ prefixes. Control characters code units are replaced with escape sequences \uHHHH, or with the shorter forms, \b (BACKSPACE), \f (FORM FEED), \n (LINE FEED), \r (CARRIAGE RETURN), \t (CHARACTER TABULATION).

Serializing "\u000a" into ""\u000a"" seems correct, right?

@cyberphone
Copy link
Author

@shmao The spec says or with the shorter forms which seems like an alternative. And it is OK according to the JSON spec. But the following lines say different things:

24.3.2.2 Runtime Semantics: QuoteJSONString ( value )
The abstract operation QuoteJSONString with argument value wraps a String value in QUOTATION MARK code
units and escapes certain other code units within it.

  1. Let product be code unit 0x0022 (QUOTATION MARK).
  2. For each code unit C in value
    a. If C is 0x0022 (QUOTATION MARK) or 0x005C (REVERSE SOLIDUS), then
    i. Let product be the concatenation of product and code unit 0x005C (REVERSE SOLIDUS).
    ii. Let product be the concatenation of product and C.
    b. Else if C is 0x0008 (BACKSPACE), 0x000C (FORM FEED), 0x000A (LINE FEED), 0x000D
    (CARRIAGE RETURN), or 0x000B (LINE TABULATION), then
    i. Let product be the concatenation of product and code unit 0x005C (REVERSE SOLIDUS).
    ii. Let abbrev be the String value corresponding to the value of C as follows:
    BACKSPACE "b"
    FORM FEED (FF) "f"
    LINE FEED (LF) "n"
    CARRIAGE RETURN (CR) "r"
    LINE TABULATION "t"
    iii. Let product be the concatenation of product and abbrev.

@shmao
Copy link
Contributor

shmao commented Mar 1, 2016

Thanks for pointing out that, @cyberphone. I will take a look at this.

@shmao
Copy link
Contributor

shmao commented Mar 4, 2016

I'm not sure how to escape 0x000B. According to the spec, 0x000B (LINE TABULATION) should be serialized as \t. But 0x000B is \v not \t. Escaping it to \t wouldn't make a round trip.

@cyberphone
Copy link
Author

@shmao Whow! You have found a bug in the spec. JSON only talks about \t (0009) so that's the one we care about. \v should be serialized as \u000b.

@cyberphone
Copy link
Author

Having looked around a bit it seems that the JSON world is rather divided and needs to consider supporting two different serializations, one which is ES6-compatible and another which is ES6-compatible but properly escaped to be declared in browser as a JavaScript object. The latter means that '<', '>', and '&' must be expressed as \unnnn. Preferably quotes around identifier-compatible property names are also dropped in the JavaScript mode.

@shmao
Copy link
Contributor

shmao commented Mar 18, 2016

@cyberphone , thanks for the additional info. Can you please point me to the specs for these two different requirements?

Sorry for the late response, I am busy with some other issues recently.

@cyberphone
Copy link
Author

@shmao Well, "on the wire" only real JSON encoding makes sense, but if you express JSON as a JavaScript object embedded in an HTML page like this

var jsonData = {
  myprop: "</script>"
};

it will break in a browser so I made (in my Java tools...), JavaScript a serializer option that:

  • escapes '<', '>', and '&'
  • removes "" around properties that adhere to JavaScript identifier syntax (excluding for example "my-prop")
  • prints data in a pretty (readable) way

The cool thing is that JSON.stringify() is unaffected by this change since it using ES6 restores data to its true JSON format!

@shmao
Copy link
Contributor

shmao commented Mar 21, 2016

The fix for serializing control characters has been merged. dotnet/corefx#6665

I think we should open a new issue for the above suggestion, escaping '<', '>', and '&', and etc as this issue is for tracking DataContractJsonSerializer being incompatible with ES6.

@shmao
Copy link
Contributor

shmao commented Mar 31, 2016

Opened dotnet/corefx#7416 for tracking the above suggestion.
Closing the issue as PR dotnet/corefx#6665 was merged.

@shmao shmao closed this as completed Mar 31, 2016
@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.0.0-rtm milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants