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

JSON is sent with Byte-order-mark (BOM) illegally #400

Closed
rocketraman opened this issue Oct 25, 2019 · 7 comments
Closed

JSON is sent with Byte-order-mark (BOM) illegally #400

rocketraman opened this issue Oct 25, 2019 · 7 comments
Assignees
Labels
area:serialization Issue with data serialization / deserialization. bug v3.x

Comments

@rocketraman
Copy link

Describe the bug
The JSON serializer currently sends JSON with a BOM.

  1. BOM is illegal in JSON: https://stackoverflow.com/questions/4990095/json-specification-and-usage-of-bom-charset-encoding

  2. .NET StreamWriter adds it by default, can be configured off: https://stackoverflow.com/questions/5266069/streamwriter-and-utf-8-byte-order-marks

  3. Looks like this code needs to change:

    return new StreamWriter(OpenResponseStream(@this, buffered, preferCompression), encoding);

To Reproduce
Steps to reproduce the behavior:

  1. Send a JSON response
  2. Identify that the response contains a BOM, for example with httpie:
$ http --body GET :7071/foo | head -c 20 | od -xc
0000000    bbef    7bbf    6322    646f    2265    203a    7022    6664
        357 273 277   {   "   c   o   d   e   "   :       "   p   d   f

The initial two bytes are the illegal BOM.

Expected behavior
Do not send the BOM for JSON as per the RFCs.

@geoperez
Copy link
Member

@rocketraman I did the change to remove the BOM in the serializer callback only. Can you check my commit?

@geoperez
Copy link
Member

My output:

geo@GDLA-LT-171107B:~$ http --body GET :8877/api/people/1 | head -c 20 | od -xc
0000000    227b    6449    3a22    3120    222c    614e    656d    3a22
          {   "   I   d   "   :       1   ,   "   N   a   m   e   "   :
0000020    2220    614d
              "   M   a

@rocketraman
Copy link
Author

@geoperez LGTM

@geoperez
Copy link
Member

OK, I'll release a new nuget over the weekend.

@geoperez
Copy link
Member

You can check the new nuget version 3.1.2.

@rdeago rdeago added area:serialization Issue with data serialization / deserialization. bug v3.x labels Nov 26, 2019
@rdeago
Copy link
Collaborator

rdeago commented Nov 26, 2019

@rocketraman can you confirm that this issue is solved?

@rocketraman
Copy link
Author

Sorry for the late reply. Yes, I can confirm this is fixed now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:serialization Issue with data serialization / deserialization. bug v3.x
Projects
None yet
Development

No branches or pull requests

3 participants