Skip to content

Commit

Permalink
Fix CBOR encoding of embedded struct fields
Browse files Browse the repository at this point in the history
Updated CBOR tests failed for ARM32 due to the parents slice being
appended to with different indices in collectFieldWeights. On ARM32 the
backing array wasn't changing, so the value was being overwritten,
resulting in field orders like `[0 [1 2] [1 2] [1 2]]` instead of
`[0 [1 0] [1 1] [1 2]]`.

Testing was performed using QEMU:

```console
> GOARCH=arm GOARM=5 go test -c -o test.arm32 ./cbor
> qemu-arm ./test.arm32 -test.run codeArray
--- FAIL: TestEncodeArray (0.01s)
    --- FAIL: TestEncodeArray/tuples (0.00s)
        cbor_test.go:378: marshaling struct { cbor_test.Embed; B []int "cbor:\"2\"" }{Embed:cbor_test.Embed{A:1, B:[]int{2, 3}, C:"IETF", D:[]uint8{0x48, 0x65, 0x6c, 0x6c, 0x6f}}, B:[]int{3, 4}}; expected 84 01 82 03 04 64 49 45 54 46 45 48 65 6c 6c 6f, got 84 45 48 65 6c 6c 6f 82 03 04 45 48 65 6c 6c 6f
--- FAIL: TestDecodeArray (0.01s)
    --- FAIL: TestDecodeArray/tuples (0.00s)
        cbor_test.go:1174: error unmarshaling 84 01 82 03 04 64 49 45 54 46 45 48 65 6c 6c 6f: error decoding array item [0 3]: unsupported type: []uint8: only primitive (u)int(N) types supported
FAIL
```

The fix was to clone the parents slice before appending to it and
passing to the recursive func.

Signed-off-by: Shiva Kumar Gurija <[email protected]>

Co-authored-by: Ben Krieger <[email protected]>
  • Loading branch information
shivakumargurija and ben-krieger committed Oct 18, 2024
1 parent 5b993c5 commit 9b17722
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 7 deletions.
5 changes: 4 additions & 1 deletion cbor/cbor.go
Original file line number Diff line number Diff line change
Expand Up @@ -1488,7 +1488,10 @@ func collectFieldWeights(parents []int, i, upper int, field func(int) reflect.St
}
}

// Return duplicates indices if flat (un)marshaling
// Duplicate parents slice, because it might be appended to
parents = slices.Clone(parents)

// Return duplicate indices if flat (un)marshaling
if n, ok := flatN(f); ok {
for j := 0; j < n; j++ {
fields = append(fields, weightedField{
Expand Down
55 changes: 49 additions & 6 deletions cbor/cbor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,10 @@ func TestEncodeArray(t *testing.T) {

t.Run("tuples", func(t *testing.T) {
type Embed struct {
A int
B int `cbor:"-"`
A int `cbor:"1"`
B []int `cbor:"-"`
C string `cbor:"3"`
D []byte `cbor:"4"`
}
for _, test := range []struct {
input any
Expand Down Expand Up @@ -349,10 +351,26 @@ func TestEncodeArray(t *testing.T) {
A int `cbor:"2"`
B string `cbor:"1"`
}{A: 1, B: "IETF"}, expect: []byte{0x82, 0x64, 0x49, 0x45, 0x54, 0x46, 0x01}},
{input: struct {
Embed
B string
}{Embed: Embed{A: 1, B: 3}, B: "IETF"}, expect: []byte{0x82, 0x01, 0x64, 0x49, 0x45, 0x54, 0x46}},
{
input: struct {
Embed
B []int `cbor:"2"`
}{
Embed: Embed{
A: 1,
B: []int{2, 3},
C: "IETF",
D: []byte("Hello"),
},
B: []int{3, 4},
},
expect: []byte{0x84,
0x01,
0x82, 0x03, 0x04,
0x64, 0x49, 0x45, 0x54, 0x46,
0x45, 0x48, 0x65, 0x6c, 0x6c, 0x6f,
},
},
} {
if got, err := cbor.Marshal(test.input); err != nil {
t.Errorf("error marshaling % x: %v", test.input, err)
Expand Down Expand Up @@ -1111,6 +1129,12 @@ func TestDecodeArray(t *testing.T) {
})

t.Run("tuples", func(t *testing.T) {
type Embed struct {
A int `cbor:"1"`
B []int `cbor:"-"`
C string `cbor:"3"`
D []byte `cbor:"4"`
}
for _, test := range []struct {
input []byte
expect any
Expand All @@ -1125,6 +1149,25 @@ func TestDecodeArray(t *testing.T) {
A int
B string
}{A: 1, B: "IETF"}, input: []byte{0x82, 0x01, 0x64, 0x49, 0x45, 0x54, 0x46}},
{
expect: struct {
Embed
B []int `cbor:"2"`
}{
Embed: Embed{
A: 1,
C: "IETF",
D: []byte("Hello"),
},
B: []int{3, 4},
},
input: []byte{0x84,
0x01,
0x82, 0x03, 0x04,
0x64, 0x49, 0x45, 0x54, 0x46,
0x45, 0x48, 0x65, 0x6c, 0x6c, 0x6f,
},
},
} {
gotAddr := reflect.New(reflect.TypeOf(test.expect)).Interface()
if err := cbor.Unmarshal(test.input, gotAddr); err != nil {
Expand Down

0 comments on commit 9b17722

Please sign in to comment.