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

parseJsonBody with readALLUTF8 instead of readAll + cast's #1799

Merged
merged 1 commit into from Jun 29, 2017

Conversation

Projects
None yet
3 participants
@wilzbach
Contributor

wilzbach commented Jun 29, 2017

#1677

FWIW in theory JSON can be encoded in UTF-16 and UTF-32 (see references below), but the current implementation doesn't handle this as well, it just silently assumes UTF-8.

References

https://tools.ietf.org/html/rfc7159#section-8

JSON text SHALL be encoded in UTF-8, UTF-16, or UTF-32. The default
encoding is UTF-8, and JSON texts that are encoded in UTF-8 are
interoperable in the sense that they will be read successfully by the
maximum number of implementations; there are many implementations
that cannot successfully read texts in other encodings (such as
UTF-16 and UTF-32).

https://www.ietf.org/rfc/rfc4627.txt

JSON text SHALL be encoded in Unicode. The default encoding is
UTF-8.

Since the first two characters of a JSON text will always be ASCII
characters [RFC0020], it is possible to determine whether an octet
stream is UTF-8, UTF-16 (BE or LE), or UTF-32 (BE or LE) by looking
at the pattern of nulls in the first four octets.

       00 00 00 xx  UTF-32BE
       00 xx 00 xx  UTF-16BE
       xx 00 00 00  UTF-32LE
       xx 00 xx 00  UTF-16LE
       xx xx xx xx  UTF-8
@wilzbach

This comment has been minimized.

Show comment
Hide comment
@wilzbach

wilzbach Jun 29, 2017

Contributor

(btw closely related: there's still #1677 in the queue - I guess you merge this after the 0.8.0 release next week?)

Contributor

wilzbach commented Jun 29, 2017

(btw closely related: there's still #1677 in the queue - I guess you merge this after the 0.8.0 release next week?)

@s-ludwig s-ludwig merged commit 0abc6fe into vibe-d:master Jun 29, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jun 29, 2017

Member

Oops, I actually wanted to wait with this too until after 0.8.0... So then I'll just create a branch for the 0.8.0 release. That means that #1677 can be merged now, too.

Member

s-ludwig commented Jun 29, 2017

Oops, I actually wanted to wait with this too until after 0.8.0... So then I'll just create a branch for the 0.8.0 release. That means that #1677 can be merged now, too.

@wilzbach wilzbach deleted the wilzbach:use-utf8 branch Jun 29, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment