Skip to content

Commit ffe5c33

Browse files
gpudemoMahya SamDaliriHaotian Zhangkelloggm
authored andcommitted
apply: avoid leaking abandoned git-header state
When find_header() sees a "diff --git" line, it calls parse_git_diff_header() to parse the git-style extended header. That parser updates the caller's struct patch as it goes, filling in the default name, old/new names, and new/delete state. But not every "diff --git" line found while scanning is ultimately accepted as the patch header. If parse_git_diff_header() returns a length that covers only the "diff --git" line, find_header() continues scanning for another header. In that case the partially parsed git-header state must not interfere with the later traditional "---" / "+++" header. Leaving that state behind can combine incompatible metadata from the abandoned git header and the later traditional header. For example, after: diff --git a/foo b/foo --- /dev/null +++ b/foo @@ -0,0 +1 @@ +x the abandoned git header can leave an old name in the patch, while the traditional header marks the patch as creating a new file. That impossible state later trips the check_preimage() assertion that a creation patch should not have a preimage. Parse a candidate git header into a temporary patch and line number. Commit that temporary state to the real patch only when the git header is actually accepted; otherwise release it and keep scanning with the original patch state unchanged. Also reject an empty parsed default name from the "diff --git" line. An empty patch->def_name is not a valid pathname, and should not be used later as a fallback when old_name and new_name are missing. Add regression tests for both the empty default-name case and the non-empty abandoned-header case above. Co-authored-by: Mahya SamDaliri <ms3539@njit.edu> Signed-off-by: Mahya SamDaliri <ms3539@njit.edu> Co-authored-by: Haotian Zhang <haotian.zhang@njit.edu> Signed-off-by: Haotian Zhang <haotian.zhang@njit.edu> Co-authored-by: Martin Kellogg <martin.kellogg@njit.edu> Signed-off-by: Martin Kellogg <martin.kellogg@njit.edu> Signed-off-by: Zephyr Yao <zhihao.yao@njit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 94f0577 commit ffe5c33

2 files changed

Lines changed: 47 additions & 7 deletions

File tree

apply.c

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,6 +1362,9 @@ int parse_git_diff_header(struct strbuf *root,
13621362
* the default name from the header.
13631363
*/
13641364
patch->def_name = git_header_name(p_value, line, len);
1365+
if (patch->def_name && !*patch->def_name)
1366+
FREE_AND_NULL(patch->def_name);
1367+
13651368
if (patch->def_name && root->len) {
13661369
char *s = xstrfmt("%s%s", root->buf, patch->def_name);
13671370
free(patch->def_name);
@@ -1632,15 +1635,27 @@ static int find_header(struct apply_state *state,
16321635
* or mode change, so we handle that specially
16331636
*/
16341637
if (!memcmp("diff --git ", line, 11)) {
1635-
int git_hdr_len = parse_git_diff_header(&state->root,
1636-
state->patch_input_file,
1637-
&state->linenr,
1638-
state->p_value, line, len,
1639-
size, patch);
1640-
if (git_hdr_len < 0)
1638+
struct patch git_patch = { 0 };
1639+
int git_linenr = state->linenr;
1640+
int git_hdr_len;
1641+
1642+
git_patch.inaccurate_eof = patch->inaccurate_eof;
1643+
git_patch.recount = patch->recount;
1644+
git_hdr_len = parse_git_diff_header(&state->root,
1645+
state->patch_input_file,
1646+
&git_linenr,
1647+
state->p_value, line, len,
1648+
size, &git_patch);
1649+
if (git_hdr_len < 0) {
1650+
release_patch(&git_patch);
16411651
return -128;
1642-
if (git_hdr_len <= len)
1652+
}
1653+
if (git_hdr_len <= len) {
1654+
release_patch(&git_patch);
16431655
continue;
1656+
}
1657+
*patch = git_patch;
1658+
state->linenr = git_linenr;
16441659
*hdrsize = git_hdr_len;
16451660
return offset;
16461661
}

t/t4100-apply-stat.sh

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,31 @@ test_expect_success 'applying a patch with a missing filename reports the input'
113113
test_cmp expect err
114114
'
115115

116+
test_expect_success 'empty default filename reports the input' '
117+
cat >empty-name.patch <<-\EOF &&
118+
diff --git "a/""b/"
119+
120+
--- /dev/null
121+
+++ "
122+
@@ -0,0 +1 @@
123+
+
124+
EOF
125+
test_must_fail git apply empty-name.patch 2>err &&
126+
test_grep "git diff header lacks filename information" err
127+
'
128+
129+
test_expect_success 'abandoned git header does not reuse names' '
130+
cat >abandoned-git-header.patch <<-\EOF &&
131+
diff --git a/foo b/foo
132+
133+
--- /dev/null
134+
+++ b/foo
135+
@@ -0,0 +1 @@
136+
+x
137+
EOF
138+
git apply --check abandoned-git-header.patch
139+
'
140+
116141
test_expect_success 'applying a patch with an invalid mode reports the input' '
117142
cat >mode.patch <<-\EOF &&
118143
diff --git a/f b/f

0 commit comments

Comments
 (0)