Skip to content

Commit

Permalink
CBG-3634: Fully escape JSON strings when using InjectJSONProperties (#…
Browse files Browse the repository at this point in the history
…6588)

* Fully escape JSON strings when using InjectJSONProperties

* Fall through to `default` case for strings, but leave warning comment to make it clear it was an intentional decision to fall through for strings.
  • Loading branch information
bbrks authored Nov 29, 2023
1 parent 47cbbc8 commit 69adee0
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 6 deletions.
5 changes: 3 additions & 2 deletions base/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -1307,10 +1307,11 @@ func InjectJSONProperties(b []byte, kvPairs ...KVPair) (new []byte, err error) {
valBytes = []byte(strconv.FormatUint(uint64(v), 10))
case uint64:
valBytes = []byte(strconv.FormatUint(v, 10))
case string:
valBytes = []byte(`"` + v + `"`)
case bool:
valBytes = []byte(strconv.FormatBool(v))
// case string:
// it's not safe to use strings without marshalling
// fall through to default below
default:
valBytes, err = JSONMarshal(kv.Val)
}
Expand Down
30 changes: 26 additions & 4 deletions base/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -801,10 +801,7 @@ func TestInjectJSONProperties(t *testing.T) {
}

assert.Equal(tt, test.expectedOutput, string(output))

var m map[string]interface{}
err = JSONUnmarshal(output, &m)
assert.NoError(tt, err, "produced invalid JSON")
assert.NoError(tt, JSONUnmarshal(output, &map[string]interface{}{}), "produced invalid JSON")
})
}
}
Expand Down Expand Up @@ -904,13 +901,38 @@ func TestInjectJSONPropertiesDiffTypes(t *testing.T) {
true,
},
},
{
input: `{"foo": "bar"}`,
output: `{"foo": "bar","quoted_string":"\"quoted\""}`,
pair: KVPair{
"quoted_string",
`"quoted"`,
},
},
{
input: `{"foo": "bar"}`,
output: `{"foo": "bar","reverse_solidus":"C:\\something\\that\\contains\\backslashes"}`,
pair: KVPair{
"reverse_solidus",
`C:\something\that\contains\backslashes`,
},
},
{
input: `{"foo": "bar"}`,
output: `{"foo": "bar","escape":"foo\u0000bar"}`,
pair: KVPair{
"escape",
"foo\x00bar",
},
},
}

for _, test := range tests {
t.Run(test.output, func(t *testing.T) {
output, err := InjectJSONProperties([]byte(test.input), test.pair)
assert.NoError(t, err)
assert.Equal(t, test.output, string(output))
assert.NoErrorf(t, JSONUnmarshal(output, &map[string]interface{}{}), "produced invalid JSON")
})
}
}
Expand Down

0 comments on commit 69adee0

Please sign in to comment.