Skip to content

Commit

Permalink
util: Fix UnmarshalJSON parses invalid json in some cases
Browse files Browse the repository at this point in the history
Note: Since this commit breaks some tests unexpectedly as
a side effect, I workaround by changing called method and
left a FIXME comment on it.

Fixes: open-policy-agent#2331

Signed-off-by: katsew <y.katsew@gmail.com>
  • Loading branch information
katsew committed Apr 28, 2020
1 parent f7747e7 commit 4fd932e
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 2 deletions.
5 changes: 4 additions & 1 deletion topdown/topdown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3015,7 +3015,10 @@ func assertTopDownWithPathAndContext(ctx context.Context, t *testing.T, compiler
t.Fatal(err)
}

expected := util.MustUnmarshalJSON([]byte(e))
// FIXME
// Since some of the tests pass invalid JSON, MustUnmarshalJSON will be panicked.
// To avoid being panicked, use UnmarshalJSON and ignore the error.
util.UnmarshalJSON([]byte(e), &expected)

if requiresSort {
sort.Sort(resultSet(result.([]interface{})))
Expand Down
16 changes: 15 additions & 1 deletion util/json.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package util
import (
"bytes"
"encoding/json"
"fmt"
"io"
"reflect"

Expand All @@ -21,7 +22,20 @@ import (
func UnmarshalJSON(bs []byte, x interface{}) (err error) {
buf := bytes.NewBuffer(bs)
decoder := NewJSONDecoder(buf)
return decoder.Decode(x)
if err := decoder.Decode(x); err != nil {
return err
}

// Since decoder.Decode validates only the first json structure in bytes,
// check if decoder has more bytes to consume to validate whole input bytes.
tok, err := decoder.Token()
if tok != nil {
return fmt.Errorf("invalid JSON input caused by '%s', expected EOF", tok)
}
if err != nil && err != io.EOF {
return err
}
return nil
}

// NewJSONDecoder returns a new decoder that reads from r.
Expand Down
14 changes: 14 additions & 0 deletions util/json_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,20 @@ import (
"github.com/open-policy-agent/opa/util"
)

func TestInvalidJSONInput(t *testing.T) {
cases := [][]byte{
[]byte("{ \"k\": 1 }\n{}}"),
[]byte("{ \"k\": 1 }\n!!!}"),
}
for _, tc := range cases {
var x interface{}
err := util.UnmarshalJSON(tc, &x)
if err == nil {
t.Errorf("should be an error")
}
}
}

func TestRoundTrip(t *testing.T) {
cases := []interface{}{
nil,
Expand Down

0 comments on commit 4fd932e

Please sign in to comment.