Skip to content

Commit 635e2a8

Browse files
authored
Merge pull request smartcontractkit#3721 from smartcontractkit/bugfix/174705938_fix_cbor_unexpected_break_code
Handle bignum encoding in CBOR
2 parents 7aef4e5 + 9107499 commit 635e2a8

File tree

5 files changed

+172
-156
lines changed

5 files changed

+172
-156
lines changed

core/store/models/cbor.go

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ package models
33
import (
44
"bytes"
55
"encoding/json"
6-
7-
"github.com/smartcontractkit/chainlink/core/utils"
6+
"fmt"
7+
"math/big"
88

99
"github.com/fxamacker/cbor/v2"
1010
)
@@ -22,7 +22,7 @@ func ParseCBOR(b []byte) (JSON, error) {
2222
return JSON{}, err
2323
}
2424

25-
coerced, err := utils.CoerceInterfaceMapToStringMap(m)
25+
coerced, err := CoerceInterfaceMapToStringMap(m)
2626
if err != nil {
2727
return JSON{}, err
2828
}
@@ -52,3 +52,57 @@ func autoAddMapDelimiters(b []byte) []byte {
5252

5353
return b
5454
}
55+
56+
// CoerceInterfaceMapToStringMap converts map[interface{}]interface{} (interface maps) to
57+
// map[string]interface{} (string maps) and []interface{} with interface maps to string maps.
58+
// Relevant when serializing between CBOR and JSON.
59+
//
60+
// It also handles the CBOR 'bignum' type as documented here: https://tools.ietf.org/html/rfc7049#section-2.4.2
61+
func CoerceInterfaceMapToStringMap(in interface{}) (interface{}, error) {
62+
switch typed := in.(type) {
63+
case map[string]interface{}:
64+
for k, v := range typed {
65+
coerced, err := CoerceInterfaceMapToStringMap(v)
66+
if err != nil {
67+
return nil, err
68+
}
69+
typed[k] = coerced
70+
}
71+
return typed, nil
72+
case map[interface{}]interface{}:
73+
m := map[string]interface{}{}
74+
for k, v := range typed {
75+
coercedKey, ok := k.(string)
76+
if !ok {
77+
return nil, fmt.Errorf("unable to coerce key %T %v to a string", k, k)
78+
}
79+
coerced, err := CoerceInterfaceMapToStringMap(v)
80+
if err != nil {
81+
return nil, err
82+
}
83+
m[coercedKey] = coerced
84+
}
85+
return m, nil
86+
case []interface{}:
87+
r := make([]interface{}, len(typed))
88+
for i, v := range typed {
89+
coerced, err := CoerceInterfaceMapToStringMap(v)
90+
if err != nil {
91+
return nil, err
92+
}
93+
r[i] = coerced
94+
}
95+
return r, nil
96+
case cbor.Tag:
97+
if value, ok := typed.Content.([]byte); ok {
98+
if typed.Number == 2 {
99+
return big.NewInt(0).SetBytes(value), nil
100+
} else if typed.Number == 3 {
101+
return big.NewInt(0).Sub(big.NewInt(-1), big.NewInt(0).SetBytes(value)), nil
102+
}
103+
}
104+
return in, nil
105+
default:
106+
return in, nil
107+
}
108+
}

core/store/models/cbor_test.go

Lines changed: 115 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@ package models
22

33
import (
44
"encoding/json"
5-
"log"
5+
"reflect"
66
"testing"
77

88
"github.com/ethereum/go-ethereum/common/hexutil"
9+
"github.com/fxamacker/cbor/v2"
910
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/require"
1012
)
1113

