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.

Sign up for GitHub

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

Jump to bottom

encoding/json: show nested fields path if DisallowUnknownFields leads to an error #58649

Open
nabokihms opened this issue Feb 22, 2023 · 3 comments
Open

encoding/json: show nested fields path if DisallowUnknownFields leads to an error #58649

nabokihms opened this issue Feb 22, 2023 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Backlog

Comments

@nabokihms
Copy link

Hello, Go community!

Problem

I tried to utilize the DisallowUnknownFields option for json.Decoder and faced a problem. It is not possible get the path of the problematic field from the error.

Example:

The structure

type Credentials struct {
	BasicAuth struct {
		Username string `json:"username"`
		Password string `json:"password"`
	} `json:"basic_auth"`
	BearerToken string `json:"bearer_token"`
}

and the following config

{
  "basic_auth": {
    "unknown_token": "test"
  },
  "unknown_token": "test"
}

on decoding will throw an error json: unknown field "unknown_token", yet it is not clear exactly which field caused it.

https://go.dev/play/p/zzLCOBn3AI8

If the config is a little, it is fine to find the field, but it is not suitable to validate cumbersome configs.

Proposal

Decoder already has all the info. The only thing left is to return it to a user.

The following block

go/src/encoding/json/decode.go

Lines 747 to 749 in 06b6759

} else if d.disallowUnknownFields {
d.saveError(fmt.Errorf("json: unknown field %q", key))
}

should be rewritten like this

} else if d.disallowUnknownFields {
        d.errorContext.FieldStack = append(d.errorContext.FieldStack, string(key))
        d.saveError(fmt.Errorf("json: unknown field %q", strings.Join(d.errorContext.FieldStack, ".")))
}

or (to avoid Join)

} else if d.disallowUnknownFields {
    errMsg := fmt.Sprintf("json: unknown field %q", key)
    if len(d.errorContext.FieldStack) > 0 {
        errMsg += fmt.Sprintf(" in context %q", d.errorContext.FieldStack)
    }
    d.saveError(fmt.Errorf(errMsg))
}
@gopherbot gopherbot added this to the Proposal milestone Feb 22, 2023
@Fazt01 Fazt01 mentioned this issue Feb 23, 2023
Open
@ianlancetaylor
Copy link
Contributor

If I understand correctly, this just changes an error message. That doesn't need to be a proposal, so taking it out of the proposal process.

CC @dsnet @mvdan

@ianlancetaylor ianlancetaylor changed the title proposal: encoding/json: Show nested fields path if DisallowUnknownFields leads to an error encoding/json: show nested fields path if DisallowUnknownFields leads to an error Mar 1, 2023
@ianlancetaylor ianlancetaylor added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed Proposal labels Mar 1, 2023
@ianlancetaylor ianlancetaylor modified the milestones: Proposal, Backlog Mar 1, 2023
@dsnet
Copy link
Member

dsnet commented Mar 1, 2023 •
edited

This is tangentially related to #43126, where the existing mechanism for recording paths is inconsistent about which type system (Go or JSON) it is sources from.

I have a slight preference for fixing #43126 first before opting more errors into having path context.

@nabokihms
Copy link
Author

@dsnet thanks for guiding me there. If it is possible, I'd to fix this error myself after #43126 become merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@ianlancetaylor @dsnet @gopherbot @nabokihms