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

planner: Correct crash in virtual doc iteration #2642

Merged
merged 5 commits into from
Aug 24, 2020

Conversation

patrick-east
Copy link
Contributor

There was an issue with iterating over virtual docs when planning
an expression that had with keywords. The iterator would potentially
be called multiple times, so the "with" values needed to be pushed
and popped accordingly. This essentially copies the pattern used in
the topdown eval flow.

Fixes: #2601
Signed-off-by: Patrick East east.patrick@gmail.com

@patrick-east patrick-east requested a review from tsandall August 19, 2020 17:12
Copy link
Member

@tsandall tsandall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this change when the iterator passed to planWithRec is called multiple times, the funcstack and ruletrie structures will be synchronized correctly. Previously, those structures would get out of sync after the iterator was called once.

LGTM

test/wasm/assets/016_with.yaml Outdated Show resolved Hide resolved
There was an issue with iterating over virtual docs when planning
an expression that had `with` keywords. The iterator would potentially
be called multiple times, so the "with" values needed to be pushed
and popped accordingly. This essentially copies the pattern used in
the topdown eval flow.

Fixes: open-policy-agent#2601
Signed-off-by: Patrick East <east.patrick@gmail.com>
The assert helper was enforcing the order of a resultset, but the
order is non-deterministic. What we really care about it that all
expected results are contained in the set (well.. array).

Signed-off-by: Patrick East <east.patrick@gmail.com>
@tsandall tsandall merged commit de0f695 into open-policy-agent:master Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

panic: Planner panic due to with keyword usage
2 participants