DiffParser silently drops or misattributes diffs for non-ASCII / space-containing file paths

[DevPatils]

Description

application/utils/harvester/diff_parser.py's DiffParser.parse() extracts changed files from a git diff using a regex on the diff --git a/<path> b/<path> header line: match = re.match(r"diff --git a/(.+?) b/", line) then current_file = match.group(1) if match else None.

This regex makes two incorrect assumptions about real-world git diff output, causing two distinct failure modes: silent data loss for non-ASCII filenames, and silent path corruption for filenames containing " b/".

Reproduction

I wrote a small, self-contained script that creates a real temporary git repository, commits real files, generates a real git diff from it, and runs that through the actual DiffParser class — no mocked/synthetic diff text, just real git output.

Bug reproduction video:

Bug.mp4

Expected behavior

• Bug A: should return 1 DiffBlock with file_path='café.md' and added_lines=['added line']
• Bug B: should return 1 DiffBlock with file_path='foo b/bar.md'

Actual behavior

• Bug A: returns 0 blocks — the entire change is silently discarded, no error or warning
• Bug B: returns 1 block, but with file_path truncated to 'foo', silently mis-attributing the change to a nonexistent path

Root cause

The regex in diff_parser.py:39 assumes the header always starts with a literal a/ and that the first " b/" substring found is the real separator. Neither assumption holds:

• Git C-quotes (wraps in "...", escapes non-ASCII bytes) any path with non-ASCII characters by default (core.quotePath=true is git's default), so the header doesn't start with a literal a/ at all in that case — the regex simply fails to match.
• The non-greedy match stops at the first " b/" substring in the line, which is ambiguous whenever the real file path itself contains that exact substring.

Existing tests in application/tests/harvester_test/diff_parser_test.py only cover plain ASCII, single-word filenames with no path-quoting or embedded " b/" — this class of edge case is completely untested.

Impact

The harvester watches external standard repositories for changes so OpenCRE can stay in sync. A real update to a file with a non-ASCII name (plausible for translated/localized docs in an international project) would be silently missed entirely — no error, no log, just as if the change never happened. A file whose path contains " b/" would have its change detected but filed under the wrong document identity, corrupting downstream traceability.

Suggested fix direction

Parse the file path from the --- a/<path> / +++ b/<path> lines instead of the diff --git header line — git provides these separately per side, avoiding the ambiguity of a single combined header line. Would also need proper un-quoting of git's C-quoted path format. Happy to open a PR for this.

[DevPatils]

@northdpole please review it so that I can start working on this.

[northdpole]

Maintainer triage (substance): confirmed real, Module A relevant.

DiffParser uses re.match(r"diff --git a/(.+?) b/", line). That fails for:

1. Quoted/C-escaped paths — git quotes non-ASCII / unusual filenames on diff --git headers, so the regex never matches and the file’s added lines are dropped or attributed wrong.
2. Paths containing b/ — non-greedy .+? stops early and corrupts file_path (obscure but real).

Acknowledged — open for a PR. Prefer parsing via +++ b/<path> / --- a/<path> headers (with unquote) rather than only the diff --git line, plus a regression test with a real git diff containing a non-ASCII path.

[Deepnar]

Hi @northdpole and @DevPatils — I’d like to contribute to #1032.

I saw that DevPatils previously expressed interest in working on this issue, so I wanted to check before creating a branch or PR. If that work is no longer active, I can take the focused parser and regression-test change described in the issue. Otherwise, I’m happy to coordinate or contribute a separate part.

My intended scope is limited to robustly parsing Git’s ---/+++ path headers, handling quoted non-ASCII paths and paths containing “ b/”, and adding local regression tests without changing unrelated harvesting behavior.

[DevPatils]

Hi @northdpole and @DevPatils — I’d like to contribute to #1032.

I saw that DevPatils previously expressed interest in working on this issue, so I wanted to check before creating a branch or PR. If that work is no longer active, I can take the focused parser and regression-test change described in the issue. Otherwise, I’m happy to coordinate or contribute a separate part.

My intended scope is limited to robustly parsing Git’s ---/+++ path headers, handling quoted non-ASCII paths and paths containing “ b/”, and adding local regression tests without changing unrelated harvesting behavior.

Hey @Deepnar am on it brother, I'll push this change today. Try solving another issue.