Skip to content

Fix tokenization of string literals containing an escaped backslash - #914

Open
Tejas5405 wants to merge 1 commit into
andialbrecht:masterfrom
Tejas5405:fix-814-escaped-backslash-in-string-literals
Open

Tejas5405 wants to merge 1 commit into
andialbrecht:masterfrom
Tejas5405:fix-814-escaped-backslash-in-string-literals

Conversation

@Tejas5405

Copy link
Copy Markdown

Closes #814

Problem

The string literal rules in sqlparse/keywords.py are ambiguous:

(r"'(''|\\'|[^'])*'", tokens.String.Single),

Once [^'] has consumed the first backslash of a \\ pair, the \\' alternative pairs the second backslash up with the closing quote and swallows it, so the literal runs on into the rest of the statement:

>>> [t.value for t in sqlparse.parse(r"SELECT '\\', '\\'")[0].flatten()]
["SELECT", " ", "'\\\\', '", "\\", "\\", "'"]

The last three tokens are Token.Error, so every downstream consumer — formatting, splitting, identifier detection — sees garbage after the first literal.

An escape is really either a doubled quote or a backslash followed by any character, and a plain character is neither a quote nor a backslash:

(r"'(''|\\.|[^'\\])*'", tokens.String.Single),

That makes the match unambiguous. The escaping forms that already worked are unaffected: 'it\'s' and 'it''s' still lex as one literal, so does a literal ending in an escaped backslash ('a\\'), multiline literals still match, and an unterminated literal still fails to match (and is reported as error tokens, as before).

The double quoted rule carried the exact same ambiguity — SELECT "\\", "\\" was mis-tokenized the same way — so it is fixed the same way. It reports String.Symbol, hence a separate test. Happy to split that line into its own pull request if you would rather review it separately.

Tests

In tests/test_regressions.py:

  • test_issue814 — the reproducer from the issue
  • test_issue814_quoted_identifier — the same bug in the double quoted rule
  • test_issue814_keeps_escaping_forms — parametrized over 'it\'s', 'it''s', 'a\\', 'plain', all of which must keep lexing as a single String.Single
  • test_issue814_unterminated_string_still_unmatched — 'abc must keep failing to match

Without the source change, test_issue814 and test_issue814_quoted_identifier fail; the other two pass before and after, so they guard the fix rather than reproduce the bug.

Executed before submitting:

  • pytest → 513 passed, 2 xfailed, 1 xpassed (506 before, plus the 7 new cases)
  • ruff check sqlparse/ (the command behind make lint) → clean
  • checked against the released 0.5.3 as well as current master

AI assistance disclosure

The investigation and the patch were prepared with AI assistance. Everything above was verified by running the lexer directly on current master, by running the full test suite both with and without the change, and by running ruff check; the changelog entry and this description are written from those results.

`(r"'(''|\\'|[^'])*'", tokens.String.Single)` is ambiguous: once `[^']`
has consumed the first backslash of `\\`, the `\\'` alternative pairs the
second backslash up with the closing quote and swallows it, so the
literal runs on into the rest of the statement.

`SELECT '\\', '\\'` therefore lexed as one string token covering
`'\\', '`, followed by three error tokens.

An escape is really either a doubled quote or a backslash followed by
any character, and a plain character is neither a quote nor a
backslash, which makes the match unambiguous. The double quoted rule
carried the same ambiguity and is fixed the same way; it reports
String.Symbol, so `SELECT "\\", "\\"` was broken identically.

Closes andialbrecht#814
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.

Incorrect Tokenization of Escaped Backslashes in SQL String Literals

1 participant