Skip to content

fix: keep malformed \x and \u{...} string escapes as written instead of corrupting the string - #6383

Open
prql-bot wants to merge 5 commits into
mainfrom
fix/malformed-hex-unicode-escapes
Open

prql-bot wants to merge 5 commits into
mainfrom
fix/malformed-hex-unicode-escapes

Conversation

@prql-bot

Copy link
Copy Markdown
Collaborator

A string's \x or \u{...} escape that isn't well-formed no longer changes the string's contents. Before this, the lexer quietly rewrote them:

Input Before After
"\x4g" 'xg' (the 4 is dropped) 'x4g'
"\u{}z" a NUL byte, then z 'u{}z'
"\u{41" (no closing brace) 'A' 'u{41'
"\u{110000}" (not a valid code point) U+FFFD 'u{110000}'

Well-formed escapes ("\x41", "\u{1F600}") are unchanged. A malformed escape is now handled like an unknown escape such as "\q": the backslash is dropped and the rest is kept as written. In parse_escape_sequence, both arms now save the position before reading hex digits and rewind to it when the escape doesn't match the form the strings reference documents: exactly two digits for \xhh, and 1–6 digits, a closing } and a valid code point for \u{...}. Before, those arms kept whatever digits they had already consumed.

Raising a lexer error would be stricter. I kept the lenient behavior because it matches how unknown escapes are already treated, and because maintainers have previously said permissive lexing is fine unless it causes a problem (#3476). Here the problem was data loss, which rewinding fixes.

Cases added to lexer::test::quotes; the \x4g case fails on main (left: "xg", right: "x4g"). cargo test -p prqlc-parser and cargo test -p prqlc pass locally.

prql-bot and others added 3 commits September 27, 2026 06:58
…upting the string

A \x followed by one hex digit dropped that digit ("\x4g" became "xg"), and
\u{...} accepted an empty, unclosed or out-of-range escape, emitting a NUL, the
code point without its brace, or U+FFFD. Both now rewind and keep the text as
written, as an unknown escape already does.

@prql-bot prql-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One behavior change in interpolated strings isn't covered by the description; see inline.

c
}
None => {
input.rewind(checkpoint);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewinding here also affects interpolated strings, because interpolation() lexes f- and s-strings with escaping on and then parses {...} out of the result. On main, f"\u{x}" and s"\u{x}" fail to compile (expected '}', but found end of input). With this change, f"\u{x}" compiles to CONCAT('u', x) and s"\u{x}" splices ux into the SQL. A malformed escape turns into a silent interpolation of whatever name follows it. Going the other way, f"\u{41" used to give 'A' and now fails with expected '{' or interpolated string variable.

This matches what an unknown escape already does (f"\q{x}" interpolates x), so it may be acceptable. It isn't mentioned in the description or the changelog, though, and whether a malformed \u{ inside f-/s-strings should interpolate or error is a maintainer's call.

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