diff --git a/python/extractor/semmle/logging.py b/python/extractor/semmle/logging.py index 31805040bf6d..64037163e697 100644 --- a/python/extractor/semmle/logging.py +++ b/python/extractor/semmle/logging.py @@ -65,6 +65,14 @@ def write_message_with_proc(level, proc_id, text): _logging_process = None +def format_message(fmt, args): + '''Applies `%`-formatting to `fmt`, but only when there are arguments to interpolate. + + This mirrors the standard library's `logging` behaviour, and means that a message that has + already been formatted -- and may therefore contain arbitrary `%` directives coming from the + code being analysed -- is passed through unharmed.''' + return fmt % args if args else fmt + def stop(): _logging_process.join() @@ -105,7 +113,7 @@ def log(self, level, fmt, *args): '''Log a message in a process safe fashion. Message will be of the form [level] fmt%args.''' if level <= self.level: - txt = fmt % args + txt = format_message(fmt, args) try: self.queue.put((self.color | level, self.proc_id, txt), False) except Exception: diff --git a/python/extractor/semmle/python/imports.py b/python/extractor/semmle/python/imports.py index 8da7df474480..98b848d3a688 100644 --- a/python/extractor/semmle/python/imports.py +++ b/python/extractor/semmle/python/imports.py @@ -252,6 +252,6 @@ def importer_from_options(options, finder, logger): importer = CachingModuleImporter(options.trap_cache, finder, logger) except Exception as ex: if options.trap_cache is not None: - logger.warn("Failed to create caching importer: %s", ex) + logger.warning("Failed to create caching importer: %s", ex) importer = ModuleImporter(finder, logger) return importer diff --git a/python/extractor/semmle/python/parser/dump_ast.py b/python/extractor/semmle/python/parser/dump_ast.py index 802bed8fac24..3a7db5ab0713 100644 --- a/python/extractor/semmle/python/parser/dump_ast.py +++ b/python/extractor/semmle/python/parser/dump_ast.py @@ -99,12 +99,12 @@ def visit(self, node, level=0, visited=None): class StdoutLogger(logging.Logger): error_count = 0 def log(self, level, fmt, *args): - sys.stdout.write(fmt % args + "\n") + sys.stdout.write(logging.format_message(fmt, args) + "\n") def info(self, fmt, *args): self.log(logging.INFO, fmt, *args) - def warn(self, fmt, *args): + def warning(self, fmt, *args): self.log(logging.WARN, fmt, *args) self.error_count += 1 diff --git a/python/extractor/semmle/python/parser/tsg_parser.py b/python/extractor/semmle/python/parser/tsg_parser.py index 356faf249ed3..83dc853b525e 100644 --- a/python/extractor/semmle/python/parser/tsg_parser.py +++ b/python/extractor/semmle/python/parser/tsg_parser.py @@ -3,6 +3,7 @@ # Functions and classes used for parsing Python files using `tree-sitter-graph` from ast import literal_eval +import re import sys import os import semmle.python.parser @@ -144,6 +145,7 @@ def read_tsg_python_output(path, logger): elif value == "#null": # e.g. `exc: #null` value = None else: # literal values, e.g. `name: "k1.k2"` or `level: 5` + value = rust_to_python_escapes(value) try: if key =="s" and value[0] == '"': # e.g. `s: "k1.k2"` value = evaluate_string(value) @@ -171,6 +173,36 @@ def read_tsg_python_output(path, logger): logger.debug("Read {} nodes and {} edges from TSG output".format(len(node_attr), len(edge_attr))) return node_attr, edge_attr +# `tsg-python` serialises string values using Rust's `Debug` formatting, which diverges from what +# Python's `literal_eval` accepts in two ways: +# - characters Rust considers non-printable -- including grapheme-extending ones such as the U+FE0F +# variation selector, U+200D zero width joiner and combining accents -- are rendered as `\u{...}`, +# a syntax Python does not know at all; +# - NUL is rendered as `\0`, which Python reads as the start of an *octal* escape, silently +# swallowing up to two more digits (NUL followed by `1` is emitted as `"\01"`, which decodes +# to `\x01`). +# Everything else Rust emits (`\t`, `\r`, `\n`, `\\`, `\"`, and unescaped characters) is read back +# identically by `literal_eval`, as verified exhaustively over every Unicode scalar value. +_RUST_ESCAPE = re.compile(r"\\(?:u\{([0-9a-fA-F]{1,6})\}|.)", re.DOTALL) + +def rust_to_python_escapes(text): + """Rewrites Rust escapes in `text` that Python would reject or misread into their equivalents. + + Matching every escape sequence (rather than only the offending ones) keeps the scan in step with + the backslashes, so an escaped backslash -- how a literal `\\u{fe0f}` in the source is + serialised -- is left alone.""" + if "\\u{" not in text and "\\0" not in text: + return text + def replace(match): + code_point = match.group(1) + if code_point is None: + return "\\x00" if match.group(0) == "\\0" else match.group(0) + code_point = int(code_point, 16) + if code_point > 0xFFFF: + return "\\U{:08x}".format(code_point) + return "\\u{:04x}".format(code_point) + return _RUST_ESCAPE.sub(replace, text) + def evaluate_string(s): s = literal_eval(s) prefix, quotes, content = split_string(s, None) diff --git a/python/extractor/semmle/python/passes/flow.py b/python/extractor/semmle/python/passes/flow.py index 9f434fe0c4c8..6ea5405a8540 100755 --- a/python/extractor/semmle/python/passes/flow.py +++ b/python/extractor/semmle/python/passes/flow.py @@ -12,7 +12,7 @@ from semmle.python.passes import unroller from semmle.python import modules import semmle.graph as graph -from semmle.logging import Logger +from semmle.logging import Logger, format_message __all__ = [ 'FlowPass' ] @@ -1924,7 +1924,7 @@ def write_ssa_phi(out, phi, arg): class FakeLogger(object): def debug(self, fmt, *args): - print(fmt % args) + print(format_message(fmt, args)) def traceback(self): print(traceback.format_exc()) diff --git a/python/extractor/semmle/worker.py b/python/extractor/semmle/worker.py index 96761a01c77e..ac8231390742 100644 --- a/python/extractor/semmle/worker.py +++ b/python/extractor/semmle/worker.py @@ -39,7 +39,7 @@ def __init__(self, max_depth, logger: Logger): changed_paths = data.get('changes', []) self.overlay_changes = { os.path.abspath(p) for p in changed_paths } except (IOError, ValueError) as e: - logger.warn("Failed to read overlay changes from '%s' (falling back to full extraction): %s", overlay_changes_file, e) + logger.warning("Failed to read overlay changes from '%s' (falling back to full extraction): %s", overlay_changes_file, e) self.overlay_changes = None def add_root(self, mod): diff --git a/python/extractor/tests/parser/unicode_escapes_new.expected b/python/extractor/tests/parser/unicode_escapes_new.expected new file mode 100644 index 000000000000..ed1ba2237543 --- /dev/null +++ b/python/extractor/tests/parser/unicode_escapes_new.expected @@ -0,0 +1,150 @@ +Module: [6, 0] - [29, 0] + body: [ + TypeAlias: [6, 0] - [6, 12] + name: + Name: [6, 5] - [6, 6] + variable: Variable('X', None) + ctx: Store + type_parameters: [] + value: + Name: [6, 9] - [6, 12] + variable: Variable('int', None) + ctx: Load + Assign: [9, 0] - [9, 37] + targets: [ + Name: [9, 0] - [9, 4] + variable: Variable('warn', None) + ctx: Store + ] + value: + Str: [9, 7] - [9, 37] + s: '⚠️ problem %s: %s' + prefix: '"' + implicitly_concatenated_parts: None + Assign: [10, 0] - [10, 35] + targets: [ + Name: [10, 0] - [10, 8] + variable: Variable('warn_raw', None) + ctx: Store + ] + value: + Str: [10, 11] - [10, 35] + s: '⚠️ problem %s: %s' + prefix: '"' + implicitly_concatenated_parts: None + Assign: [13, 0] - [13, 19] + targets: [ + Name: [13, 0] - [13, 3] + variable: Variable('zwj', None) + ctx: Store + ] + value: + Str: [13, 6] - [13, 19] + s: '👨\u200d💻' + prefix: '"' + implicitly_concatenated_parts: None + Assign: [16, 0] - [16, 14] + targets: [ + Name: [16, 0] - [16, 3] + variable: Variable('nfd', None) + ctx: Store + ] + value: + Str: [16, 6] - [16, 14] + s: 'café' + prefix: '"' + implicitly_concatenated_parts: None + Assign: [17, 0] - [17, 28] + targets: [ + Name: [17, 0] - [17, 11] + variable: Variable('soft_hyphen', None) + ctx: Store + ] + value: + Str: [17, 14] - [17, 28] + s: 'soft\xadhyphen' + prefix: '"' + implicitly_concatenated_parts: None + Assign: [20, 0] - [20, 12] + targets: [ + Name: [20, 0] - [20, 6] + variable: Variable('café', None) + ctx: Store + ] + value: + Name: [20, 9] - [20, 12] + variable: Variable('nfd', None) + ctx: Load + Assign: [23, 0] - [23, 23] + targets: [ + Name: [23, 0] - [23, 3] + variable: Variable('raw', None) + ctx: Store + ] + value: + Str: [23, 6] - [23, 23] + s: '⚠️\\u{fe0f}' + prefix: 'r"' + implicitly_concatenated_parts: None + Assign: [24, 0] - [24, 23] + targets: [ + Name: [24, 0] - [24, 6] + variable: Variable('joined', None) + ctx: Store + ] + value: + JoinedStr: [24, 9] - [24, 23] + values: [ + Str: [24, 9] - [24, 12] + s: '' + prefix: 'f"' + implicitly_concatenated_parts: None + Name: [24, 12] - [24, 15] + variable: Variable('zwj', None) + ctx: Load + Str: [24, 15] - [24, 23] + s: '⚠️' + prefix: 'f"' + implicitly_concatenated_parts: None + ] + Assign: [25, 0] - [25, 37] + targets: [ + Name: [25, 0] - [25, 12] + variable: Variable('concatenated', None) + ctx: Store + ] + value: + Str: [25, 15] - [25, 37] + s: '⚠️👨\u200d💻' + prefix: '"' + implicitly_concatenated_parts: [ + StringPart: [25, 15] - [25, 23] + prefix: '"' + text: '"⚠️"' + s: '⚠️' + StringPart: [25, 24] - [25, 37] + prefix: '"' + text: '"👨\u200d💻"' + s: '👨\u200d💻' + ] + Assign: [28, 0] - [28, 17] + targets: [ + Name: [28, 0] - [28, 1] + variable: Variable('d', None) + ctx: Store + ] + value: + Dict: [28, 4] - [28, 17] + items: [ + KeyValuePair: [28, 5] - [28, 16] + key: + Str: [28, 5] - [28, 13] + s: '⚠️' + prefix: '"' + implicitly_concatenated_parts: None + value: + Num: [28, 15] - [28, 16] + n: 1 + text: '1' + ] + ] diff --git a/python/extractor/tests/parser/unicode_escapes_new.py b/python/extractor/tests/parser/unicode_escapes_new.py new file mode 100644 index 000000000000..72cc435be7ef --- /dev/null +++ b/python/extractor/tests/parser/unicode_escapes_new.py @@ -0,0 +1,28 @@ +# Characters that Rust's `Debug` formatting escapes as `\u{...}` when `tsg-python` serialises the +# source text. See https://github.com/github/codeql/issues/22435. + +# PEP 695 syntax is what makes the old parser bail out and hand the file to `tsg-python` in the +# first place, so keep the reported reproducer intact. +type X = int + +# U+FE0F variation selector, next to a `%` directive. +warn = "\u26a0\ufe0f problem %s: %s" +warn_raw = "⚠️ problem %s: %s" + +# U+200D zero width joiner. +zwj = "👨‍💻" + +# Combining acute accent (NFD), and a soft hyphen. +nfd = "café" +soft_hyphen = "soft­hyphen" + +# Combining marks are valid in identifiers too. +café = nfd + +# In f-strings, raw strings and implicit concatenations too. +raw = r"⚠️\u{fe0f}" +joined = f"{zwj}⚠️" +concatenated = "⚠️" "👨‍💻" + +# ... and outside of string literals. +d = {"⚠️": 1} # comment with ⚠️ and 👨‍💻 diff --git a/python/extractor/tests/test_tsg_parser.py b/python/extractor/tests/test_tsg_parser.py new file mode 100644 index 000000000000..b4df0007e905 --- /dev/null +++ b/python/extractor/tests/test_tsg_parser.py @@ -0,0 +1,85 @@ +import unittest + +from ast import literal_eval + +from semmle.logging import format_message +from semmle.python.parser.tsg_parser import evaluate_string, rust_to_python_escapes + + +class RustEscapeTest(unittest.TestCase): + """`tsg-python` serialises strings with Rust's `Debug` formatting, which escapes characters such + as U+FE0F as `\\u{...}` -- a syntax Python's `literal_eval` does not accept -- and NUL as `\\0`, + which Python reads as an octal escape.""" + + def test_untouched_without_escapes(self): + text = '"caf\u00e9 \u2713 \U0001f4be"' + self.assertEqual(rust_to_python_escapes(text), text) + + def test_basic_multilingual_plane(self): + self.assertEqual(rust_to_python_escapes(r'"\u{fe0f}"'), r'"\ufe0f"') + self.assertEqual(rust_to_python_escapes(r'"\u{200d}"'), r'"\u200d"') + + def test_short_and_astral_code_points(self): + self.assertEqual(rust_to_python_escapes(r'"\u{0}"'), r'"\u0000"') + self.assertEqual(rust_to_python_escapes(r'"\u{1f4a9}"'), r'"\U0001f4a9"') + + def test_other_escapes_are_preserved(self): + self.assertEqual(rust_to_python_escapes(r'"a\nb\"c\u{ad}"'), r'"a\nb\"c\u00ad"') + + def test_escaped_backslash_is_not_an_escape_introducer(self): + # How a raw string `r"\u{fe0f}"` in the analysed source gets serialised: the `\u{fe0f}` is + # literal text, not an escape, and must survive unchanged. + self.assertEqual(rust_to_python_escapes(r'"\\u{fe0f}"'), r'"\\u{fe0f}"') + + def test_nul_is_not_left_as_an_octal_escape(self): + # Rust renders NUL as `\0`; Python would read that as the start of an octal escape and + # swallow the digits that follow, decoding `"\01"` to U+0001 instead of NUL then `1`. + self.assertEqual(rust_to_python_escapes(r'"\01"'), r'"\x001"') + + def test_every_escape_shape_round_trips(self): + # Rust's `Debug for str` only ever emits these escape shapes. Check that each round-trips + # with every printable ASCII neighbour before and after it. + for escape_shape, expected in [ + (r'\0', "\x00"), + (r'\t', "\t"), + (r'\n', "\n"), + (r'\r', "\r"), + (r'\\', "\\"), + (r'\"', '"'), + (r'\u{1}', "\u0001"), + (r'\u{1f}', "\u001f"), + (r'\u{300}', "\u0300"), + (r'\u{fe0f}', "\ufe0f"), + (r'\u{e0100}', "\U000e0100"), + (r'\u{10fffe}', "\U0010fffe"), + ]: + for neighbour in map(chr, range(0x20, 0x7F)): + rendered_neighbour = {"\\": r"\\", '"': r'\"'}.get(neighbour, neighbour) + for position, text, expected_value in [ + ("before", '"' + rendered_neighbour + escape_shape + '"', neighbour + expected), + ("after", '"' + escape_shape + rendered_neighbour + '"', expected + neighbour), + ]: + with self.subTest( + escape_shape=escape_shape, + neighbour=neighbour, + position=position, + ): + self.assertEqual(literal_eval(rust_to_python_escapes(text)), expected_value) + + def test_evaluate_string_on_reported_value(self): + # The exact value from https://github.com/github/codeql/issues/22435 that used to raise + # `truncated \uXXXX escape`. + value = rust_to_python_escapes('"\\"\u26a0\\u{fe0f} problem %s: %s\\""') + self.assertEqual(evaluate_string(value), "\u26a0\ufe0f problem %s: %s") + + +class FormatMessageTest(unittest.TestCase): + """A pre-formatted log message may contain `%` directives coming from the analysed source, and + must not be `%`-formatted again.""" + + def test_no_arguments(self): + message = "Error while parsing value '%s: %s'" + self.assertEqual(format_message(message, ()), message) + + def test_with_arguments(self): + self.assertEqual(format_message("%s and %s", ("a", "b")), "a and b") diff --git a/python/extractor/tsg-python/src/main.rs b/python/extractor/tsg-python/src/main.rs index 6e07a8cd86e7..19b2f6dcfece 100644 --- a/python/extractor/tsg-python/src/main.rs +++ b/python/extractor/tsg-python/src/main.rs @@ -710,6 +710,9 @@ fn main() -> Result<()> { add_syntax_error_nodes(&mut graph, &syntax_errors); } + // `pretty_print` renders string values with Rust's `Debug` formatting, so non-printable and + // grapheme-extending characters come out as `\u{...}`. The reader on the other side + // (`semmle/python/parser/tsg_parser.py`) translates those into Python escapes. print!("{}", graph.pretty_print()); Ok(()) } diff --git a/python/ql/lib/change-notes/2026-08-27-tsg-parser-unicode-escapes.md b/python/ql/lib/change-notes/2026-08-27-tsg-parser-unicode-escapes.md new file mode 100644 index 000000000000..56b70081d529 --- /dev/null +++ b/python/ql/lib/change-notes/2026-08-27-tsg-parser-unicode-escapes.md @@ -0,0 +1,4 @@ +--- +category: fix +--- +* Fixed a bug where a Python file could be silently dropped from the analysis (with a spurious "A parse error occurred" diagnostic) when it contained a string literal, comment, or identifier with a character such as the U+FE0F emoji variation selector, a U+200D zero width joiner, or a combining accent.