[I] [Bug] The SQL92 selector parser ignores trailing tokens, so a malformed filter silently drops clauses instead of failing [rocketmq]

["iloveeyjafjalla (via GitHub)" <gi...@apache.org>]

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: commits-unsubscribe@rocketmq.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] [Bug] The SQL92 selector parser ignores trailing tokens, so a malformed filter silently drops clauses instead of failing [rocketmq]

["RockteMQ-AI (via GitHub)" <gi...@apache.org>]

RockteMQ-AI commented on issue #11437:
URL: https://github.com/apache/rocketmq/issues/11437#issuecomment-6166898813

   ## 🤖 Bot Evaluation: ✅ Bug Confirmed
   
   **Category:** `type/bug`
   **Severity:** Medium — silent data loss in message filtering; consumers receive messages that should have been filtered out.
   
   ### Verification
   
   I verified the bug by inspecting the source code on `develop` branch:
   
   1. **`filter/src/main/java/org/apache/rocketmq/filter/parser/SelectorParser.java`** — The `JmsSelector()` start rule parses one `orExpression()` and returns immediately. There is no `jj_consume_token(0)` or equivalent EOF check anywhere in the file.
   
   2. **`filter/src/main/java/org/apache/rocketmq/filter/parser/SelectorParser.jj`** (lines 200–210) — The grammar rule for `JmsSelector()` has no `<EOF>` token at the end:
      ```
      BooleanExpression JmsSelector() :
      { Expression left=null; }
      {
          ( left = orExpression() )
          { return asBooleanExpression(left); }
      }
      ```
   
   3. **Impact confirmed:** `SelectorParser.parse("a = 3 b = 4")` returns `(a = 3)` — the second clause is silently dropped. This affects both client-side subscription (`MessageSelector.bySql(...)`) and broker-side filter compilation (`SqlFilter.compile(...)` via `ConsumerFilterManager.build`).
   
   ### Root Cause
   
   The JavaCC-generated parser consumes only as much input as matches the grammar and never asserts that the entire input string was consumed. Any trailing tokens after a valid expression are silently ignored.
   
   ### Suggested Fix
   
   Add `<EOF>` to the grammar rule in `SelectorParser.jj`:
   ```
   BooleanExpression JmsSelector() :
   { Expression left=null; }
   {
       ( left = orExpression() ) <EOF>
       { return asBooleanExpression(left); }
   }
   ```
   Then regenerate `SelectorParser.java` from the grammar. This will cause the parser to throw a `ParseException` when trailing tokens remain.
   
   ### Notes
   
   - The issue description is thorough and well-reproduced. Thank you @iloveeyjafjalla!
   - The referenced PR #602 (2018) attempted a similar fix but was not merged.
   - The author mentioned they will open a PR — looking forward to it.
   
   ---
   *Evaluated by [RockteMQ-AI](https://github.com/RockteMQ-AI) bot · Issue evaluation workflow*


-- 
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: commits-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org