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

print fails for variables assigned in destructured array #4078

Closed
johanfylling opened this issue Dec 1, 2021 · 0 comments · Fixed by #4086 or #4099
Closed

print fails for variables assigned in destructured array #4078

johanfylling opened this issue Dec 1, 2021 · 0 comments · Fixed by #4086 or #4099
Assignees
Labels

Comments

@johanfylling
Copy link
Contributor

OPA print function fails when printing variable assigned in destructured array.

  • OPA version: 0.34.2/HEAD@main
  • Example policy:
package example

jwt := "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"

test = payload {
    [_, payload, _] := io.jwt.decode(jwt)
    print(payload)
}
  • Example CLI command: opa eval -d <broken.rego> "data.example.test"
  • Example output that OPA returned:
{
  "errors": [
    {
      "message": "var __local1__ is undeclared",
      "code": "rego_compile_error",
      "location": {
        "file": "broken.rego",
        "row": 7,
        "col": 11
      }
    }
  ]
}
@srenatus srenatus self-assigned this Dec 2, 2021
srenatus added a commit to srenatus/opa that referenced this issue Dec 2, 2021
For both partial and complete rules, when variables from destructured
arrays in assignments had been used in the rule head, the print rewriting
would fail.

Now, this situation is accounted for by starting the safe VarSet with

    r.Head.Vars()

instead of

    r.Head.Args.Vars()

The former will check each of the Args, Key and Value fields of Head: if
it's non-empty, it'll add .Vars() to the returned VarSet.

Fixes open-policy-agent#4078.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
tsandall pushed a commit that referenced this issue Dec 3, 2021
For both partial and complete rules, when variables from destructured
arrays in assignments had been used in the rule head, the print rewriting
would fail.

Now, this situation is accounted for by starting the safe VarSet with

    r.Head.Vars()

instead of

    r.Head.Args.Vars()

The former will check each of the Args, Key and Value fields of Head: if
it's non-empty, it'll add .Vars() to the returned VarSet.

Fixes #4078.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
@tsandall tsandall reopened this Dec 3, 2021
srenatus added a commit to srenatus/opa that referenced this issue Dec 8, 2021
outputVarsForExprEq, which is used in the rewriting of `print()` calls,
wasn't able to find `x` in

    [_, x, _] = f(1)

and thus certain print expressions failed during the rewriting stage of
print() calls in the compiler.

Now, the "collection (array/object) = func call" is handled.

Furthermore, this adds calls to isRefSafe in a few places: as it turned
out, the test cases

    {"array/ref", "[1,2,x] = a[_]", "[a]", "[x]"},
    {"object/ref", `{"x": x} = a[_]`, "[a]", "[x]"},

would have, without the check, passed without "a" in "safe" (3rd field).
The fact that these cases are including "a" is evidence for this being
the correct behaviour.

Fixes open-policy-agent#4078.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
srenatus added a commit that referenced this issue Dec 8, 2021
outputVarsForExprEq, which is used in the rewriting of `print()` calls,
wasn't able to find `x` in

    [_, x, _] = f(1)

and thus certain print expressions failed during the rewriting stage of
print() calls in the compiler.

Now, the "collection (array/object) = func call" is handled.

Furthermore, this adds calls to isRefSafe in a few places: as it turned
out, the test cases

    {"array/ref", "[1,2,x] = a[_]", "[a]", "[x]"},
    {"object/ref", `{"x": x} = a[_]`, "[a]", "[x]"},

would have, without the check, passed without "a" in "safe" (3rd field).
The fact that these cases are including "a" is evidence for this being
the correct behaviour.

Fixes #4078.

Signed-off-by: Stephan Renatus <stephan.renatus@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
3 participants