Skip to content

Commit 5ea0176

Browse files
committed
apply: parse names out of "diff --git" more carefully
"git apply" uses the pathname parsed out of the "diff --git" header to decide which path is being patched, but this is used only when there is no other names available in the patch. When there is any content change (like we can see in this patch, that modifies the contents of "apply.c") or rename (which comes with "rename from" and "rename to" extended diff headers), the names are available without having to parse this header. When we do need to parse this header, a special care needs to be taken, as the name of a directory or a file can have a SP in it so it is not like "find a space, and take everything before the space and that is the preimage filename, everything after the space is the postimage filename". We have a loop that stops at every SP on the "diff --git a/dir/file b/dir/foo" line and see if that SP is the right place that separates such a pair of names. Unfortunately, this loop can terminate prematurely when a crafted directory name ended with a SP. The next pathname component after that SP (i.e. the beginning of the possible postimage filename) will be a slash, and instead of rejecting that position as the valid separation point between pre- and post-image filenames and keep looping, we stopped processing right there. The fix is simple. Instead of stopping and giving up, keep going on when we see such a condition. Reported-by: Han Young <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 0d1bd1d commit 5ea0176

File tree

2 files changed

+30
-1
lines changed

2 files changed

+30
-1
lines changed

apply.c

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1277,8 +1277,15 @@ static char *git_header_name(int p_value,
12771277
return NULL; /* no postimage name */
12781278
second = skip_tree_prefix(p_value, name + len + 1,
12791279
line_len - (len + 1));
1280+
/*
1281+
* If we are at the SP at the end of a directory,
1282+
* skip_tree_prefix() may return NULL as that makes
1283+
* it appears as if we have an absolute path.
1284+
* Keep going to find another SP.
1285+
*/
12801286
if (!second)
1281-
return NULL;
1287+
continue;
1288+
12821289
/*
12831290
* Does len bytes starting at "name" and "second"
12841291
* (that are separated by one HT or SP we just

t/t4126-apply-empty.sh

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,4 +66,26 @@ test_expect_success 'apply --index create' '
6666
git diff --exit-code
6767
'
6868

69+
test_expect_success 'apply with no-contents and a funny pathname' '
70+
mkdir "funny " &&
71+
>"funny /empty" &&
72+
git add "funny /empty" &&
73+
git diff HEAD "funny /" >sample.patch &&
74+
git diff -R HEAD "funny /" >elpmas.patch &&
75+
git reset --hard &&
76+
rm -fr "funny " &&
77+
78+
git apply --stat --check --apply sample.patch &&
79+
test_must_be_empty "funny /empty" &&
80+
81+
git apply --stat --check --apply elpmas.patch &&
82+
test_path_is_missing "funny /empty" &&
83+
84+
git apply -R --stat --check --apply elpmas.patch &&
85+
test_must_be_empty "funny /empty" &&
86+
87+
git apply -R --stat --check --apply sample.patch &&
88+
test_path_is_missing "funny /empty"
89+
'
90+
6991
test_done

0 commit comments

Comments
 (0)