patch_parse: fix parsing addition/deletion of file with space

The diff header format is a strange beast in that it is inherently unparseable in an unambiguous way. While parsing a/file.txt b/file.txt is obvious and trivially doable, parsing a diff header of a/file b/file ab.txt b/file b/file ab.txt is not (but in fact valid and created by git.git). Due to that, we have relaxed our diff header parser in commit 80226b5f (patch_parse: allow parsing ambiguous patch headers, 2017-09-22), so that we started to bail out when seeing diff headers with spaces in their file names. Instead, we try to use the "---" and "+++" lines, which are unambiguous. In some cases, though, we neither have a useable file name from the header nor from the "---" or "+++" lines. This is the case when we have a deletion or addition of a file with spaces: the header is unparseable and the other lines will simply show "/dev/null". This trips our parsing logic when we try to extract the prefix (the "a/" part) that is being used in the path line, where we unconditionally try to dereference a NULL pointer in such a scenario. We can fix this by simply not trying to parse the prefix in cases where we have no useable path name. That'd leave the parsed patch without either `old_prefix` or `new_prefix` populated. But in fact such cases are already handled by users of the patch object, which simply opt to use the default prefixes in that case.

patch_parse: fix parsing addition/deletion of file with space
The diff header format is a strange beast in that it is inherently unparseable in an unambiguous way. While parsing a/file.txt b/file.txt is obvious and trivially doable, parsing a diff header of a/file b/file ab.txt b/file b/file ab.txt is not (but in fact valid and created by git.git). Due to that, we have relaxed our diff header parser in commit 80226b5f (patch_parse: allow parsing ambiguous patch headers, 2017-09-22), so that we started to bail out when seeing diff headers with spaces in their file names. Instead, we try to use the "---" and "+++" lines, which are unambiguous. In some cases, though, we neither have a useable file name from the header nor from the "---" or "+++" lines. This is the case when we have a deletion or addition of a file with spaces: the header is unparseable and the other lines will simply show "/dev/null". This trips our parsing logic when we try to extract the prefix (the "a/" part) that is being used in the path line, where we unconditionally try to dereference a NULL pointer in such a scenario. We can fix this by simply not trying to parse the prefix in cases where we have no useable path name. That'd leave the parsed patch without either `old_prefix` or `new_prefix` populated. But in fact such cases are already handled by users of the patch object, which simply opt to use the default prefixes in that case.
96b6fe11 · Patrick Steinhardt · c79f9dae · 96b6fe11
Commit 96b6fe11 authored Mar 29, 2019 by Patrick Steinhardt
Hide whitespace changes
Inline Side-by-side

Showing with 2 additions and 2 deletions

src/patch_parse.c
+2 -2

No files found.
--- a/src/patch_parse.c
+++ b/src/patch_parse.c
@@ -928,8 +928,8 @@ static int check_filenames(git_patch_parsed *patch)
 	prefixed_old = (!added && patch->old_path) ? patch->old_path : patch->header_old_path;
 	prefixed_new = (!deleted && patch->new_path) ? patch->new_path : patch->header_new_path;
-	if (check_prefix(&patch->old_prefix, &old_prefixlen, patch, prefixed_old) < 0 ||
+	if ((prefixed_old && check_prefix(&patch->old_prefix, &old_prefixlen, patch, prefixed_old) < 0) ||
-	    check_prefix(&patch->new_prefix, &new_prefixlen, patch, prefixed_new) < 0)
+	    (prefixed_new && check_prefix(&patch->new_prefix, &new_prefixlen, patch, prefixed_new) < 0))
 		return -1;
 	/* Prefer the rename filenames as they are unambiguous and unprefixed */