[ISSUE #11437] fix(filter): reject selectors that have trailing tokens

[iloveeyjafjalla]

Which Issue(s) This PR Fixes

• Fixes [Bug] The SQL92 selector parser ignores trailing tokens, so a malformed filter silently drops clauses instead of failing #11437

Brief Description

The generated SQL92 parser never checked that the whole selector was consumed: its start rule parses one expression and returns it.

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:

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 Selector parse EOF #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.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[RockteMQ-AI]

Review by github-manager-bot

Summary

Fixes a correctness issue in the SQL92 filter parser where trailing tokens after a valid expression were silently ignored. For example, "a = 3 b = 4" would parse as just "a = 3", causing consumers to silently receive messages that violate the dropped clause.

Findings

• [Critical] SelectorParser.java:73-76 — The fix adds jj_consume_token(0) (EOF check) at the end of the JmsSelector() start rule. This ensures that any trailing tokens after a complete expression cause a parse failure, which is the correct behavior. Without this check, the parser would silently accept malformed selectors, leading to incorrect message filtering.
• [Info] SelectorParser.jj:47-51 — The grammar source is updated to include <EOF> at the end of the start rule, keeping it in sync with the generated parser. This is good practice for maintainability.
• [Info] ParserTest.java — Two regression tests are added:
  • testParse_trailingTokenIsRejected: verifies "a = 3 b = 4" is rejected
  • testParse_trailingGarbageAfterLiteralIsRejected: verifies "a < 10bbbbb" is rejected
    Both tests correctly assert that MQFilterException is thrown.

Suggestions

• The fix is minimal, focused, and addresses the root cause. The approach of adding an EOF check is the standard way to ensure complete input consumption in parser generators like JavaCC.
• The PR description mentions that PR #602 (2018) attempted the same fix but was closed due to time constraints. It is good to see this revisited and properly merged.
• The change is backward compatible for valid selectors, as they will continue to parse successfully. Only malformed selectors with trailing tokens will now fail, which is the correct behavior.

Overall

This is an important correctness fix that prevents silent data loss in message filtering. The implementation is clean, the test coverage is good, and the change is minimal. LGTM.

---

Automated review by github-manager-bot

[RockteMQ-AI]

Review by github-manager-bot

Summary

Fixes the SQL92 selector parser to reject selectors with trailing tokens by adding an EOF check to the start rule. Previously, silently returned .

Findings

• [Info] SelectorParser.java:100-101 — Adding (EOF) at the end of is the correct fix. The grammar source is also updated for consistency.
• [Info] ParserTest.java — Two regression tests cover both the missing-AND case and the garbage-after-literal case from the original issue.

Verdict

LGTM. Important correctness/security fix — silently truncated selectors could cause consumers to receive messages they did not intend to filter for.

---

Automated review by github-manager-bot