Golang encoding/json: eof error of NewDecoder().Decode() should be same with Unmarshal()

Please answer these questions before submitting your issue. Thanks!

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

go version go1.10.1 darwin/amd64

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (`go env`)?

GOARCH="amd64" GOOS="darwin"

What did you do?

https://play.golang.org/p/gr5cHhrpmbK

What did you expect to see?

shown in previous link

What did you see instead?

shown in previous link

Comment From: agnivade

/cc @dsnet

Comment From: isayme

BTW. a solution is change io.ErrUnexpectedEOF to &SyntaxError{"unexpected end of JSON input", dec.scan.bytes} in https://github.com/golang/go/blob/c99300229de4e69220790c71da14785dc52c3d68/src/encoding/json/stream.go#L126

Comment From: ianlancetaylor

Seems worth changing, want to send a CL?

Comment From: agnivade

@ianlancetaylor - Just concerned, can it be considered a borderline breaking change ?

Comment From: ianlancetaylor

@agnivade Yes, it's clearly possible to write a program that will observe this change. I'm guessing that few programs test explicitly for io.ErrUnexpectedEOF. But I don't actually know. The only way to find out would be to try it.

Actually thinking about it a bit more I'm not sure which way to change it. Is it better to always return a json.SyntaxError? Or is is better to not wrap io.ErrUnexpectedEOF in a json.SyntaxError, just as we generally avoid wrapping io.EOF?

Comment From: agnivade

A quick github search for TestInvalidJSON gives some hits which are potential breakages.

https://github.com/lemonldap-ng-controller/lemonldap-ng-controller/blob/a151ed71970d6199a936f028a25fcc2969da7aff/internal/lemonldapng/converter/converter_test.go#L56
https://github.com/smagch/patch/blob/fee8a6dae4f796e0714a27c0a06d356e3059ce33/parse_test.go#L85

There might be more but I stopped searching after I got these.

Both of these do json.Unmarshal, so I guess it might be safer to change json.NewDecoder. But again, there just might be some code out there which breaks.

Comment From: dsnet

Arguments for changing it to *json.SyntaxError: * io.ErrUnexpectedEOF is documented as indicating that "EOF was encountered in the middle of reading a fixed-size block or data structure", for which this does not seem to be the case. * ~~This situation really does look like syntax error. There is no amount of characters that you can add to {"name":"} to make it valid JSON.~~ EDIT: not true, you can append "} as well here.

That being said, I may have expected {"name":" to return io.ErrUnexpectedEOF, since it is possible to append "} and obtain valid JSON. However, this is not the case today, either.

See https://play.golang.org/p/93ozJzoQSRR

Comment From: dsnet

I support changing it to io.ErrUnexpectedEOF for the following reasons: * Despite the documentation on ErrUnexpectedEOF, I (and my observation of other parser code) has commonly interpreted that error as indicating that is some number of bytes you could be appended to the string to turn in into a valid input. * It is generally safer to move from a less distinguishable to a more distinguishable error. While SyntaxError is distinguishable, it is functionally less so than ErrUnexpectedEOF, which only requires a comparison instead of a type assertion.

Comment From: agnivade

@dsnet - I tried changing it to io.ErrUnexpectedEOF and several tests failed -.

--- FAIL: TestUnmarshal (0.00s)
    decode_test.go:1016: #38: checkValid: &errors.errorString{s:"unexpected EOF"}
--- FAIL: TestUnmarshalSyntax (0.00s)
    decode_test.go:1962: expected syntax error for Unmarshal("\"hello"): got *errors.errorString
    decode_test.go:1962: expected syntax error for Unmarshal("[1,2,3"): got *errors.errorString
    decode_test.go:1962: expected syntax error for Unmarshal("{\"key\":1"): got *errors.errorString
    decode_test.go:1962: expected syntax error for Unmarshal("{\"key\":1,"): got *errors.errorString
FAIL
FAIL    encoding/json   1.086s

Whereas, if I do the other way and change to SyntaxError, no tests fail. It seems to me that changing to SyntaxError may actually be less invasive.

But your call. Let me know.

Comment From: agnivade

ping @dsnet @mvdan

Comment From: mvdan

I know very little about encoding/json's history with errors, sorry. It sounds to me like parsing "hello is a SyntaxError by definition, but I can see the points in favor of using io errors without wrapping.

While SyntaxError is distinguishable, it is functionally less so than ErrUnexpectedEOF, which only requires a comparison instead of a type assertion.

Sounds to me like the Go2 error proposals would help a bit here :) Maybe we can wrap io errors in json.SyntaxError once/if those proposals are implemented.

It seems to me that changing to SyntaxError may actually be less invasive.

If it's just tests that break, and the breakage is consistent with what we're changing here, I'd just fix the tests. Unless what you mean is that unrelated tests are starting to fail. The cases you pasted do look like unexpected EOFs.

Comment From: agnivade

Sure, I can fix the tests. Just needed a confirmation on whether to go ahead or not.

Comment From: montanaflynn

I'm also hitting this, it's awkward to have to check if err == io.ErrUnexpectedEOF before a type assertion switch containing *json.SyntaxError.

Comment From: s3rj1k

Any news on this?

Comment From: mvdan

I think the solution here needs to be adapted given that error wrapping is a reality, e.g. see https://golang.org/pkg/errors/#Is.

SyntaxError should now wrap the underlying error, such as io.ErrUnexpectedEOF. And the json APIs should all be consistent with this.

If anyone wants to work on a patch and send it, I'm happy to review it. Otherwise, I don't think anyone is actively working on this, so there won't be any news unless someone leaves a comment with an update.

Comment From: mvdan

I should also clarify - such a change is too risky now that we're in the middle of the 1.14 freeze, so a CL would probably be merged once the tree reopens in roughly six weeks from now.

Comment From: ali2210

Sir I'm using go1.14

var member Member // struct type err := json.NewDecoder(request.Body).Decode(&member); if err != nil{println(err)} // yeah this return eof & I expected value

Comment From: ahmad-abuziad

to reflect the issue and get io.ErrUnexpectedEOF decode the following JSON {"key_without_value": to get json.SyntaxError decode the following JSON {'key_surrounded_by_single_quotes':

io.ErrUnexpectedEOF

var ErrUnexpectedEOF = errors.New("unexpected EOF")

json.SyntaxError

type SyntaxError struct { msg string // description of error Offset int64 // error occurred after reading Offset bytes }

Note that json.SyntaxError provides the Offset

Comment From: gopherbot

Change https://go.dev/cl/687115 mentions this issue: encoding/json/v2: report wrapped io.ErrUnexpectedEOF

Comment From: dsnet

json/v2 fixes this such that Unmarshal, UnmarshalRead, and UnmarshalDecode are consistent about how it reports a truncated input. In particular, it reports an io.ErrUnexpectedEOF wrapped within a jsontext.SyntacticError.

However, we'll likely just have v1 preserve existing semantics to avoid breaking anyone who is depending on io.ErrUnexpectedEOF.

Golang encoding/json: eof error of NewDecoder().Decode() should be same with Unmarshal()

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

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

What did you expect to see?

What did you see instead?

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

What operating system and processor architecture are you using (`go env`)?