From accf61c672e38104b3e26611ec3ebe0511e58848 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20Magyar?= Date: Sat, 11 Jul 2026 08:33:50 +0100 Subject: [PATCH] fix(tree-sitter): recover declarations after embedded NUL bytes (#2430) * fix(tree-sitter): recover from embedded NUL bytes Normalize embedded NUL bytes only in parser input so tree-sitter keeps recovering through the full source shape. Pass the file label through the worker for diagnostics and cover both direct-string and callback parse paths with regression tests. * fix(review): align safe-parser contract count * test(tree-sitter): cover worker NUL diagnostics (#2430) --- .../core/ingestion/workers/parse-worker.ts | 12 +- gitnexus/src/core/tree-sitter/safe-parse.ts | 32 ++++- gitnexus/test/integration/worker-pool.test.ts | 89 +++++++++++++ gitnexus/test/unit/safe-parse.test.ts | 120 ++++++++++++++++++ 4 files changed, 243 insertions(+), 10 deletions(-) diff --git a/gitnexus/src/core/ingestion/workers/parse-worker.ts b/gitnexus/src/core/ingestion/workers/parse-worker.ts index 0aafd58e7..bf0e19f07 100644 --- a/gitnexus/src/core/ingestion/workers/parse-worker.ts +++ b/gitnexus/src/core/ingestion/workers/parse-worker.ts @@ -1285,9 +1285,15 @@ const processFileGroup = ( let tree; try { - tree = parseSourceSafe(parser, parseContent, undefined, { - bufferSize: getTreeSitterBufferSize(parseContent), - }); + tree = parseSourceSafe( + parser, + parseContent, + undefined, + { + bufferSize: getTreeSitterBufferSize(parseContent), + }, + file.path, + ); } catch (err) { reportWarning( `Failed to parse file ${file.path}: ${err instanceof Error ? err.message : String(err)}`, diff --git a/gitnexus/src/core/tree-sitter/safe-parse.ts b/gitnexus/src/core/tree-sitter/safe-parse.ts index 0d53eaf9a..c1d39f9bc 100644 --- a/gitnexus/src/core/tree-sitter/safe-parse.ts +++ b/gitnexus/src/core/tree-sitter/safe-parse.ts @@ -181,7 +181,7 @@ export function getParseDiagnostics(tree: Parser.Tree): { /** * Parse `sourceText` safely on every platform. * - * This is the single "parse safely" entry point and its contract covers three + * This is the single "parse safely" entry point and its contract covers four * concerns: * * 1. **Windows crash workaround.** Inputs longer than 32 767 chars are fed @@ -201,8 +201,13 @@ export function getParseDiagnostics(tree: Parser.Tree): { * the tree is **returned anyway** — error recovery is a downgrade, never a * drop. Callers wanting the boolean use {@link parseHadErrors}. * - * @param label optional context (e.g. file path) attached to timeout errors - * and degraded-parse logs. Non-breaking trailing param. + * 4. **Embedded NUL recovery.** U+0000 is replaced with one ASCII space in + * the parser-only input. The one-for-one substitution keeps tree indices + * aligned with the original source while preventing language lexers from + * swallowing declarations during error recovery. + * + * @param label optional context (e.g. file path) attached to timeout errors, + * recovery warnings, and degraded-parse logs. Non-breaking trailing param. */ export function parseSourceSafe( parser: Parser, @@ -211,17 +216,30 @@ export function parseSourceSafe( options?: Parser.Options, label?: string, ): Parser.Tree { + let parserInput = sourceText; + if (sourceText.includes('\0')) { + let nullByteCount = 0; + parserInput = sourceText.replaceAll('\0', () => { + nullByteCount += 1; + return ' '; + }); + logger.warn( + { ...(label ? { file: label } : {}), nullByteCount }, + 'replaced embedded NUL bytes before tree-sitter parsing', + ); + } + const budgetMs = resolveParseTimeoutMs(); const armed = armParseBudget(parser, budgetMs); let tree: Parser.Tree | null; try { - if (sourceText.length <= DIRECT_PARSE_LIMIT_CHARS) { - tree = parser.parse(sourceText, oldTree, options); + if (parserInput.length <= DIRECT_PARSE_LIMIT_CHARS) { + tree = parser.parse(parserInput, oldTree, options); } else { const input: Parser.Input = (index) => { - if (index >= sourceText.length) return null; - return sourceText.slice(index, index + SAFE_PARSE_CHUNK_CHARS); + if (index >= parserInput.length) return null; + return parserInput.slice(index, index + SAFE_PARSE_CHUNK_CHARS); }; tree = parser.parse(input, oldTree, options); } diff --git a/gitnexus/test/integration/worker-pool.test.ts b/gitnexus/test/integration/worker-pool.test.ts index b8740957b..d9f6577a9 100644 --- a/gitnexus/test/integration/worker-pool.test.ts +++ b/gitnexus/test/integration/worker-pool.test.ts @@ -13,6 +13,7 @@ import { WorkerPoolDispatchError, } from '../../src/core/ingestion/workers/worker-pool.js'; import { pathToFileURL } from 'node:url'; +import { spawn } from 'node:child_process'; import path from 'node:path'; import fs from 'node:fs'; import os from 'node:os'; @@ -135,6 +136,94 @@ describe('worker pool integration', () => { expect(names).toContain('validateInput'); }); + it.skipIf(!hasDistWorker)( + 'includes the source path in embedded-NUL warnings', + async () => { + const filePath = 'src/NullByteDemo.java'; + const source = 'public interface Demo { /** embedded \0 */ void after(); }'; + // The worker logger writes directly to fd 2, so capture it at a child-process boundary. + const runner = ` + const { Worker } = require('node:worker_threads'); + const { pathToFileURL } = require('node:url'); + const worker = new Worker(pathToFileURL(${JSON.stringify(DIST_WORKER)})); + let dispatched = false; + worker.on('message', (message) => { + if (message && message.type === 'ready' && !dispatched) { + dispatched = true; + worker.postMessage({ + type: 'sub-batch', + files: [{ path: ${JSON.stringify(filePath)}, content: ${JSON.stringify(source)} }], + }); + } else if (message && message.type === 'sub-batch-done') { + process.stdout.write('SUB_BATCH_DONE\\n'); + } + }); + worker.on('error', (error) => { + process.stderr.write(String(error && error.stack ? error.stack : error)); + process.exit(1); + }); + `; + const child = spawn(process.execPath, ['--eval', runner], { + stdio: ['ignore', 'pipe', 'pipe'], + }); + let stdout = ''; + let stderr = ''; + + try { + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + reject( + new Error(`timed out waiting for parse worker; stdout=${stdout}; stderr=${stderr}`), + ); + }, 15_000); + let complete = false; + const finishIfComplete = (): void => { + if ( + !complete && + stdout.includes('SUB_BATCH_DONE') && + stderr.includes('replaced embedded NUL bytes before tree-sitter parsing') + ) { + complete = true; + clearTimeout(timeout); + resolve(); + } + }; + child.stdout.on('data', (chunk) => { + stdout += String(chunk); + finishIfComplete(); + }); + child.stderr.on('data', (chunk) => { + stderr += String(chunk); + finishIfComplete(); + }); + child.once('error', (error) => { + clearTimeout(timeout); + reject(error); + }); + child.once('exit', (code, signal) => { + if (!complete) { + clearTimeout(timeout); + reject(new Error(`parse worker exited early: code=${code}, signal=${signal}`)); + } + }); + }); + const warningLine = stderr + .split('\n') + .find((line) => line.includes('replaced embedded NUL bytes before tree-sitter parsing')); + if (!warningLine) throw new Error(`missing embedded-NUL warning in stderr: ${stderr}`); + expect(JSON.parse(warningLine)).toMatchObject({ + level: 40, + file: filePath, + nullByteCount: 1, + msg: 'replaced embedded NUL bytes before tree-sitter parsing', + }); + } finally { + child.kill(); + } + }, + 20_000, + ); + it.skipIf(!hasDistWorker)('parses multiple files across workers', async () => { const workerUrl = pathToFileURL(DIST_WORKER) as URL; pool = createWorkerPool(workerUrl, 2); diff --git a/gitnexus/test/unit/safe-parse.test.ts b/gitnexus/test/unit/safe-parse.test.ts index f6d025bc2..8d2b58552 100644 --- a/gitnexus/test/unit/safe-parse.test.ts +++ b/gitnexus/test/unit/safe-parse.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import Parser from 'tree-sitter'; +import Java from 'tree-sitter-java'; import Python from 'tree-sitter-python'; // Mock the logger so the throttled degraded-parse logs (emitted at `debug`, @@ -33,6 +34,22 @@ const makeParser = (): Parser => { return p; }; +const makeJavaParser = (): Parser => { + const parser = new Parser(); + parser.setLanguage(Java); + return parser; +}; + +const buildNullByteJavaSource = (paddingChars = 0): string => `public interface Demo { + void before(); + /**${'x'.repeat(paddingChars)} @example paramsMap={"dataStyle":"\0"} */ + String batchGetStructure(java.util.Map paramsMap); + void after0(); + void after1(); + void after2(); +} +`; + const buildSource = (chars: number, lineLen = 80): string => { const line = 'x = 1' + ' '.repeat(Math.max(0, lineLen - 6)) + '\n'; const lines = Math.ceil(chars / line.length); @@ -97,6 +114,109 @@ describe('parseSourceSafe', () => { }); }); +describe('parseSourceSafe — embedded NUL recovery (#2426)', () => { + afterEach(() => { + debugSpy.mockClear(); + warnSpy.mockClear(); + resetDegradedParseCounter(); + }); + + it.each([ + ['direct string', 0], + ['callback', 17_000], + ])('recovers all Java methods through the %s path', (_path, paddingChars) => { + const source = buildNullByteJavaSource(paddingChars); + const tree = parseSourceSafe( + makeJavaParser(), + source, + undefined, + undefined, + 'NullByteDemoService.java', + ); + const methods = tree.rootNode.descendantsOfType('method_declaration'); + + expect(tree.rootNode.hasError).toBe(false); + expect(tree.rootNode.endIndex).toBe(source.length); + expect(methods.map((method) => method.childForFieldName('name')?.text)).toEqual([ + 'before', + 'batchGetStructure', + 'after0', + 'after1', + 'after2', + ]); + expect(methods[2]?.childForFieldName('name')?.startIndex).toBe(source.indexOf('after0')); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + it.each([ + ['direct string', 'short\0source'], + ['callback', `${'x'.repeat(17_000)}\0source`], + ])('never exposes a NUL to the %s parser input', (_path, source) => { + let capturedInput: string | Parser.Input | undefined; + const stub = { + setTimeoutMicros: () => {}, + parse: (input: string | Parser.Input) => { + capturedInput = input; + return { rootNode: null } as unknown as Parser.Tree; + }, + } as unknown as Parser; + + parseSourceSafe(stub, source); + + if (typeof capturedInput === 'string') { + expect(capturedInput).not.toContain('\0'); + expect(capturedInput).toHaveLength(source.length); + } else { + expect(capturedInput).toBeTypeOf('function'); + let reconstructed = ''; + for (let index = 0; index < source.length; index += 16 * 1024) { + const chunk = capturedInput?.(index, { row: 0, column: index }); + expect(chunk).not.toContain('\0'); + reconstructed += chunk ?? ''; + } + expect(reconstructed).toHaveLength(source.length); + } + + expect(warnSpy).toHaveBeenCalledWith( + { nullByteCount: 1 }, + 'replaced embedded NUL bytes before tree-sitter parsing', + ); + }); + + it('reports all replacements with the supplied file label', () => { + const source = buildNullByteJavaSource().replace('after1', '\0after1'); + + parseSourceSafe(makeJavaParser(), source, undefined, undefined, 'src/Demo.java'); + + expect(warnSpy).toHaveBeenCalledOnce(); + expect(warnSpy).toHaveBeenCalledWith( + { file: 'src/Demo.java', nullByteCount: 2 }, + 'replaced embedded NUL bytes before tree-sitter parsing', + ); + }); + + it('keeps clean input on the existing path without a NUL warning', () => { + const source = buildNullByteJavaSource().replace('\0', ' '); + const tree = parseSourceSafe(makeJavaParser(), source, undefined, undefined, 'src/Demo.java'); + + expect(tree.rootNode.hasError).toBe(false); + expect(warnSpy).not.toHaveBeenCalled(); + }); + + it('does not consume the degraded-tree warning allowance', () => { + parseSourceSafe(makeJavaParser(), buildNullByteJavaSource()); + const parser = makeParser(); + const malformed = 'def broken(:\n return (1 + \n'; + + for (let index = 0; index < 20; index += 1) { + parseSourceSafe(parser, malformed); + } + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(debugSpy).toHaveBeenCalledTimes(20); + }); +}); + describe('parseSourceSafe — runaway-parse timeout (#1922)', () => { const ORIGINAL_BUDGET = process.env.GITNEXUS_PARSE_TIMEOUT_MS;