fix(search): use NUL-separated filelist + xargs -0 for paths with spaces#693
fix(search): use NUL-separated filelist + xargs -0 for paths with spaces#693lg320531124 wants to merge 2 commits into
Conversation
d0a0c48 to
8954b9b
Compare
|
Huge thanks for opening this PR and for the work you put into it. The maintainer shop is currently full, so this may sit for a bit before it gets a proper review. We will come back to this as soon as possible with real feedback; I wanted to make sure it did not sit unacknowledged in the meantime. |
When a repo path contains spaces (e.g. /Users/name/My Project/), the scoped search pipeline breaks because: 1. write_scoped_filelist() writes newline-separated paths → xargs splits on spaces, passing truncated paths to grep 2. xargs has no -0 flag, so it can't handle NUL separators Fix: write NUL-separated paths in write_scoped_filelist() via fwrite(), and add -0 flag to xargs in build_grep_cmd(). Non-scoped mode was already safe (root_path is single-quoted in the grep command). Closes DeusData#687 Signed-off-by: lg320531124 <lg320531124@users.noreply.github.com>
…tent
The NUL-separated filelist from the previous commit breaks the Windows
search path: PowerShell 'Get-Content | ForEach-Object { Select-String
-LiteralPath src/mcp/mcp.c }' reads line-by-line and rejects NUL bytes with
'Illegal characters in path', making search_code return empty and
failing test_mcp.c:1098 (search_code_multi_word ASSERT HandleRequest).
Windows never needed NUL — Get-Content feeds each path as one line to
-LiteralPath, which accepts spaces without shell word-splitting. NUL was
only needed on Unix for 'xargs -0' to survive spaces/newlines in paths.
Branch on _WIN32: '\n' on Windows (PowerShell line reader), '\0' on
Unix (xargs -0). Fixes the CI regression introduced while fixing DeusData#687.
Signed-off-by: lg320531124 <lg320531124@users.noreply.github.com>
8954b9b to
f2ff630
Compare
|
Thank you @lg320531124 — you correctly root-caused #687, and the fix concept is exactly right: a newline-separated filelist + plain One issue kept it from merging as-is: the filelist writer builds each record in a fixed I've distilled your fix into #751 ( Closing as superseded by the distilled PR, with full credit to you. Really appreciate the correct diagnosis. 🙏 |
search_code's scoped grep writes an "<root>/<file>" filelist and pipes it to xargs. With a newline-separated filelist + plain `xargs`, a space anywhere in the absolute path splits one file into several bogus arguments, so grep finds nothing and scoped search returns 0 matches for any repo indexed under a path containing a space (DeusData#687). Fix: write NUL-separated records and consume them with `xargs -0`, so each path stays a single argument regardless of spaces. On Windows the scoped path uses PowerShell `Get-Content | Select-String -LiteralPath`, which reads line-by-line and already handles spaces, so the filelist keeps newline separators there. write_scoped_filelist writes each record piece-by-piece with fwrite/fputc (no fixed-size stack buffer), so arbitrarily long absolute paths cannot overflow. Reproduce-first: search_code_scoped_path_with_spaces_issue687 indexes a repo rooted at "<tmp>/my project" and asserts total_grep_matches > 0 (RED on the old newline + plain-xargs code -> 0 matches; GREEN after). Full suite green. Distilled from DeusData#693 (thanks @lg320531124). Keeps that PR's correct NUL/xargs-0 idea and its Windows newline branch, but replaces its char buf[2048] + unbounded memcpy (a stack buffer overflow reachable via long indexed paths, PATH_MAX 4096) with the direct piece-by-piece write, and adds the reproduce-first guard it lacked. Closes DeusData#687 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com> Co-Authored-By: lg320531124 <lg320531124@users.noreply.github.com>
Problem
search_codereturns 0 matches when the indexed repo path contains spaces (e.g./Users/name/My Project/).Root cause:
write_scoped_filelist()writes newline-separated paths, andxargssplits on whitespace — so a path like/tmp/test repo/src/main.cbecomes two arguments:/tmp/testandrepo/src/main.c.Fix
write_scoped_filelist(): write NUL-separated paths viafwrite()instead offprintf()with newline separatorsbuild_grep_cmd(): add-0flag toxargsso it reads NUL-terminated entriesNon-scoped mode was already safe (
root_pathis single-quoted in the grep command:grep -rn ... '%s').Verification
Indexed and searched a repo at
/tmp/test repo/—search_codenow returns correct results:{"results":[{"node":"main","qualified_name":"tmp-test-repo.src.main.main","label":"Function","file":"src/main.c","start_line":2,"end_line":5,"match_lines":[3]}],"total_grep_matches":1,"total_results":1}Previously: 0 matches.
Closes #687