iloveeyjafjalla opened a new pull request, #11438:
URL: https://github.com/apache/rocketmq/pull/11438
### Which Issue(s) This PR Fixes
- Fixes #11437
### Brief Description
The generated SQL92 parser never checked that the whole selector was
consumed: its start rule parses one expression and returns it.
```java
final public BooleanExpression JmsSelector() throws ParseException {
Expression left = orExpression();
return asBooleanExpression(left);
}
```
Anything after a syntactically complete expression was therefore silently
discarded, and the truncated expression was returned as the filter —
`SelectorParser.parse("a = 3 b = 4")` returned `(a = 3)` (a user who forgets
`AND` gets this) and `SelectorParser.parse("a < 10bbbbb")` returned `(a < 10)`.
Since the parse succeeds, the broker compiles the shortened expression
(`ConsumerFilterManager.build` → `SqlFilter.compile` → `SelectorParser.parse`)
and the group silently receives messages that violate the dropped clause, with
no error surfaced anywhere.
The change consumes the end-of-input token at the end of the start rule, so
a selector with trailing tokens is rejected exactly like any other selector
that does not match the grammar:
```java
Expression left = orExpression();
// the selector has to end here: trailing tokens mean the user's expression
was
// truncated by the parser instead of being reported as invalid
jj_consume_token(0);
return asBooleanExpression(left);
```
The grammar source gets the corresponding `<EOF>` (`SelectorParser.jj`,
start rule) so the two stay in sync; `filter/pom.xml` has no JavaCC plugin, so
the generated `SelectorParser.java` is the checked-in source of truth and the
`.jj` edit is for consistency only (its header also lists the modifications
made when this file was ported from ActiveMQ, and dropping the end-of-input
check is not among them — ActiveMQ's start rule is `orExpression() <EOF>`).
Prior art for context: PR #602 (2018) made this same change and was closed
with "close due to time too long and wrong to merge to master, welcome to
create a new PR to the apache: develop branch" — this PR is that change against
the current `develop`.
Valid selectors are unaffected; only selectors whose token stream continues
past a complete expression now fail at parse time with the same
`MQFilterException` ("Invalid MessageSelector.") that other invalid selectors
produce.
### How Did You Test This Change?
Two regression tests in `ParserTest`, both asserting the parse raises
`MQFilterException`:
- `testParse_trailingTokenIsRejected`: `"a = 3 b = 4"`
- `testParse_trailingGarbageAfterLiteralIsRejected`: `"a < 10bbbbb"` (the
example from #602)
Results on Temurin JDK 8 (the JDK CI uses), base `0dd48c39d` plus this patch:
- before the fix: both fail with `Expecting value to be false but was true`
(the parse returns an expression instead of throwing)
- after the fix: `mvn -B -ntp -pl filter test` — `Tests run: 111, Failures:
0, Errors: 0, Skipped: 0`
- `mvn -B -ntp -pl filter clean package` (module tests + checkstyle +
spotbugs enabled): `BUILD SUCCESS`, `You have 0 Checkstyle violations.`,
`BugInstance size is 0`
This change was prepared with AI assistance (analysis, patch and test
drafting); the diff and every result above were reproduced locally by running
the module's tests against the base commit and the patched tree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
