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

encoding/json: Encoder adds new line at the end of response which is different from how Marshal works #37083

Open
jpninanjohn opened this issue Feb 6, 2020 · 5 comments
Milestone

Comments

@jpninanjohn
Copy link

@jpninanjohn jpninanjohn commented Feb 6, 2020

What version of Go are you using (go version)?

$ go version

 1.13.5

Issue:

Encoder and Marshal behaves differently. Refer

There is a newline added for encoder. Refer to this code.

Where does this break:

In cases where content length is asserted on. Eg. In gin, context.JSON uses Encoder that adds the new line to response thus making the content length of response one more than what was in the actual response. The convention is broken as many libraries dont read the newline. Hence read content length will be one lesser than content length sent in the response.
This fails in HTTP clients due to a difference in the actual content length and the Content-Length header

Refer this issue

What did you expect to see?

Encoder and Marshal should result in the exact same JSON string.

What did you see instead?

An additional line added in response when encoder is used to convert object to json.


cc @dineshba @KaushikNeelichetty @kishaningithub

@AlexRouSg

This comment has been minimized.

Copy link
Contributor

@AlexRouSg AlexRouSg commented Feb 6, 2020

This is working as documented.
https://golang.org/pkg/encoding/json/#Encoder.Encode

Encode writes the JSON encoding of v to the stream, followed by a newline character.

@cagedmantis cagedmantis changed the title Encoder adds new line at the end of response which is different from how Marshal works encoding/json: Encoder adds new line at the end of response which is different from how Marshal works Feb 6, 2020
@cagedmantis cagedmantis added this to the Backlog milestone Feb 6, 2020
@jpninanjohn

This comment has been minimized.

Copy link
Author

@jpninanjohn jpninanjohn commented Feb 7, 2020

@AlexRouSg Yes, issue is not that its not documented, but rather that the difference breaks in the scenario mentioned. I dont mind the newline if nothing breaks but since it does, is it actually needed? (CMIIW, its only there for prettification.. right??)

@cagedmantis can I raise a PR for this?

@AlexRouSg

This comment has been minimized.

Copy link
Contributor

@AlexRouSg AlexRouSg commented Feb 7, 2020

Please see https://golang.org/doc/go1compat for why this cannot be changed. Because it is documented behaviour, someone somewhere is going to depend on it and changing it will break their code.

The most you can do is propose new API but the bar for that is very high, see https://golang.org/doc/faq#x_in_std
and https://github.com/golang/proposal

@AlexRouSg

This comment has been minimized.

Copy link
Contributor

@AlexRouSg AlexRouSg commented Feb 7, 2020

cc @rsc @dsnet @bradfitz @mvdan as per owners

@mvdan

This comment has been minimized.

Copy link
Member

@mvdan mvdan commented Feb 7, 2020

To give some context, the newline isn't there for pretty output. If that were the case, it would be coupled with Indent, but it isn't.

The reason is so that you can write a stream of JSON objects delimited by lines, instead of having to write an entire list of objects all at once. This is a well known way to stream objects in JSON. For example, go test -json uses this, so that it can show you test results as they happen, instead of dumping all the output at the end of the program.

Similarly, the decoder supports this kind of streaming of objects.

What is your suggestion here? Like @AlexRouSg said, we can't change the behavior, even if we wanted to - we would break too many existing programs.

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.