encoding/json/v2: report error when unmarshaling into ambiguous set of fields When MatchCaseInsensitiveNames is specified, an incoming JSON name may match multiple Go struct fields to prospectively unmarshal into. The v1 default behavior is to select the first field declared. This change makes it such that the v2 default is to report an error if multiple candidate fields exist. The ReportErrorsWithLegacySemantics options opts into v1 behavior. Updates #79733 Change-Id: Idcfca4fe0a21c7675c61ee836c51da1b1a719c7b Reviewed-on: https://go-review.googlesource.com/c/go/+/792780 Reviewed-by: Damien Neil <dneil@google.com> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Michael Pratt <mpratt@google.com>
diff --git a/src/encoding/json/v2/arshal_default.go b/src/encoding/json/v2/arshal_default.go index 5760ae7..4799eaf 100644 --- a/src/encoding/json/v2/arshal_default.go +++ b/src/encoding/json/v2/arshal_default.go
@@ -1324,12 +1324,18 @@ name := jsonwire.UnquoteMayCopy(val, flags.IsVerbatim()) f := fields.byActualName[string(name)] if f == nil { + var numMatch int for _, f2 := range fields.lookupByFoldedName(name) { if f2.matchFoldedName(name, &uo.Flags) { - f = f2 - break + if f == nil { + f = f2 // use first seen name by breadth-first order + } + numMatch++ } } + if numMatch > 1 && !uo.Flags.Get(jsonflags.ReportErrorsWithLegacySemantics) { + return newUnmarshalErrorAfter(dec, t, errAmbiguousName) + } if f == nil { if uo.Flags.Get(jsonflags.RejectUnknownMembers) && fields.embeddedFallback == nil { err := newUnmarshalErrorAfter(dec, t, ErrUnknownName)
diff --git a/src/encoding/json/v2/arshal_test.go b/src/encoding/json/v2/arshal_test.go index 6fd6a8b..554ee65 100644 --- a/src/encoding/json/v2/arshal_test.go +++ b/src/encoding/json/v2/arshal_test.go
@@ -7220,11 +7220,18 @@ inVal: new(structNoCase), want: addr(structNoCase{AaA: "AaA", AAa: "AAa", Aaa: "Aaa", AAA: "AAA", AA_A: "AA_A"}), }, { - name: jsontest.Name("Structs/NoCase/CaseInsensitiveDefault"), + name: jsontest.Name("Structs/NoCase/CaseInsensitiveDefault/Ambiguous"), + opts: []Options{jsonflags.ReportErrorsWithLegacySemantics | 1}, inBuf: `{"aa_a":"aa_a"}`, inVal: new(structNoCase), want: addr(structNoCase{AaA: "aa_a"}), }, { + name: jsontest.Name("Structs/NoCase/CaseInsensitiveDefault/Ambiguous"), + inBuf: `{"aa_a":"aa_a"}`, + inVal: new(structNoCase), + want: addr(structNoCase{}), + wantErr: EU(errAmbiguousName).withPos(`{`, "/aa_a").withType('"', T[structNoCase]()), + }, { name: jsontest.Name("Structs/NoCase/MatchCaseSensitiveDelimiter"), opts: []Options{jsonflags.MatchCaseSensitiveDelimiter | 1}, inBuf: `{"aa_a":"aa_a"}`, @@ -7238,17 +7245,24 @@ want: addr(structNoCase{AA_A: "aa_a"}), }, { name: jsontest.Name("Structs/NoCase/Merge/AllowDuplicateNames"), - opts: []Options{jsontext.AllowDuplicateNames(true)}, + opts: []Options{jsontext.AllowDuplicateNames(true), jsonflags.ReportErrorsWithLegacySemantics | 1}, inBuf: `{"AaA":"AaA","aaa":"aaa","aAa":"aAa"}`, inVal: new(structNoCase), want: addr(structNoCase{AaA: "aAa"}), }, { - name: jsontest.Name("Structs/NoCase/Merge/RejectDuplicateNames"), + name: jsontest.Name("Structs/NoCase/Merge/AllowDuplicateNames/Ambiguous"), + opts: []Options{jsontext.AllowDuplicateNames(true)}, + inBuf: `{"AaA":"AaA","aaa":"aaa","aAa":"aAa"}`, + inVal: new(structNoCase), + want: addr(structNoCase{AaA: "AaA"}), + wantErr: EU(errAmbiguousName).withPos(`{"AaA":"AaA",`, "/aaa").withType('"', T[structNoCase]()), + }, { + name: jsontest.Name("Structs/NoCase/Merge/RejectDuplicateNames/Ambiguous"), opts: []Options{jsontext.AllowDuplicateNames(false)}, inBuf: `{"AaA":"AaA","aaa":"aaa"}`, inVal: new(structNoCase), want: addr(structNoCase{AaA: "AaA"}), - wantErr: newDuplicateNameError("", []byte(`"aaa"`), len64(`{"AaA":"AaA",`)), + wantErr: EU(errAmbiguousName).withPos(`{"AaA":"AaA",`, "/aaa").withType('"', T[structNoCase]()), }, { name: jsontest.Name("Structs/CaseSensitive"), inBuf: `{"BOOL": true, "STRING": "hello", "BYTES": "AQID", "INT": -64, "UINT": 64, "FLOAT": 3.14159}`, @@ -7271,17 +7285,11 @@ inVal: addr(structNoCaseEmbedTextValue{AAA: "before"}), want: addr(structNoCaseEmbedTextValue{AAA: "after"}), }, { - name: jsontest.Name("Structs/DuplicateName/NoCase/NoCaseConflict"), - inBuf: `{"aaa":"aaa","aaA":"aaA"}`, + name: jsontest.Name("Structs/AmbiguousName/NoCase"), + inBuf: `{"aaa":"aaa"}`, inVal: addr(structNoCaseEmbedTextValue{}), - want: addr(structNoCaseEmbedTextValue{AaA: "aaa"}), - wantErr: newDuplicateNameError("", []byte(`"aaA"`), len64(`{"aaa":"aaa",`)), - }, { - name: jsontest.Name("Structs/DuplicateName/NoCase/OverwriteNoCase"), - inBuf: `{"aaa":"aaa","aaA":"aaA"}`, - inVal: addr(structNoCaseEmbedTextValue{}), - want: addr(structNoCaseEmbedTextValue{AaA: "aaa"}), - wantErr: newDuplicateNameError("", []byte(`"aaA"`), len64(`{"aaa":"aaa",`)), + want: addr(structNoCaseEmbedTextValue{}), + wantErr: EU(errAmbiguousName).withPos(`{`, "/aaa").withType('"', T[structNoCaseEmbedTextValue]()), }, { name: jsontest.Name("Structs/DuplicateName/Embed/Unknown"), inBuf: `{"unknown":""}`,
diff --git a/src/encoding/json/v2/doc.go b/src/encoding/json/v2/doc.go index b6c8448..21262e2 100644 --- a/src/encoding/json/v2/doc.go +++ b/src/encoding/json/v2/doc.go
@@ -97,7 +97,8 @@ // the value must either be 'ignore' or 'strict'. // The 'ignore' value specifies that matching is case-insensitive // where dashes and underscores are also ignored. If multiple fields match, -// the first declared field in breadth-first order takes precedence. +// then the field with an exact name match is selected, otherwise an error +// is reported due to an ambiguous set of candidate fields to unmarshal into. // The 'strict' value specifies that matching is case-sensitive. // This takes precedence over the [MatchCaseInsensitiveNames] option. //
diff --git a/src/encoding/json/v2/errors.go b/src/encoding/json/v2/errors.go index fd9431d..cbaf95c 100644 --- a/src/encoding/json/v2/errors.go +++ b/src/encoding/json/v2/errors.go
@@ -39,6 +39,10 @@ // This error is only returned if [RejectUnknownMembers] is true. var ErrUnknownName = errors.New("unknown object member name") +// errAmbiguousName indicates a JSON object member could be unmarshal +// into multiple candidate Go struct fields. +var errAmbiguousName = errors.New("ambiguous object member name") + const errorPrefix = "json: " func isSemanticError(err error) bool { @@ -396,9 +400,9 @@ } // Special handling for unknown names. - if e.Err == ErrUnknownName { + if e.Err == ErrUnknownName || e.Err == errAmbiguousName { sb.WriteString(": ") - sb.WriteString(ErrUnknownName.Error()) + sb.WriteString(e.Err.Error()) sb.WriteString(" ") sb.WriteString(strconv.Quote(e.JSONPointer.LastToken())) if parent := e.JSONPointer.Parent(); parent != "" {
diff --git a/src/encoding/json/v2/options.go b/src/encoding/json/v2/options.go index fe3bb8e..ebb311c 100644 --- a/src/encoding/json/v2/options.go +++ b/src/encoding/json/v2/options.go
@@ -190,7 +190,7 @@ // MatchCaseInsensitiveNames specifies that JSON object members are matched // against Go struct fields using a case-insensitive match of the name. // If a name matches multiple fields, it chooses the field with an exact -// match of the name, otherwise it uses the field that appears first. +// match of the name, otherwise it reports an error. // Go struct fields explicitly marked with `case:strict` or `case:ignore` // always use case-sensitive (or case-insensitive) name matching, // regardless of the value of this option.
diff --git a/src/encoding/json/v2_options.go b/src/encoding/json/v2_options.go index a4de2e6..0ce601a0 100644 --- a/src/encoding/json/v2_options.go +++ b/src/encoding/json/v2_options.go
@@ -460,6 +460,11 @@ // are ignored and the Go struct uses a best-effort representation. // In contrast, the v2 semantic is to report a runtime error. // +// - When unmarshaling with [jsonv2.MatchCaseInsensitiveNames], if a JSON +// object name has a non-exact match with multiple Go struct fields, then +// an error is not reported and instead the first declared field is used. +// In contrast, the v2 semantic is to report a runtime error. +// // - When unmarshaling, the syntactic structure of the JSON input // is fully validated before performing the semantic unmarshaling // of the JSON data into the Go value. Practically speaking,