Skip to content

Improve UnknownFieldError message #669

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Improve UnknownFieldError message #669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions decode.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,13 @@ func (e *DupMapKeyError) Error() string {

// UnknownFieldError describes detected unknown field in CBOR map when decoding to Go struct.
type UnknownFieldError struct {
Index int
Struct string // name of the struct being decoded
Field string // field of the CBOR map that was unexpected
Index int // index of the field in the CBOR map
Comment on lines +210 to +212
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename two fields:

Suggested change
Struct string // name of the struct being decoded
Field string // field of the CBOR map that was unexpected
Index int // index of the field in the CBOR map
StructName string // name of the struct being decoded
FieldName string // unknown field name decoded from CBOR map key
Index int // index of the field in the CBOR map

}

func (e *UnknownFieldError) Error() string {
return fmt.Sprintf("cbor: found unknown field at map element index %d", e.Index)
return fmt.Sprintf("cbor: found unknown field: struct '%s' has no field '%s', at map element index %d", e.Struct, e.Field, e.Index)
Copy link
Owner

@fxamacker fxamacker Jun 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using %s to add field name here to an error message requires security considerations.

In this PR, field name is decoded from untrusted data, so we need to consider the possibility it might contain non-printable characters.

We can use %q instead of %s to escape non-printable characters.

Please also reword the error message to be more consistent with the old message, like this:

Suggested change
return fmt.Sprintf("cbor: found unknown field: struct '%s' has no field '%s', at map element index %d", e.Struct, e.Field, e.Index)
return fmt.Sprintf("cbor: found unknown field %q at map element index %d in struct %q", e.Field, e.Index, e.Struct)

}

// UnacceptableDataItemError is returned when unmarshaling a CBOR input that contains a data item
Expand Down Expand Up @@ -2617,8 +2619,8 @@ MapEntryLoop:
var k any

t := d.nextCBORType()
var keyBytes []byte
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes don't cover all use cases in parseMapToStruct().

Valid map keys in this function can be byte string, text string, or integer (to support keyasint feature). Currently, keyBytes is only used when map key type is byte string or text string.

So we also need to handle unknown field when map key is integer.

Also, please add a test for unknown field of integer type.

if t == cborTypeTextString || (t == cborTypeByteString && d.dm.fieldNameByteString == FieldNameByteStringAllowed) {
var keyBytes []byte
if t == cborTypeTextString {
keyBytes, lastErr = d.parseTextString()
if lastErr != nil {
Expand Down Expand Up @@ -2767,7 +2769,7 @@ MapEntryLoop:

if f == nil {
if errOnUnknownField {
err = &UnknownFieldError{j}
err = &UnknownFieldError{Struct: tInfo.typ.String(), Field: string(keyBytes), Index: j}
d.skip() // Skip value
j++
// skip the rest of the map
Expand Down
4 changes: 2 additions & 2 deletions decode_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6825,7 +6825,7 @@ func TestExtraErrorCondUnknownField(t *testing.T) {
name: "CBOR map unknown field with ExtraDecErrorUnknownField",
data: hexDecode("a461416161614261626143616361446164"), // map[string]string{"A": "a", "B": "b", "C": "c", "D": "d"}
dm: dmUnknownFieldError,
wantErrorMsg: "cbor: found unknown field at map element index 3",
wantErrorMsg: "cbor: found unknown field: struct 'cbor.s' has no field 'D', at map element index 3",
},
}
for _, tc := range testCases {
Expand Down Expand Up @@ -6885,7 +6885,7 @@ func TestStreamExtraErrorCondUnknownField(t *testing.T) {
}

data := hexDecode("a461416161614461646142616261436163a3614161616142616261436163") // map[string]string{"A": "a", "D": "d", "B": "b", "C": "c"}, map[string]string{"A": "a", "B": "b", "C": "c"}
wantErrorMsg := "cbor: found unknown field at map element index 1"
wantErrorMsg := "cbor: found unknown field: struct 'cbor.s' has no field 'D', at map element index 1"
wantObj := s{A: "a", B: "b", C: "c"}

dmUnknownFieldError, _ := DecOptions{ExtraReturnErrors: ExtraDecErrorUnknownField}.DecMode()
Expand Down
Loading