From 0c20a0f03f502d1fc447cfa341e3f1d65f3c7ee7 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Sun, 26 Apr 2026 09:09:18 -0500 Subject: [PATCH 1/2] Refactor encoding of indefinite-length data item --- stream.go | 112 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 66 insertions(+), 46 deletions(-) diff --git a/stream.go b/stream.go index 731506bb..966c953c 100644 --- a/stream.go +++ b/stream.go @@ -180,9 +180,10 @@ func (e *IndefiniteLengthMapOddItemCountError) Error() string { // Encoder writes CBOR values to io.Writer. type Encoder struct { - w io.Writer - em *encMode - indefs []indefDataItem + w io.Writer + em *encMode + indefs []indefDataItem + scratch [1]byte // reused for single-byte writes (indefinite-length head and break code) } // NewEncoder returns a new encoder that writes to w using the default encoding options. @@ -193,24 +194,9 @@ func NewEncoder(w io.Writer) *Encoder { // Encode writes the CBOR encoding of v. func (enc *Encoder) Encode(v any) error { if len(enc.indefs) > 0 { - switch enc.indefs[len(enc.indefs)-1].typ { - case cborTypeTextString: - if v == nil { - return errors.New("cbor: cannot encode nil for indefinite-length text string") - } - k := reflect.TypeOf(v).Kind() - if k != reflect.String { - return errors.New("cbor: cannot encode item type " + k.String() + " for indefinite-length text string") - } - case cborTypeByteString: - if v == nil { - return errors.New("cbor: cannot encode nil for indefinite-length byte string") - } - t := reflect.TypeOf(v) - k := t.Kind() - if (k != reflect.Array && k != reflect.Slice) || t.Elem().Kind() != reflect.Uint8 { - return errors.New("cbor: cannot encode item type " + k.String() + " for indefinite-length byte string") - } + err := validateIndefiniteLengthChunk(enc.indefs[len(enc.indefs)-1].typ, v) + if err != nil { + return err } } @@ -223,10 +209,15 @@ func (enc *Encoder) Encode(v any) error { putEncodeBuffer(buf) - if err == nil && len(enc.indefs) > 0 { + if err != nil { + return err + } + + if len(enc.indefs) > 0 { enc.indefs[len(enc.indefs)-1].count++ } - return err + + return nil } // StartIndefiniteByteString starts indefinite-length byte string encoding. @@ -266,26 +257,37 @@ func (enc *Encoder) EndIndefinite() error { if len(enc.indefs) == 0 { return errors.New("cbor: cannot encode \"break\" code outside indefinite length values") } + + // Verify that indefinite-length map has even number of elements top := enc.indefs[len(enc.indefs)-1] if top.typ == cborTypeMap && top.count%2 != 0 { return &IndefiniteLengthMapOddItemCountError{count: top.count} } - _, err := enc.w.Write([]byte{cborBreakFlag}) - if err == nil { - enc.indefs = enc.indefs[:len(enc.indefs)-1] - // Increment parent container's item count because the child - // (indefinite-length data item) is fully written to the stream. - if len(enc.indefs) > 0 { - enc.indefs[len(enc.indefs)-1].count++ - } + + // Write break code + enc.scratch[0] = cborBreakFlag + _, err := enc.w.Write(enc.scratch[:]) + if err != nil { + return err } - return err + + enc.indefs = enc.indefs[:len(enc.indefs)-1] + + // Increment parent container's item count because the child + // (indefinite-length data item) is fully written to the stream. + if len(enc.indefs) > 0 { + enc.indefs[len(enc.indefs)-1].count++ + } + + return nil } func (enc *Encoder) startIndefinite(typ cborType) error { if enc.em.indefLength == IndefLengthForbidden { return &IndefiniteLengthError{typ} } + + // Verify that new indefinite-length data item is not a chunk in indefinite-length byte/text string. if len(enc.indefs) > 0 { parent := enc.indefs[len(enc.indefs)-1].typ if parent == cborTypeByteString || parent == cborTypeTextString { @@ -293,22 +295,40 @@ func (enc *Encoder) startIndefinite(typ cborType) error { " as chunk of indefinite-length " + parent.String()) } } - var head byte - switch typ { - case cborTypeByteString: - head = cborByteStringWithIndefiniteLengthHead - case cborTypeTextString: - head = cborTextStringWithIndefiniteLengthHead - case cborTypeArray: - head = cborArrayWithIndefiniteLengthHead - case cborTypeMap: - head = cborMapWithIndefiniteLengthHead + + // Write indefinite-length head. + enc.scratch[0] = byte(typ) | additionalInformationAsIndefiniteLengthFlag + _, err := enc.w.Write(enc.scratch[:]) + if err != nil { + return err } - _, err := enc.w.Write([]byte{head}) - if err == nil { - enc.indefs = append(enc.indefs, indefDataItem{typ: typ}) + + enc.indefs = append(enc.indefs, indefDataItem{typ: typ}) + return nil +} + +func validateIndefiniteLengthChunk(indefiniteLengthCborType cborType, chunk any) error { + switch indefiniteLengthCborType { + case cborTypeTextString: + if chunk == nil { + return errors.New("cbor: cannot encode nil for indefinite-length text string") + } + k := reflect.TypeOf(chunk).Kind() + if k != reflect.String { + return errors.New("cbor: cannot encode item type " + k.String() + " for indefinite-length text string") + } + + case cborTypeByteString: + if chunk == nil { + return errors.New("cbor: cannot encode nil for indefinite-length byte string") + } + t := reflect.TypeOf(chunk) + k := t.Kind() + if (k != reflect.Array && k != reflect.Slice) || t.Elem().Kind() != reflect.Uint8 { + return errors.New("cbor: cannot encode item type " + k.String() + " for indefinite-length byte string") + } } - return err + return nil } // RawMessage is a raw encoded CBOR value. From 0cdb674b5e4ad98b16d97c1536f4d8359052b606 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Sun, 26 Apr 2026 11:56:59 -0500 Subject: [PATCH 2/2] Fix indefinite-length string chunk validation Previously, the encoder rejected invalid chunks in indefinite-length strings based on the Go type. This could mistakenly reject objects that don't match the expected Go type, even if they encode valid chunk data (e.g., with a user-defined CBORMarshaler), and vice versa. This commit validates a chunk by inspecting the encoded chunk's CBOR type to ensure that the encoded chunk data is definite-length items of the same major type as the parent. --- stream.go | 64 +++++++++----- stream_test.go | 229 +++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 266 insertions(+), 27 deletions(-) diff --git a/stream.go b/stream.go index 966c953c..282b3f7d 100644 --- a/stream.go +++ b/stream.go @@ -194,7 +194,7 @@ func NewEncoder(w io.Writer) *Encoder { // Encode writes the CBOR encoding of v. func (enc *Encoder) Encode(v any) error { if len(enc.indefs) > 0 { - err := validateIndefiniteLengthChunk(enc.indefs[len(enc.indefs)-1].typ, v) + err := validateIndefiniteLengthChunkByType(enc.indefs[len(enc.indefs)-1].typ, v) if err != nil { return err } @@ -203,6 +203,15 @@ func (enc *Encoder) Encode(v any) error { buf := getEncodeBuffer() err := encode(buf, enc.em, reflect.ValueOf(v)) + + // Validate the encoded chunk against the indefinite-length data item using a byte-based check. + // This reliably detects chunks from cbor.Marshaler, registered tags, StringToByteString, etc., + // which may produce a chunk inconsistent with the parent's major type. + // Applies only to indefinite-length byte/text string parents (RFC 8949 Section 3.2.3). + if err == nil && len(enc.indefs) > 0 { + err = validateIndefiniteLengthChunkByData(enc.indefs[len(enc.indefs)-1].typ, buf.Bytes(), v) + } + if err == nil { _, err = enc.w.Write(buf.Bytes()) } @@ -307,26 +316,41 @@ func (enc *Encoder) startIndefinite(typ cborType) error { return nil } -func validateIndefiniteLengthChunk(indefiniteLengthCborType cborType, chunk any) error { - switch indefiniteLengthCborType { - case cborTypeTextString: - if chunk == nil { - return errors.New("cbor: cannot encode nil for indefinite-length text string") - } - k := reflect.TypeOf(chunk).Kind() - if k != reflect.String { - return errors.New("cbor: cannot encode item type " + k.String() + " for indefinite-length text string") - } +// validateIndefiniteLengthChunkByType rejects chunks based solely on their Go type. +func validateIndefiniteLengthChunkByType(indefiniteLengthCborType cborType, v any) error { + if indefiniteLengthCborType != cborTypeByteString && + indefiniteLengthCborType != cborTypeTextString { + return nil + } + if v == nil { + return errors.New("cbor: cannot encode nil for indefinite-length " + indefiniteLengthCborType.String()) + } + return nil +} - case cborTypeByteString: - if chunk == nil { - return errors.New("cbor: cannot encode nil for indefinite-length byte string") - } - t := reflect.TypeOf(chunk) - k := t.Kind() - if (k != reflect.Array && k != reflect.Slice) || t.Elem().Kind() != reflect.Uint8 { - return errors.New("cbor: cannot encode item type " + k.String() + " for indefinite-length byte string") - } +// validateIndefiniteLengthChunkByData checks that chunk is a definite-length +// CBOR data item with a matching major type. +// No-op for indefinite-length array/map, where any data item is valid. +func validateIndefiniteLengthChunkByData(indefiniteLengthCborType cborType, chunk []byte, v any) error { + if indefiniteLengthCborType != cborTypeByteString && + indefiniteLengthCborType != cborTypeTextString { + return nil + } + + if len(chunk) == 0 { + return errors.New("cbor: cannot encode item type " + reflect.TypeOf(v).Kind().String() + + " for indefinite-length " + indefiniteLengthCborType.String()) + } + + t, ai := parseInitialByte(chunk[0]) + if t != indefiniteLengthCborType { + return errors.New("cbor: cannot encode item type " + reflect.TypeOf(v).Kind().String() + + " for indefinite-length " + indefiniteLengthCborType.String()) + } + + if ai == additionalInformationAsIndefiniteLengthFlag { + return errors.New("cbor: cannot encode indefinite-length " + indefiniteLengthCborType.String() + + " as chunk of indefinite-length " + indefiniteLengthCborType.String()) } return nil } diff --git a/stream_test.go b/stream_test.go index df106963..dc915b8d 100644 --- a/stream_test.go +++ b/stream_test.go @@ -844,7 +844,7 @@ func TestIndefiniteByteString(t *testing.T) { } } -func TestIndefiniteByteStringWithInvalidChunk(t *testing.T) { +func TestIndefiniteByteStringWithInvalidChunk(t *testing.T) { //nolint:gocyclo t.Run("array as chunk", func(t *testing.T) { var w bytes.Buffer encoder := NewEncoder(&w) @@ -911,6 +911,126 @@ func TestIndefiniteByteStringWithInvalidChunk(t *testing.T) { } }) } + + t.Run("byte slice with ByteSliceLaterFormat option", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode item type slice for indefinite-length byte string" + + em, err := EncOptions{ByteSliceLaterFormat: ByteSliceLaterFormatBase64}.EncMode() + if err != nil { + t.Fatalf("EncMode() returned error %v", err) + } + + var w bytes.Buffer + encoder := em.NewEncoder(&w) + if err := encoder.StartIndefiniteByteString(); err != nil { + t.Fatalf("StartIndefiniteByteString() returned error %v", err) + } + if err := encoder.Encode([]byte("foo")); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) + + t.Run("nil byte slice with NilContainerAsNull (default) option", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode item type slice for indefinite-length byte string" + + em, err := EncOptions{NilContainers: NilContainerAsNull}.EncMode() // NilContainerAsNull is the default option + if err != nil { + t.Fatalf("EncMode() returned error %v", err) + } + + var w bytes.Buffer + encoder := em.NewEncoder(&w) + if err := encoder.StartIndefiniteByteString(); err != nil { + t.Fatalf("StartIndefiniteByteString() returned error %v", err) + } + var nilBytes []byte + if err := encoder.Encode(nilBytes); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) + + t.Run("RawMessage with tag chunk", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode item type slice for indefinite-length byte string" + + var w bytes.Buffer + encoder := NewEncoder(&w) + if err := encoder.StartIndefiniteByteString(); err != nil { + t.Fatalf("StartIndefiniteByteString() returned error %v", err) + } + raw := RawMessage{0xd8, 0x22, 0x43, 0x01, 0x02, 0x03} // 34(h'010203') + if err := encoder.Encode(raw); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) + + t.Run("RawMessage with indefinite-length byte string chunk", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode indefinite-length byte string as chunk of indefinite-length byte string" + var w bytes.Buffer + encoder := NewEncoder(&w) + if err := encoder.StartIndefiniteByteString(); err != nil { + t.Fatalf("StartIndefiniteByteString() returned error %v", err) + } + raw := RawMessage{0x5f, 0x41, 0x31, 0xff} // (_ h'31') + if err := encoder.Encode(raw); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) + + t.Run("CBORMarshaler with byte slice kind emits int", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode item type slice for indefinite-length byte string" + + var w bytes.Buffer + encoder := NewEncoder(&w) + if err := encoder.StartIndefiniteByteString(); err != nil { + t.Fatalf("StartIndefiniteByteString() returned error %v", err) + } + cborData := []byte{0x18, 0x7b} // 123 + if err := encoder.Encode(bytesMarshaler(cborData)); err == nil { + t.Errorf("Encode() didn't return an error ") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) + + t.Run("CBORMarshaler with byte slice kind emits text string", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode item type slice for indefinite-length byte string" + + var w bytes.Buffer + encoder := NewEncoder(&w) + if err := encoder.StartIndefiniteByteString(); err != nil { + t.Fatalf("StartIndefiniteByteString() returned error %v", err) + } + cborData := []byte{0x63, 'f', 'o', 'o'} // "foo" + if err := encoder.Encode(bytesMarshaler(cborData)); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) + + t.Run("CBORMarshaler with byte slice kind emits indefinite-length byte string", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode indefinite-length byte string as chunk of indefinite-length byte string" + + var w bytes.Buffer + encoder := NewEncoder(&w) + if err := encoder.StartIndefiniteByteString(); err != nil { + t.Fatalf("StartIndefiniteByteString() returned error %v", err) + } + cborData := []byte{0x5f, 0x41, 0x61, 0xff} // (_ h'61') + if err := encoder.Encode(bytesMarshaler(cborData)); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) } func TestIndefiniteTextString(t *testing.T) { @@ -943,8 +1063,8 @@ func TestIndefiniteTextStringWithInvalidChunk(t *testing.T) { } if err := encoder.Encode([]byte{1, 2}); err == nil { t.Errorf("Encode() didn't return an error") - } else if err.Error() != "cbor: cannot encode item type slice for indefinite-length text string" { - t.Errorf("Encode() returned error %q, want %q", err.Error(), "cbor: cannot encode item type slice for indefinite-length text string") + } else if err.Error() != "cbor: cannot encode item type slice for indefinite-length UTF-8 text string" { + t.Errorf("Encode() returned error %q, want %q", err.Error(), "cbor: cannot encode item type slice for indefinite-length UTF-8 text string") } }) @@ -956,8 +1076,8 @@ func TestIndefiniteTextStringWithInvalidChunk(t *testing.T) { } if err := encoder.Encode(123); err == nil { t.Errorf("Encode() didn't return an error") - } else if err.Error() != "cbor: cannot encode item type int for indefinite-length text string" { - t.Errorf("Encode() returned error %q, want %q", err.Error(), "cbor: cannot encode item type int for indefinite-length text string") + } else if err.Error() != "cbor: cannot encode item type int for indefinite-length UTF-8 text string" { + t.Errorf("Encode() returned error %q, want %q", err.Error(), "cbor: cannot encode item type int for indefinite-length UTF-8 text string") } }) @@ -969,8 +1089,8 @@ func TestIndefiniteTextStringWithInvalidChunk(t *testing.T) { } if err := encoder.Encode(nil); err == nil { t.Errorf("Encode() didn't return an error") - } else if err.Error() != "cbor: cannot encode nil for indefinite-length text string" { - t.Errorf("Encode() returned error %q, want %q", err.Error(), "cbor: cannot encode nil for indefinite-length text string") + } else if err.Error() != "cbor: cannot encode nil for indefinite-length UTF-8 text string" { + t.Errorf("Encode() returned error %q, want %q", err.Error(), "cbor: cannot encode nil for indefinite-length UTF-8 text string") } }) @@ -1001,6 +1121,101 @@ func TestIndefiniteTextStringWithInvalidChunk(t *testing.T) { } }) } + + t.Run("string with StringToByteString option", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode item type string for indefinite-length UTF-8 text string" + + em, err := EncOptions{String: StringToByteString}.EncMode() + if err != nil { + t.Fatalf("EncMode() returned error %v", err) + } + var w bytes.Buffer + encoder := em.NewEncoder(&w) + if err := encoder.StartIndefiniteTextString(); err != nil { + t.Fatalf("StartIndefiniteTextString() returned error %v", err) + } + if err := encoder.Encode("foo"); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) + + t.Run("RawMessage with indefinite-length text string chunk", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode indefinite-length UTF-8 text string as chunk of indefinite-length UTF-8 text string" + + var w bytes.Buffer + encoder := NewEncoder(&w) + if err := encoder.StartIndefiniteTextString(); err != nil { + t.Fatalf("StartIndefiniteTextString() returned error %v", err) + } + raw := RawMessage{0x7f, 0x61, 0x61, 0xff} // (_ "a") + if err := encoder.Encode(raw); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) + + t.Run("CBORMarshaler with string kind emits uint", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode item type string for indefinite-length UTF-8 text string" + + var w bytes.Buffer + encoder := NewEncoder(&w) + if err := encoder.StartIndefiniteTextString(); err != nil { + t.Fatalf("StartIndefiniteTextString() returned error %v", err) + } + cborData := []byte{0x18, 0x7b} // 123 + if err := encoder.Encode(stringMarshaler(cborData)); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) + + t.Run("CBORMarshaler with string kind emits byte string", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode item type string for indefinite-length UTF-8 text string" + + var w bytes.Buffer + encoder := NewEncoder(&w) + if err := encoder.StartIndefiniteTextString(); err != nil { + t.Fatalf("StartIndefiniteTextString() returned error %v", err) + } + cborData := []byte{0x43, 0x01, 0x02, 0x03} // h'010203' + if err := encoder.Encode(stringMarshaler(cborData)); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) + + t.Run("CBORMarshaler with string kind emits indefinite-length text string", func(t *testing.T) { + expectedErrorMsg := "cbor: cannot encode indefinite-length UTF-8 text string as chunk of indefinite-length UTF-8 text string" + + var w bytes.Buffer + encoder := NewEncoder(&w) + if err := encoder.StartIndefiniteTextString(); err != nil { + t.Fatalf("StartIndefiniteTextString() returned error %v", err) + } + cborData := []byte{0x7f, 0x61, 0x61, 0xff} // (_ "a") + if err := encoder.Encode(stringMarshaler(cborData)); err == nil { + t.Errorf("Encode() didn't return an error") + } else if err.Error() != expectedErrorMsg { + t.Errorf("Encode() returned error %q, want %q", err.Error(), expectedErrorMsg) + } + }) +} + +type bytesMarshaler []byte + +func (m bytesMarshaler) MarshalCBOR() ([]byte, error) { + return []byte(m), nil +} + +type stringMarshaler string + +func (m stringMarshaler) MarshalCBOR() ([]byte, error) { + return []byte(m), nil } func TestIndefiniteArray(t *testing.T) {