Skip to content

fix: require end of input when parsing a single statement - #2733

Open
MoshiCoCo wants to merge 1 commit into
JSQLParser:masterfrom
MoshiCoCo:master
Open

MoshiCoCo wants to merge 1 commit into
JSQLParser:masterfrom
MoshiCoCo:master

Conversation

@MoshiCoCo

Copy link
Copy Markdown

Problem

Statement() — the production behind CCJSqlParserUtil.parse(String), parse(Reader), parse(InputStream) and parseAST() — silently returns a truncated AST when the input holds more than one statement, instead of reporting a parse error.

Since ST_SEMICOLON matches blank line runs as well as ";":

<ST_SEMICOLON : ( ";" | ("\n\n\n") | "\n/\n" | "\ngo\n" ) >

this is reachable from ordinary formatted SQL that contains no semicolon at all:

// two statements as far as the lexer is concerned
CCJSqlParserUtil.parse("SELECT a FROM dual WHERE x = ?\n\n\n AND y = ?");
// -> SELECT a FROM dual WHERE x = ?   (no exception, tail dropped)

CCJSqlParserUtil.parse("SELECT a FROM dual; SELECT b FROM dual");
// -> SELECT a FROM dual               (no exception, tail dropped)

Downstream tools that parse a statement and then reuse the original metadata are broken by this. A concrete case from a production MyBatis stack: PageHelper's DefaultCountSqlParser calls newParser(reader).Statement() to rewrite a paged query into its count query, and ExecutorUtil.executeAutoCount keeps the original parameterMappings. A mapper whose dynamic SQL renders three consecutive newlines (for example an XML comment block that MyBatis strips, leaving the newlines behind) is truncated at that point, so the count statement ends up with fewer ? placeholders than mappings — Parameter index out of range, or, when the dropped tail has no placeholders, a silently wrong row count.

Root cause

Regression from 063d2442 ("feat: return any UnsupportedStatement content", first shipped in jsqlparser-5.0), which replaced the terminator of Statement() with a choice:

- [ <ST_SEMICOLON> ] <EOF>
+ ( <ST_SEMICOLON> | <EOF> )

Consuming a separator became an alternative to reaching EOF, so the end-of-input validation was lost. Statements() kept its trailing <EOF> and was unaffected.

Fix

src/main/jjtree/net/sf/jsqlparser/parser/JSqlParserCC.jjt, both branches of Statement():

  ]
+ <EOF>
  )
  |
  (
      (stm = SingleStatement() | stm = Block())
-     ( <ST_SEMICOLON> | <EOF> )
+     ( <ST_SEMICOLON> )* <EOF>
  )

Trailing separators stay legal (any number of them, so "…;", "…\n\n\n" and "… -- done" still parse), while any unconsumed statement is now reported. This restores the contract that 4.9 and earlier had.

Compatibility

  • Statements() and SingleStatement() are untouched. Walking a script statement by statement remains the incremental API's job, which is what CCJSqlParserUtil.streamStatements already uses.
  • The UnsupportedStatement support added by 063d2442 is preserved — that alternative is guarded by its own LOOKAHEAD and stays reachable.
  • Behaviour change: input with trailing content now throws instead of returning a partial statement. Callers that want "first statement only" should use SingleStatement().

Tests

  • CCJSqlParserUtilTest#testParseRejectsUnconsumedInput (new): rejects newline- and semicolon-separated input through parse(String), parse(InputStream), parseAST and a direct Statement() call; still accepts trailing separators and comments; parseStatements keeps splitting correctly.
  • SqlRoutineBodyBoundaryTest: the case that asserted the lenient behaviour (directStatementParserLeavesFollowingStatementAvailable) is rewritten to drive the parser with SingleStatement(). This preserves its original intent — verifying that a routine body ends where the following statement begins — and additionally asserts the separators in between.
  • Full suite: Tests run: 9093, Failures: 0, Errors: 0, Skipped: 25.

- `Statement()` lost its `<EOF>` validation in 063d244 ("feat: return any `UnsupportedStatement`
  content"), which replaced `[ <ST_SEMICOLON> ] <EOF>` with `( <ST_SEMICOLON> | <EOF> )`; ever
  since, input holding more than one statement silently returned a truncated AST instead of
  failing, so callers such as PageHelper's count SQL parser reuse the full parameter mappings
  against a partial statement
- `ST_SEMICOLON` also matches blank line runs such as `"\n\n\n"`, which makes the silent
  truncation reachable from ordinary formatted SQL that has no semicolon at all
- restore the pre-063d2442 contract with `( <ST_SEMICOLON> )* <EOF>` in both `Statement()`
  branches, keeping trailing separators and comments legal while rejecting any unconsumed
  statement
- `Statements()` (which kept its trailing `<EOF>`) and `SingleStatement()` are untouched; walking
  a script statement by statement stays the incremental API's job
- rewrite the routine boundary test that asserted the lenient behaviour to drive the parser with
  `SingleStatement()`, which is how `streamStatements` consumes multi statement scripts

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant