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

asString doesn't escape U+2028 and U+2029 #1

Closed
rmunn opened this issue Aug 31, 2014 · 3 comments
Closed

asString doesn't escape U+2028 and U+2029 #1

rmunn opened this issue Aug 31, 2014 · 3 comments

Comments

@rmunn
Copy link

rmunn commented Aug 31, 2014

Due to the difference between what's legal in JSON and Javascript strings, it's a good idea to escape the two characters that are legal in JSON output but not legal in Javascript output, U+2028 and U+2029, so that the JSON parser's output can be safely used to produce JSONP. See rack/rack-contrib#37 for a more detailed discussion of the rationale.

@rmunn
Copy link
Author

rmunn commented Aug 31, 2014

Note that this might not be pjson's responsibility. If the intent of pjson is never to be used for producing JSONP, but only "pure" JSON, then special-casing two Unicode characters that are perfectly legal in JSON strings would likely hurt performance a little for no gain. But in that case, it might be a good idea to at least document that these two characters are not escaped, to avoid any surprises: if someone later tries to use the library to produce JSONP, they would need to know that it's their responsibility to search for and escape U+2028 and U+2029.

@gerritjvv
Copy link
Owner

hi, good catch.

The library so far has only concentrated on getting the max speed.
I'll see if I can add it without major impact on perf, otherwise I'll document it in the readme as you describe.

@gerritjvv
Copy link
Owner

for the moment I'll put in a JSONP section explaining this in the README.
I'm going to close this issue and open an new one for v0.2.1 with a asJSONP implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants