Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

json.unmarshal unmarshals an invalid json in some cases #2331

Closed
katsew opened this issue Apr 23, 2020 · 1 comment · Fixed by #2346
Closed

json.unmarshal unmarshals an invalid json in some cases #2331

katsew opened this issue Apr 23, 2020 · 1 comment · Fixed by #2346
Labels

Comments

@katsew
Copy link
Contributor

katsew commented Apr 23, 2020

Hi, I wrote some tests in conftest, which is implemented on top of opa.
I hit an unexpected result in some cases with json.unmarshal.

Here is an example to reproduce the issue.
https://play.openpolicyagent.org/p/XS3RGihDpm

Expected Behavior

It should be panicked when invalid json is passed into json.unmarshal.

Actual Behavior

json.unmarshal succeeded.

Steps to Reproduce the Problem

  1. write invalid json
  2. write rule which uses json.unmarshal with input of step 1
  3. execute
@patrick-east
Copy link
Contributor

Looks like the problem might be with how the decoding is happening. Under the hood we are using encoding/json's decoder https://golang.org/pkg/encoding/json/#Decoder.Decode which decodes the next json encoded value.. so in the example from the playground { "k": 1 }{}} it seems like it will decode the first valid part {"k": 1} and returns it without checking anything else. A quick look at the source for Decode() seems to confirm this.

katsew added a commit to katsew/opa that referenced this issue Apr 28, 2020
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>
katsew added a commit to katsew/opa that referenced this issue May 1, 2020
patrick-east pushed a commit that referenced this issue May 1, 2020
Fixes: #2331

Signed-off-by: katsew <y.katsew@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants