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

Index out of range error in handling of bound refs #238

Closed
tsandall opened this issue Feb 2, 2017 · 0 comments
Closed

Index out of range error in handling of bound refs #238

tsandall opened this issue Feb 2, 2017 · 0 comments
Labels

Comments

@tsandall
Copy link
Member

tsandall commented Feb 2, 2017

Evaluation encounters an index out of range error if the ref being evaluated is bound to another ref that has a longer ground prefix than the original ref. For instance:

{
    "a": {
        "b": {
            "c": {
                "d": "e"
            }
        }
    }
}
package ex

p = x :- data.a.b.c.d = x

q :- p, p

When the second ref to "p" is evaluated, the ground prefix is obtained from the binding for p (which is is data.a.b.c.d).

Example:

OPA 0.4.1-dev (commit 89d13d0-dirty, built at 2017-02-02T02:41:04Z)

Run 'help' to see a list of commands.

> data
panic: runtime error: index out of range

goroutine 1 [running]:
panic(0x478480, 0xc4200100f0)
	/usr/local/Cellar/go/1.7.5/libexec/src/runtime/panic.go:500 +0x1a1
github.com/open-policy-agent/opa/topdown.evalRefRecNonGround(0xc420188640, 0xc4201f8600, 0x3, 0x4, 0xc4201f6de0, 0x5, 0x5, 0xc4201f6db0, 0x4, 0x715700)
	/Users/torin/go/src/github.com/open-policy-agent/opa/topdown/topdown.go:1220 +0x11ec
github.com/open-policy-agent/opa/topdown.evalRefRec(0xc420188640, 0xc4201f8600, 0x3, 0x4, 0xc4201f6db0, 0x1, 0x1)
	/Users/torin/go/src/github.com/open-policy-agent/opa/topdown/topdown.go:1086 +0x2f9
github.com/open-policy-agent/opa/topdown.evalRef(0xc420188640, 0xc4201ef5b8, 0x0, 0x1, 0xc4201f8600, 0x3, 0x4, 0xc4201f6db0, 0x715680, 0x4)
	/Users/torin/go/src/github.com/open-policy-agent/opa/topdown/topdown.go:998 +0x5a0
github.com/open-policy-agent/opa/topdown.evalRef(0xc420188640, 0xc4201ef5b0, 0x1, 0x2, 0xc4201f8600, 0x3, 0x4, 0xc4201f6db0, 0xc4201f84c0, 0x344101)
	/Users/torin/go/src/github.com/open-policy-agent/opa/topdown/topdown.go:1022 +0x1bc
github.com/open-policy-agent/opa/topdown.evalRef(0xc420188640, 0xc4201ef5a8, 0x2, 0x3, 0xc4201ed360, 0x2, 0x2, 0xc4201f6db0, 0x49c700, 0x1)
	/Users/torin/go/src/github.com/open-policy-agent/opa/topdown/topdown.go:1022 +0x1bc
github.com/open-policy-agent/opa/topdown.evalRef(0xc420188640, 0xc4201ef5a0, 0x3, 0x4, 0xc4200b4848, 0x1, 0x1, 0xc4201f6db0, 0x0, 0xc4201bb788)
@tsandall tsandall added the bug label Feb 2, 2017
tsandall added a commit to tsandall/opa that referenced this issue Feb 3, 2017
References were not being evaluated correctly. The top-level evalRefRec
function was computing the ground prefix from the reference's binding
and then performing the rest of the evaluation with the original
reference and the prefix. Instead, the rest of the evaluation should
proceed with the plugged value and the prefix. This fix was not
compatible with how reference bindings were stored. Before, reference
bindings were keyed with the original (potentially non-ground)
reference. Now, they are keyed with the evaluated version of the
reference. This version is guaranteed to be ground. As a result, the
PlugValue handling of references must be updated to handle cases such
as:

orig ref:   p[x][y]
locals:     {x: 1, y: 2}
refs:       {p[1][2]: "foo"}

The solution is to recursively plug the ref. E.g., p[x][y] => p[1][y] =>
p[1][2] => "foo".

Fixes open-policy-agent#238
tsandall added a commit to tsandall/opa that referenced this issue Mar 22, 2017
If the entire ref is already bound, do not bother re-evaluating it. This
prevents a recursive binding from being added in the case where the same
local ref appears twice and is bound to a data ref. The fix for open-policy-agent#238 was
caused a stack overflow in this case!

Fixes open-policy-agent#298
tsandall added a commit that referenced this issue Mar 22, 2017
If the entire ref is already bound, do not bother re-evaluating it. This
prevents a recursive binding from being added in the case where the same
local ref appears twice and is bound to a data ref. The fix for #238 was
caused a stack overflow in this case!

Fixes #298
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant