Skip to content

Fix rule evaluation for list options #1077

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

Merged
merged 2 commits into from
Oct 19, 2016
Merged

Conversation

imbriaco
Copy link
Member

Previously, the rule evaluator would coerce list type options into a string before evaluating the comparisons contained within the right-hand-side of an in operation. For example:

String Comparison:

Example Command: my-command --list foo --list bar --list baz
Rule: when command is my-command with option[list] in [ foo ] allow
Evaluates: "foobarbaz" == "foo"

This would only return the expected result if the list option only had a single value.

Regex Match:

Example Command: my-command --list foo --list bar --list baz
Rule: when command is my-command with option[list] in [ /foo/ ] allow
Evaluates: Regex.match?(~r/foo/, "foobarbaz")

The Regex version of this would usually return the expected results, though there could be unexpected behavior which would also affect the string comparison version.

Edge Cases, Oh My:

For instance, the following command and rule would evaluate to true which is unlikely to be the expected behavior.

Example Command: my-command --list f --list oo
Rule: when command is my-command with option[list] in [ foo ] allow
Evaluates: "foo" == "foo"

This PR cases the in expression to be evaluated against each item in the option rather than on a stringified version of the option.

@imbriaco imbriaco added this to the Cog 0.16 milestone Oct 19, 2016
Copy link
Member

@kevsmith kevsmith left a comment

Choose a reason for hiding this comment

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

👍

@imbriaco imbriaco merged commit 1719602 into master Oct 19, 2016
@imbriaco imbriaco deleted the imbriaco/option-list-perm-bugfix branch October 19, 2016 19:57
@imbriaco imbriaco removed the review label Oct 19, 2016
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.

2 participants