1214
func Test_ParseCBOR(t *testing.T) {
@@ -21,28 +23,34 @@ func Test_ParseCBOR(t *testing.T) {
2123
{
2224
"hello world",
2325
`0xbf6375726c781a68747470733a2f2f657468657270726963652e636f6d2f61706964706174689f66726563656e7463757364ffff`,
24-
jsonMustUnmarshal(`{"path":["recent","usd"],"url":"https://etherprice.com/api"}`),
26+
jsonMustUnmarshal(t, `{"path":["recent","usd"],"url":"https://etherprice.com/api"}`),
2527
false,
2628
},
2729
{
2830
"trailing empty bytes",
2931
`0xbf6375726c781a68747470733a2f2f657468657270726963652e636f6d2f61706964706174689f66726563656e7463757364ffff000000`,
30-
jsonMustUnmarshal(`{"path":["recent","usd"],"url":"https://etherprice.com/api"}`),
32+
jsonMustUnmarshal(t, `{"path":["recent","usd"],"url":"https://etherprice.com/api"}`),
3133
false,
3234
},
3335
{
3436
"nested maps",
3537
`0xbf657461736b739f6868747470706f7374ff66706172616d73bf636d73676f68656c6c6f5f636861696e6c696e6b6375726c75687474703a2f2f6c6f63616c686f73743a36363930ffff`,
36-
jsonMustUnmarshal(`{"params":{"msg":"hello_chainlink","url":"http://localhost:6690"},"tasks":["httppost"]}`),
38+
jsonMustUnmarshal(t, `{"params":{"msg":"hello_chainlink","url":"http://localhost:6690"},"tasks":["httppost"]}`),
3739
false,
3840
},
3941
{
4042
"missing initial start map marker",
4143
`0x636B65796576616C7565ff`,
42-
jsonMustUnmarshal(`{"key":"value"}`),
44+
jsonMustUnmarshal(t, `{"key":"value"}`),
4345
false,
4446
},
45-
{"empty object", `0xa0`, jsonMustUnmarshal(`{}`), false},
47+
{
48+
"bignums",
49+
`0xbf676269676e756d739fc249010000000000000000c258204000000000000000000000000000000000000000000000000000000000000000c348ffffffffffffffffc358203fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff`,
50+
jsonMustUnmarshal(t, `{"bignums":[18446744073709551616,28948022309329048855892746252171976963317496166410141009864396001978282409984,-18446744073709551616,-28948022309329048855892746252171976963317496166410141009864396001978282409984]}`),
51+
false,
52+
},
53+
{"empty object", `0xa0`, jsonMustUnmarshal(t, `{}`), false},
4654
{"empty string", `0x`, JSON{}, false},
4755
{"invalid CBOR", `0xff`, JSON{}, true},
4856
}
@@ -120,11 +128,108 @@ func Test_autoAddMapDelimiters(t *testing.T) {
120128
}
121129
}
122130

123-
func jsonMustUnmarshal(in string) JSON {
131+
func jsonMustUnmarshal(t *testing.T, in string) JSON {
124132
var j JSON
125133
err := json.Unmarshal([]byte(in), &j)
126-
if err != nil {
127-
log.Panicf("Failed to unmarshal '%s'", in)
128-
}
134+
require.NoError(t, err)
129135
return j
130136
}
137+
138+
func TestCoerceInterfaceMapToStringMap(t *testing.T) {
139+
t.Parallel()
140+
141+
tests := []struct {
142+
name string
143+
input interface{}
144+
want interface{}
145+
}{
146+
{"empty map", map[interface{}]interface{}{}, map[string]interface{}{}},
147+
{"simple map", map[interface{}]interface{}{"key": "value"}, map[string]interface{}{"key": "value"}},
148+
{"int map", map[int]interface{}{1: "value"}, map[int]interface{}{1: "value"}},
149+
{
150+
"nested string map map",
151+
map[string]interface{}{"key": map[interface{}]interface{}{"nk": "nv"}},
152+
map[string]interface{}{"key": map[string]interface{}{"nk": "nv"}},
153+
},
154+
{
155+
"nested map map",
156+
map[interface{}]interface{}{"key": map[interface{}]interface{}{"nk": "nv"}},
157+
map[string]interface{}{"key": map[string]interface{}{"nk": "nv"}},
158+
},
159+
{
160+
"nested map array",
161+
map[interface{}]interface{}{"key": []interface{}{1, "value"}},
162+
map[string]interface{}{"key": []interface{}{1, "value"}},
163+
},
164+
{"empty array", []interface{}{}, []interface{}{}},
165+
{"simple array", []interface{}{1, "value"}, []interface{}{1, "value"}},
166+
{
167+
"nested array map",
168+
[]interface{}{map[interface{}]interface{}{"key": map[interface{}]interface{}{"nk": "nv"}}},
169+
[]interface{}{map[string]interface{}{"key": map[string]interface{}{"nk": "nv"}}},
170+
},
171+
}
172+
173+
for _, test := range tests {
174+
t.Run(test.name, func(t *testing.T) {
175+
decoded, err := CoerceInterfaceMapToStringMap(test.input)
176+
require.NoError(t, err)
177+
assert.True(t, reflect.DeepEqual(test.want, decoded))
178+
})
179+
}
180+
}
181+
182+
func TestCoerceInterfaceMapToStringMap_BadInputs(t *testing.T) {
183+
t.Parallel()
184+
185+
tests := []struct {
186+
name string
187+
input interface{}
188+
}{
189+
{"error map", map[interface{}]interface{}{1: "value"}},
190+
{"error array", []interface{}{map[interface{}]interface{}{1: "value"}}},
191+
}
192+
193+
for _, test := range tests {
194+
t.Run(test.name, func(t *testing.T) {
195+
_, err := CoerceInterfaceMapToStringMap(test.input)
196+
assert.Error(t, err)
197+
})
198+
}
199+
}
200+
201+
func TestJSON_CBOR(t *testing.T) {
202+
t.Parallel()
203+
204+
tests := []struct {
205+
name string
206+
in JSON
207+
}{
208+
{"empty object", JSON{}},
209+
{"array", jsonMustUnmarshal(t, `[1,2,3,4]`)},
210+
{
211+
"basic object",
212+
jsonMustUnmarshal(t, `{"path":["recent","usd"],"url":"https://etherprice.com/api"}`),
213+
},
214+
{
215+
"complex object",
216+
jsonMustUnmarshal(t, `{"a":{"1":[{"b":"free"},{"c":"more"},{"d":["less", {"nesting":{"4":"life"}}]}]}}`),
217+
},
218+
}
219+
220+
for _, test := range tests {
221+
t.Run(test.name, func(t *testing.T) {
222+
encoded, err := test.in.CBOR()
223+
assert.NoError(t, err)
224+
225+
var decoded interface{}
226+
err = cbor.Unmarshal(encoded, &decoded)
227+
228+
assert.NoError(t, err)
229+
230+
decoded, err = CoerceInterfaceMapToStringMap(decoded)
231+
assert.NoError(t, err)
232+
assert.True(t, reflect.DeepEqual(test.in.Result.Value(), decoded))
233+
})
234+
}
235+
}

core/store/models/common_test.go

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,12 @@ package models_test
33
import (
44
"encoding/json"
55
"net/url"
6-
"reflect"
76
"testing"
87
"time"
98

109
"github.com/smartcontractkit/chainlink/core/internal/cltest"
1110
"github.com/smartcontractkit/chainlink/core/store/models"
12-
"github.com/smartcontractkit/chainlink/core/utils"
1311

14-
"github.com/fxamacker/cbor/v2"
1512
"github.com/stretchr/testify/assert"
1613
"github.com/stretchr/testify/require"
1714
)
@@ -195,42 +192,6 @@ func TestJSON_Delete(t *testing.T) {
195192
}
196193
}
197194

198-
func TestJSON_CBOR(t *testing.T) {
199-
t.Parallel()
200-
201-
tests := []struct {
202-
name string
203-
in models.JSON
204-
}{
205-
{"empty object", models.JSON{}},
206-
{"array", cltest.JSONFromString(t, `[1,2,3,4]`)},
207-
{
208-
"hello world",
209-
cltest.JSONFromString(t, `{"path":["recent","usd"],"url":"https://etherprice.com/api"}`),
210-
},
211-
{
212-
"complex object",
213-
cltest.JSONFromString(t, `{"a":{"1":[{"b":"free"},{"c":"more"},{"d":["less", {"nesting":{"4":"life"}}]}]}}`),
214-
},
215-
}
216-
217-
for _, test := range tests {
218-
t.Run(test.name, func(t *testing.T) {
219-
encoded, err := test.in.CBOR()
220-
assert.NoError(t, err)
221-
222-
var decoded interface{}
223-
err = cbor.Unmarshal(encoded, &decoded)
224-
225-
assert.NoError(t, err)
226-
227-
decoded, err = utils.CoerceInterfaceMapToStringMap(decoded)
228-
assert.NoError(t, err)
229-
assert.True(t, reflect.DeepEqual(test.in.Result.Value(), decoded))
230-
})
231-
}
232-
}
233-
234195
func TestWebURL_UnmarshalJSON_Error(t *testing.T) {
235196
t.Parallel()
236197
j := []byte(`"NotAUrl"`)

core/utils/utils.go

Lines changed: 0 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -290,49 +290,6 @@ func MinUint(first uint, vals ...uint) uint {
290290
return min
291291
}
292292

293-
// CoerceInterfaceMapToStringMap converts map[interface{}]interface{} (interface maps) to
294-
// map[string]interface{} (string maps) and []interface{} with interface maps to string maps.
295-
// Relevant when serializing between CBOR and JSON.
296-
func CoerceInterfaceMapToStringMap(in interface{}) (interface{}, error) {
297-
switch typed := in.(type) {
298-
case map[string]interface{}:
299-
for k, v := range typed {
300-
coerced, err := CoerceInterfaceMapToStringMap(v)
301-
if err != nil {
302-
return nil, err
303-
}
304-
typed[k] = coerced
305-
}
306-
return typed, nil
307-
case map[interface{}]interface{}:
308-
m := map[string]interface{}{}
309-
for k, v := range typed {
310-
coercedKey, ok := k.(string)
311-
if !ok {
312-
return nil, fmt.Errorf("unable to coerce key %T %v to a string", k, k)
313-
}
314-
coerced, err := CoerceInterfaceMapToStringMap(v)
315-
if err != nil {
316-
return nil, err
317-
}
318-
m[coercedKey] = coerced
319-
}
320-
return m, nil
321-
case []interface{}:
322-
r := make([]interface{}, len(typed))
323-
for i, v := range typed {
324-
coerced, err := CoerceInterfaceMapToStringMap(v)
325-
if err != nil {
326-
return nil, err
327-
}
328-
r[i] = coerced
329-
}
330-
return r, nil
331-
default:
332-
return in, nil
333-
}
334-
}
335-
336293
// UnmarshalToMap takes an input json string and returns a map[string]interface i.e. a raw object
337294
func UnmarshalToMap(input string) (map[string]interface{}, error) {
338295
var output map[string]interface{}

0 commit comments

Comments
 (0)