cbor icon indicating copy to clipboard operation
cbor copied to clipboard

bug: Unmarshal and Valid should check following extraneous data

Open zensh opened this issue 3 years ago • 2 comments

What version of fxamacker/cbor are you using?

v2.4.0

Does this issue reproduce with the latest release?

Yes

What OS and CPU architecture are you using (go env)?

MacOS, m1

go env Output
$ go env

What did you do?

data := []byte{0, 1, 2, 3}
err := cbor.Valid(data) // should return a error

var val interface{}
err = cbor.Unmarshal(data, &val) // should return a error

What did you expect to see?

What did you see instead?

zensh avatar Jul 20 '22 11:07 zensh

https://github.com/fxamacker/cbor/blob/5cd39e10d523e537553c37fbf35187b537664706/valid.go#L76

	_, err := d.validInternal(0)
	if err == nil && len(d.data) != d.off {
		err = &SyntaxError{"cbor: unexpected following extraneous " + strconv.Itoa(len(d.data)-d.off) + " bytes"}
	}
	return err

zensh avatar Jul 20 '22 11:07 zensh

@zensh Thank you for reporting this!

Unmarshal() and Valid() work this way to be consistent with decoder, which can continue decoding extra data. But I understand your point because Unmarshal() and Valid() only decodes one piece of data.

Rejecting extraneous data would be a breaking change. So maybe it can be added as a non-default DecOption. This new option would only apply to Unmarshal() and Valid(). Thoughts?

fxamacker avatar Jul 23 '22 21:07 fxamacker

@zensh Thanks again for raising this issue!

After rereading relevant sections in RFC 8949 and comparing to how this is handled in encoding/json, I changed this from enhancement to a bug.

It makes more sense to return extraneous data error by default when calling Unmarshal() and Valid(). This way, the change would not modify API signature and it would be a bug fix to be more consistent with RFC 8949 and also handle this like encoding/json package.

Would you like to update PR #360 or would you like me to open a new PR?

fxamacker avatar Dec 30 '22 22:12 fxamacker