Skip to content

Commit

Permalink
Handle more legacy semantics in binary data representation (#93)
Browse files Browse the repository at this point in the history
Only reject whitespace in base32 or base64 data if
FormatBytesWithLegacySemantics is not specified.

Only reject mismatching array lengths only if
UnmarshalArrayFromAnyLength is not specified.
  • Loading branch information
dsnet authored Dec 30, 2024
1 parent b558781 commit 0240acd
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
4 changes: 2 additions & 2 deletions arshal_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ func makeBytesArshaler(t reflect.Type, fncs *arshaler) *arshaler {
if err != nil {
return newUnmarshalErrorAfter(dec, t, err)
}
if len(val) != encodedLen(len(b)) {
if len(val) != encodedLen(len(b)) && !uo.Flags.Get(jsonflags.FormatBytesWithLegacySemantics) {
// TODO(https://go.dev/issue/53845): RFC 4648, section 3.3,
// specifies that non-alphabet characters must be rejected.
// Unfortunately, the "base32" and "base64" packages allow
Expand All @@ -402,7 +402,7 @@ func makeBytesArshaler(t reflect.Type, fncs *arshaler) *arshaler {
if va.Kind() == reflect.Array {
dst := va.Bytes()
clear(dst[copy(dst, b):]) // noop if len(b) <= len(dst)
if len(b) != len(dst) {
if len(b) != len(dst) && !uo.Flags.Get(jsonflags.UnmarshalArrayFromAnyLength) {
err := fmt.Errorf("decoded length of %d mismatches array length of %d", len(b), len(dst))
return newUnmarshalErrorAfter(dec, t, err)
}
Expand Down
18 changes: 18 additions & 0 deletions arshal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4856,12 +4856,24 @@ func TestUnmarshal(t *testing.T) {
inVal: new([2]byte),
want: addr([2]byte{1, 0}),
wantErr: EU(errors.New("decoded length of 1 mismatches array length of 2")).withType('"', T[[2]byte]()),
}, {
name: jsontest.Name("Bytes/ByteArray2/Underflow/Allowed"),
opts: []Options{jsonflags.UnmarshalArrayFromAnyLength | 1},
inBuf: `"AQ=="`,
inVal: new([2]byte),
want: addr([2]byte{1, 0}),
}, {
name: jsontest.Name("Bytes/ByteArray2/Overflow"),
inBuf: `"AQID"`,
inVal: new([2]byte),
want: addr([2]byte{1, 2}),
wantErr: EU(errors.New("decoded length of 3 mismatches array length of 2")).withType('"', T[[2]byte]()),
}, {
name: jsontest.Name("Bytes/ByteArray2/Overflow/Allowed"),
opts: []Options{jsonflags.UnmarshalArrayFromAnyLength | 1},
inBuf: `"AQID"`,
inVal: new([2]byte),
want: addr([2]byte{1, 2}),
}, {
name: jsontest.Name("Bytes/ByteArray3/Valid"),
inBuf: `"AQID"`,
Expand Down Expand Up @@ -6386,6 +6398,12 @@ func TestUnmarshal(t *testing.T) {
inBuf: `{"Base64": "aa=\r="}`,
inVal: new(structFormatBytes),
wantErr: EU(errors.New("illegal character '\\r' at offset 3")).withPos(`{"Base64": `, "/Base64").withType('"', T[[]byte]()),
}, {
name: jsontest.Name("Structs/Format/Bytes/Base64/NonAlphabet/Ignored"),
opts: []Options{jsonflags.FormatBytesWithLegacySemantics | 1},
inBuf: `{"Base64": "aa=\r\n="}`,
inVal: new(structFormatBytes),
want: &structFormatBytes{Base64: []byte{105}},
}, {
name: jsontest.Name("Structs/Format/Bytes/Invalid/Base64/NonAlphabet/Space"),
inBuf: `{"Base64": "aa= ="}`,
Expand Down
6 changes: 6 additions & 0 deletions v1/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,12 @@ func EscapeInvalidUTF8(v bool) Options {
// In contrast, the v2 default is to report an error unmarshaling
// a JSON array when expecting some form of binary data encoding.
//
// - When unmarshaling, '\r' and '\n' characters are ignored
// within the encoded "base32" and "base64" data.
// In contrast, the v2 default is to report an error in order to be
// strictly compliant with RFC 4648, section 3.3,
// which specifies that non-alphabet characters must be rejected.
//
// This affects either marshaling or unmarshaling.
// The v1 default is true.
func FormatBytesWithLegacySemantics(v bool) Options {
Expand Down

0 comments on commit 0240acd

Please sign in to comment.