{"thread":{"id":"65909","subject":"[PATCH] apply: avoid leaking abandoned git-header state","startedAt":"2026-07-02T04:18:05Z","lastAt":"2026-08-26T20:31:12Z","messageCount":2,"participants":["Zephyr Yao","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"546955","messageId":"20260702041759.51572-1-zhihao.yao@njit.edu","threadId":"65909","inReplyTo":null,"subject":"[PATCH] apply: avoid leaking abandoned git-header state","fromName":"Zephyr Yao","fromEmail":"zot.zot.yao@gmail.com","sentAt":"2026-07-02T04:17:59Z","receivedAt":"2026-07-02T04:18:05Z","isPatch":true,"body":"When find_header() sees a \"diff --git\" line, it calls\nparse_git_diff_header() to parse the git-style extended header. That parser\nupdates the caller's struct patch as it goes, filling in the default name,\nold/new names, and new/delete state.\n\nBut not every \"diff --git\" line found while scanning is ultimately accepted\nas the patch header. If parse_git_diff_header() returns a length that covers\nonly the \"diff --git\" line, find_header() continues scanning for another\nheader. In that case the partially parsed git-header state must not interfere\nwith the later traditional \"---\" / \"+++\" header.\n\nLeaving that state behind can combine incompatible metadata from the\nabandoned git header and the later traditional header. For example, after:\n\n\tdiff --git a/foo b/foo\n\n\t--- /dev/null\n\t+++ b/foo\n\t@@ -0,0 +1 @@\n\t+x\n\nthe abandoned git header can leave an old name in the patch, while the\ntraditional header marks the patch as creating a new file. That impossible\nstate later trips the check_preimage() assertion that a creation patch should\nnot have a preimage.\n\nParse a candidate git header into a temporary patch and line number. Commit\nthat temporary state to the real patch only when the git header is actually\naccepted; otherwise release it and keep scanning with the original patch\nstate unchanged.\n\nAlso reject an empty parsed default name from the \"diff --git\" line.\nAn empty patch->def_name is not a valid pathname, and should not be\nused later as a fallback when old_name and new_name are missing.\n\nAdd regression tests for both the empty default-name case and the non-empty\nabandoned-header case above.\n\nCo-authored-by: Mahya SamDaliri <ms3539@njit.edu>\nSigned-off-by: Mahya SamDaliri <ms3539@njit.edu>\nCo-authored-by: Haotian Zhang <haotian.zhang@njit.edu>\nSigned-off-by: Haotian Zhang <haotian.zhang@njit.edu>\nCo-authored-by: Martin Kellogg <martin.kellogg@njit.edu>\nSigned-off-by: Martin Kellogg <martin.kellogg@njit.edu>\nSigned-off-by: Zephyr Yao <zhihao.yao@njit.edu>\n---\n apply.c               | 29 ++++++++++++++++++++++-------\n t/t4100-apply-stat.sh | 25 +++++++++++++++++++++++++\n 2 files changed, 47 insertions(+), 7 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 5e54453..2ce9b6a 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -1362,6 +1362,9 @@ int parse_git_diff_header(struct strbuf *root,\n \t * the default name from the header.\n \t */\n \tpatch->def_name = git_header_name(p_value, line, len);\n+\tif (patch->def_name && !*patch->def_name)\n+\t\tFREE_AND_NULL(patch->def_name);\n+\n \tif (patch->def_name && root->len) {\n \t\tchar *s = xstrfmt(\"%s%s\", root->buf, patch->def_name);\n \t\tfree(patch->def_name);\n@@ -1632,15 +1635,27 @@ static int find_header(struct apply_state *state,\n \t\t * or mode change, so we handle that specially\n \t\t */\n \t\tif (!memcmp(\"diff --git \", line, 11)) {\n-\t\t\tint git_hdr_len = parse_git_diff_header(&state->root,\n-\t\t\t\t\t\t\t\tstate->patch_input_file,\n-\t\t\t\t\t\t\t\t&state->linenr,\n-\t\t\t\t\t\t\t\tstate->p_value, line, len,\n-\t\t\t\t\t\t\t\tsize, patch);\n-\t\t\tif (git_hdr_len < 0)\n+\t\t\tstruct patch git_patch = { 0 };\n+\t\t\tint git_linenr = state->linenr;\n+\t\t\tint git_hdr_len;\n+\n+\t\t\tgit_patch.inaccurate_eof = patch->inaccurate_eof;\n+\t\t\tgit_patch.recount = patch->recount;\n+\t\t\tgit_hdr_len = parse_git_diff_header(&state->root,\n+\t\t\t\t\t\t\t    state->patch_input_file,\n+\t\t\t\t\t\t\t    &git_linenr,\n+\t\t\t\t\t\t\t    state->p_value, line, len,\n+\t\t\t\t\t\t\t    size, &git_patch);\n+\t\t\tif (git_hdr_len < 0) {\n+\t\t\t\trelease_patch(&git_patch);\n \t\t\t\treturn -128;\n-\t\t\tif (git_hdr_len <= len)\n+\t\t\t}\n+\t\t\tif (git_hdr_len <= len) {\n+\t\t\t\trelease_patch(&git_patch);\n \t\t\t\tcontinue;\n+\t\t\t}\n+\t\t\t*patch = git_patch;\n+\t\t\tstate->linenr = git_linenr;\n \t\t\t*hdrsize = git_hdr_len;\n \t\t\treturn offset;\n \t\t}\ndiff --git a/t/t4100-apply-stat.sh b/t/t4100-apply-stat.sh\nindex 8393076..d3406ed 100755\n--- a/t/t4100-apply-stat.sh\n+++ b/t/t4100-apply-stat.sh\n@@ -113,6 +113,31 @@ test_expect_success 'applying a patch with a missing filename reports the input'\n \ttest_cmp expect err\n '\n \n+test_expect_success 'empty default filename reports the input' '\n+\tcat >empty-name.patch <<-\\EOF &&\n+\tdiff --git \"a/\"\"b/\"\n+\n+\t--- /dev/null\n+\t+++ \"\n+\t@@ -0,0 +1 @@\n+\t+\n+\tEOF\n+\ttest_must_fail git apply empty-name.patch 2>err &&\n+\ttest_grep \"git diff header lacks filename information\" err\n+'\n+\n+test_expect_success 'abandoned git header does not reuse names' '\n+\tcat >abandoned-git-header.patch <<-\\EOF &&\n+\tdiff --git a/foo b/foo\n+\n+\t--- /dev/null\n+\t+++ b/foo\n+\t@@ -0,0 +1 @@\n+\t+x\n+\tEOF\n+\tgit apply --check abandoned-git-header.patch\n+'\n+\n test_expect_success 'applying a patch with an invalid mode reports the input' '\n \tcat >mode.patch <<-\\EOF &&\n \tdiff --git a/f b/f\n-- \n2.47.0\n"},{"id":"551313","messageId":"xmqqtsogei9d.fsf@gitster.g","threadId":"65909","inReplyTo":"20260702041759.51572-1-zhihao.yao@njit.edu","subject":"Re: [PATCH] apply: avoid leaking abandoned git-header state","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-08-26T20:31:10Z","receivedAt":"2026-08-26T20:31:12Z","isPatch":true,"body":"Zephyr Yao <zot.zot.yao@gmail.com> writes:\n\n> When find_header() sees a \"diff --git\" line, it calls\n> parse_git_diff_header() to parse the git-style extended header. That parser\n> updates the caller's struct patch as it goes, filling in the default name,\n> old/new names, and new/delete state.\n>\n> But not every \"diff --git\" line found while scanning is ultimately accepted\n> as the patch header. If parse_git_diff_header() returns a length that covers\n> only the \"diff --git\" line, find_header() continues scanning for another\n> header. In that case the partially parsed git-header state must not interfere\n> with the later traditional \"---\" / \"+++\" header.\n\nThis patch has gathered no response.  Perhaps the e-mail received no\nreply because it was sent in early July around the holiday, or\nperhaps nobody was interested in the topic.  In any case, I am\ncleaning up the \"What's cooking\" report and noticed that this has\nbeen in the \"Needs review\" state for a long time.\n\nSo I took a look.\n\nThese cross checks are primarily sanity checks.  Having the parser\nnotice a discrepancy and abort is a good thing.  The user is\nsupposed to inspect the situation and fix a malformed patch (such\nas one containing a stray 'diff --git' header unrelated to the\nactual patch).\n\nSo I do not think we want to apply this patch.\n\nThanks.\n"}]}