Repository navigation
Fix/strict filter parsing - #2
Merged
Merged
Conversation
The parser stopped at the first complete expression and ignored whatever followed, and the lexer skipped characters it did not know. A filter such as `status eq "a") or (status pr` parsed as `status eq "a"`, so a typo silently widened a query, and a caller that wrapped client text in parentheses behind a required condition could be escaped. The grammar now has an entry rule anchored at end of input, and lexer errors fail the parse like parser errors do. A null or blank filter is rejected with a clear message, and surrounding whitespace is ignored.
Callers that must always apply a condition of their own (an owner, a tenant) next to a client-supplied filter had no safe way to combine the two and joined filter text by hand, which lets a client filter containing `or` match rows outside the condition. scopeFilter checks that each part is one complete filter on its own and wraps both in parentheses, so the client's part cannot widen the scope whatever operators or attributes it uses. A missing scope is an error; a missing client filter returns the scope alone. The README documents strict parsing and scoping.
Surrounding whitespace is ignored before parsing, so the reported position was relative to the trimmed text. It now counts the leading whitespace and points at the character in the input the caller passed.
A filter that does not parse is the mistake of whoever wrote the filter, usually a client, and callers need to tell it apart from other failures to answer with a client error. It extends IllegalArgumentException, so existing handlers keep working. A scope that does not parse in scopeFilter stays a plain IllegalArgumentException: the scope is written by the calling code, and a broken one must not be reported as a client mistake.
Found in review of the strict-parsing change. A filter built for the purpose could exhaust the stack: the parser and the filter tree recurse once per level of parentheses, per logical operator and per attribute path segment. The lexer now rejects a filter past 50 levels of parentheses, 500 logical operators or 10 path segments, before the parser recurses. A list of values does not recurse and is not limited. A whole number that does not fit in a long surfaced as a raw NumberFormatException. It is now a FilterSyntaxException, and a whole number written with an exponent (1E+5) parses instead of failing. Error positions are an index into the expression the caller passed. ANTLR's column restarts after a line break inside a string value and counts code points, so the reported position was wrong after either. A missing filter or prefix is a mistake in the calling code, so it is a plain IllegalArgumentException and no longer a FilterSyntaxException, and the prefix is checked before parsing. ScimEvaluator visits the new entry rule, so visiting the tree from parser.filter() returns the filter instead of null. scopeFilter evaluates each part the way parseFilter does and parses the combined expression before returning it, so whatever it returns, parseFilter accepts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.