Skip to content

Commit

Permalink
Add JSON validity check to cosmosInlineJSON
Browse files Browse the repository at this point in the history
  • Loading branch information
webmaster128 committed Apr 8, 2024
1 parent 5bea209 commit 9dca1d9
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 deletions.
12 changes: 6 additions & 6 deletions x/tx/signing/aminojson/encoder.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,20 +82,20 @@ func nullSliceAsEmptyEncoder(enc *Encoder, v protoreflect.Value, w io.Writer) er
}

// cosmosInlineJSON takes bytes and inlines them into a JSON document.
// This assumes the bytes contain valid JSON since otherwise the resulting document is invalid.
//
// This requires the bytes contain valid JSON since otherwise the resulting document would be invalid.
// Invalid JSON will result in an error.
//
// This replicates the behavior of JSON messages embedded in protobuf bytes
// required for CosmWasm, e.g.:
// https://github.com/CosmWasm/wasmd/blob/08567ff20e372e4f4204a91ca64a371538742bed/x/wasm/types/tx.go#L20-L22
func cosmosInlineJSON(_ *Encoder, v protoreflect.Value, w io.Writer) error {
switch bz := v.Interface().(type) {
case []byte:
// Caution, this does not include a validity check as json.RawMessage can be anything 🤷‍♂️
blob, err := json.RawMessage(bz).MarshalJSON()
if err != nil {
return err
if !json.Valid(bz) {
return errors.New("invalid JSON bytes")
}
_, err = w.Write(blob)
_, err := w.Write(bz)
return err
default:
return fmt.Errorf("unsupported type %T", bz)
Expand Down
18 changes: 10 additions & 8 deletions x/tx/signing/aminojson/encoder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,15 +56,17 @@ func TestCosmosInlineJSON(t *testing.T) {
wantErr: false,
wantOutput: `"hey yo"`,
},
// This test case is a bit tricky. Conceptually it makes no sense for this
// to pass. The question is just where the JSON validity check is done.
// In wasmd we have it in the message field validation. But it might make
// sense to move it here instead.
// For now it's better to consider this case undefined behaviour.
"supported type - invalid JSON": {
value: protoreflect.ValueOfBytes([]byte(`foo`)),
wantErr: false,
wantOutput: `foo`,
value: protoreflect.ValueOfBytes([]byte(`foo`)),
wantErr: true,
},
"supported type - invalid JSON (empty)": {
value: protoreflect.ValueOfBytes([]byte(``)),
wantErr: true,
},
"supported type - invalid JSON (nil bytes)": {
value: protoreflect.ValueOfBytes(nil),
wantErr: true,
},
"unsupported type - bool": {
value: protoreflect.ValueOfBool(true),
Expand Down

0 comments on commit 9dca1d9

Please sign in to comment.