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: exclude panics from duplicate keys #3

Open
josharian opened this issue Dec 27, 2018 · 5 comments
Open

json: exclude panics from duplicate keys #3

josharian opened this issue Dec 27, 2018 · 5 comments

Comments

@josharian
Copy link
Collaborator

go-fuzz finds the following crasher: "{\"o\":0,\"o\":null}". This doesn't survive an Unmarshal/Marshal round trip. However, this is working as intended: The second value for key o overwrites the first during Unmarshal, and thus the round-trip fails. See golang/go#24415 for more discussion. We should teach the json Fuzz function that duplicate keys means a round trip failure is OK.

@elwinar
Copy link

elwinar commented Apr 26, 2019

Adding my comment from golang/go#31309:

About this, I'm not sure we should consider this OK for the parser: what happens is that the field O **int is first filled with a value, then the last pointer of the chain is niled, while on the second unmarshal the first pointer of the chain is niled, hence the deep-equal failure.

This is valid behavior, but I find it inconsistent somewhat: I would expected both payloads to end-up with the same level of pointer being nil, not being dependent on the previous value of the field.

@josharian
Copy link
Collaborator Author

Discussion of how the json parser should handle particular inputs should probably happen in a new issue in the Go repo. (Sorry you keep getting bounced around.)

@elwinar
Copy link

elwinar commented Apr 26, 2019

I expected it. I just wanted to keep this issue updated (or at least linked) because the project here spans so much it's a little hard to keep track of everything (I've spend half my free time yesterday bouncing around while discovering :P ).

@josharian
Copy link
Collaborator Author

I didn't see a new issue get filed, so just in case, cc @mvdan, who probably has opinions about the json parser's behavior.

@mvdan
Copy link

mvdan commented Apr 30, 2019

I don't have an opinion unless someone opens an issue on the tracker with a specific problem or proposed change ;)

The decoder could certainly be taught an option to reject duplicate keys on maps and structs, though. That's the kind of thing I had in mind to fix golang/go#14750 sometime in 1.14.

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

3 participants