iloveeyjafjalla opened a new issue, #11437: URL: https://github.com/apache/rocketmq/issues/11437
### Before Creating the Bug Report - [x] I found a bug, not just asking a question, which should be created in [GitHub Discussions](https://github.com/apache/rocketmq/discussions). - [x] I have searched the [GitHub Issues](https://github.com/apache/rocketmq/issues) and [GitHub Discussions](https://github.com/apache/rocketmq/discussions) of this repository and believe that this is not a duplicate. - [x] I have confirmed that this bug belongs to the current repository, not other repositories of RocketMQ. ### Runtime platform environment OS: any (client-side SQL92 filter parsing: `MessageSelector.bySql(...)` / `FilterAPI`, and the broker-side compile of the same expression) ### RocketMQ version branch: develop Git commit id: 0dd48c39d ### JDK Version Compiler: Temurin 1.8.0_504 (target 1.8, same as CI) ### Describe the Bug The generated SQL92 parser never checks that the whole selector was consumed — its start rule parses one expression and returns: ```java final public BooleanExpression JmsSelector() throws ParseException { Expression left = orExpression(); return asBooleanExpression(left); } ``` (`filter/src/main/java/org/apache/rocketmq/filter/parser/SelectorParser.java:99-102`; there is no `jj_consume_token(0)` anywhere in the file, and the grammar source has the same rule without `<EOF>` — `filter/src/main/java/org/apache/rocketmq/filter/parser/SelectorParser.jj:200-211`.) So anything after a syntactically complete expression is silently thrown away, and the truncated expression is returned as if it were the whole filter: - `SelectorParser.parse("a = 3 b = 4")` returns `(a = 3)` — the second clause is dropped instead of being reported as a syntax error (a user who forgets `AND` gets exactly this); - `SelectorParser.parse("a < 10bbbbb")` returns `(a < 10)`. Verified today on this tree: `filter/src/test/java/org/apache/rocketmq/filter/ParserTest.java` with two assertions that expect `MQFilterException` for these inputs fails with `Expecting value to be false but was true`, i.e. both parses succeed. ### Steps to Reproduce 1. Client side: `consumer.subscribe(topic, MessageSelector.bySql("a = 3 b = 4"))` (or `FilterAPI.build(topic, "a = 3 b = 4", SQL92)`): no exception is raised, the subscription is registered with the expression `a = 3`. 2. Broker side: `ConsumerFilterManager.build` compiles the same expression, `FilterFactory.INSTANCE.get(SQL92).compile(...)` → `SqlFilter.compile` → `SelectorParser.parse` (`filter/src/main/java/org/apache/rocketmq/filter/SqlFilter.java:36`) succeeds, so the filter data is built for the truncated expression. 3. Consumers in the group then receive messages that violate the dropped clause (e.g. `b != 4`), with no error anywhere — the only signal a user could notice is the filter behaving as if half of it did not exist. ### What Did You Expect to See? A selector that does not match the grammar is rejected with `MQFilterException` ("Invalid MessageSelector."), the way every other malformed selector is — this is the behaviour the module's own test intent encodes (`ParserTest.testParse_invalid` asserts that an input the grammar cannot accept raises `MQFilterException`), and the behaviour of the grammar this file is ported from, whose start rule is `orExpression() <EOF>`. ### What Did You See Instead? The parse succeeds with a prefix of the selector, so a malformed filter is accepted and silently applied in its truncated form — messages that the user's filter excludes are delivered instead of the subscription failing fast. ### Additional Context Prior art, for full context: PR #602 (2018) added the end-of-input token to the grammar for exactly these cases 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 report is that change against the current `develop`, where the defect still reproduces. Minimal fix: consume the end-of-input token at the end of the start rule (`jj_consume_token(0)` in the generated parser, `<EOF>` in the grammar source so both stay in sync). Valid selectors are unaffected; any selector with trailing tokens now fails at parse time with the same `MQFilterException` as other invalid selectors. Note that `filter/pom.xml` has no JavaCC plugin, so the generated parser is the checked-in source of truth and the grammar file is updated for consistency only. I will open a PR with the fix plus regression tests in `ParserTest` (red before: both parses return an expression; green after: both raise `MQFilterException`). -- 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]
