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: custom type unmarshaling #47078

Closed
imacks opened this issue Jul 7, 2021 · 3 comments
Closed

encoding/json: custom type unmarshaling #47078

imacks opened this issue Jul 7, 2021 · 3 comments
Labels
FeatureRequest FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@imacks
Copy link
Contributor

imacks commented Jul 7, 2021

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

$ go 1.16.5

Does this issue reproduce with the latest release?

Yes

What did you do?

I have a slightly non-spec json input to parse:

{
  "foo": NaN
}

I tried unmarshaling to json.RawMessage, but can't go past json.Unmarshal. The error was thrown by the checkValid func.

What did you expect to see?

This is similar to issue #3480. But since NaN is not quoted, the parser dismiss it as invalid json and never get to the custom UnmarshalJSON.

What did you see instead?

Error saying cannot parse at 'N'.

This limitation also apply to things like Inf, 0x1234, etc. It would be great if there is an option to relax validation so that it can tolerate unquoted values, with the understanding that these values will be handled by custom unmarshalers. Obviously, the parser should return error if such values contain reserved chars like , : {} [].

@mknyszek mknyszek added FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 7, 2021
@mknyszek mknyszek added this to the Backlog milestone Jul 7, 2021
@mknyszek
Copy link
Contributor

mknyszek commented Jul 7, 2021

I think NaN might be a lost cause (see #3480 and #25721), at least in the current encoding/json API. Closing this issue. Please feel free to comment about reopening if you think there's something new not captured in those conversations that you'd like to bring up about this, though.

@mknyszek mknyszek closed this as completed Jul 7, 2021
@imacks
Copy link
Contributor Author

imacks commented Jul 7, 2021

Wow tks for the prompt response. I may be mistaken, but the 2 issues you mentioned talked about making the parser recognize specific keywords such as NaN, Inf, etc., which is not a good idea imo as it goes against the JSON spec.

Here, I am saying that the parser should not be too eager to throw an error when it read an invalid value. For example, the parser will error out at x when I want to be able to unmarshal {"foo":0xBEEF}, even if I am unmarshaling to map[string]json.RawMessage. My idea is that so long as we assume that a non-string value cannot contain any reserved character , [ ] { } : " and we're unmarshaling that value to json.RawMessage, the parser can simply assign without validation and continue. This gives the user a way to handle non-JSON spec values further down the line.

Since the proposed will change the parser's current behavior, maybe there can be a x/json package for this?

@dolmen
Copy link
Contributor

dolmen commented Jul 9, 2021

This is just NOT JSON. Don't use encoding/json to parse that input.

My idea is that so long as we assume that a non-string value cannot contain any reserved character , [ ] { } : " and we're unmarshaling that value to json.RawMessage, the parser can simply assign without validation and continue.

This would break existing code that assumes that content of json.RawMessage contains pure JSON.

@golang golang locked and limited conversation to collaborators Jul 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

4 participants