Git crashed on `git apply --check`

Thank you for filling out a Git bug report!
Please answer the following questions to help us understand your issue.

What did you do before the bug happened? (Steps to reproduce your issue) Checking the following patch:

b/tests/constraint_type_signature_lifting.test

new file mode 100644
index 0000000..0000000

@@ -0,0 +1,40 @@
+name: constraint types in signatures are lifted to generic parameters
+expect: pass
+stop: run
+errors: 0
+
+[source]
+constraint Readable = {
+    fn uint64 Readable.read(Readable *self);
+}
+
+type File = struct {
+    uint64 size;
+};
+
+fn uint64 File.read(File *self) {
+    return self.size;
+}
+
+fn Readable identity(Readable value) {
+    return value;
+}
+
+fn uint64 read_size(Readable file) {
+    return file.read();
+}
+
+fn int64 main() {
+    File file = {42};
+    File copy = identity(file);
+    if (copy.size != 42) return 1;
+    if (read_size(file) != 42) return 2;
+    return 0;
+}
+
+[stdout]
+
+[stderr]
+
+[exit]
+0

What did you expect to happen?
The patch check to pass

What happened instead?
"git: apply.c:3856: check_preimage: Assertion `patch->is_new <= 0' failed.i"

[System Info]
git version:
git version 2.54.0
cpu: x86_64
built from commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
sizeof-long: 8
sizeof-size_t: 8
shell-path: /bin/sh
rust: enabled
gettext: enabled
libcurl: 8.19.0
OpenSSL: OpenSSL 3.6.2 7 Apr 2026
zlib-ng: 2.3.3
SHA-1: SHA1_DC
SHA-256: SHA256_BLK
default-ref-format: files
default-hash: sha1
uname: Linux 7.0.10-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 23 May 2026 14:21:20 +0000 x86_64
compiler info: gnuc: 15.2
libc info: glibc: 2.43
$SHELL (typically, interactive shell): /usr/bin/bash

[Enabled Hooks]

Hi,

Thanks for the report. I was unable to reproduce it with the patch as quoted (the "diff --git" line is wrapped in your mail), but I can trigger the same assertion on current master, and on 2.43, when the "--- /dev/null" line comes before "new file mode" in the header:

    diff --git a/n.txt b/n.txt
    --- /dev/null
    +++ b/n.txt
    new file mode 100644
    @@ -0,0 +1 @@
    +hello

Was your patch handwritten or generated by a tool, and could you please check whether its header lines are in that order? If you can, please attach the exact file so that we can confirm it is the same problem.

Thanks,
Tanga

Adding the list back

Hello,

Thank you for sending the original patch file. The crash was caused by a blank line after the diff --git line in the new-file section, and it is reproducible on master. I am currently working on a fix.

Best,
Tanga

When a "diff --git" line is followed by a line that is not a git header line, such as a blank line or an indented "new file mode" line, parse_git_diff_header() stops right after the "diff --git" line. find_header() then treats it as an empty git header and keeps looking, as it has done since 206de27efa (git-apply: ignore empty git headers, 2005-06-12). It finds the "--- /dev/null" and "+++ b/<path>" lines and reads the rest as a traditional patch.

But parse_git_diff_header() has already set old_name and new_name from the "diff --git" line. These names stay in the patch. parse_traditional_patch() sets is_new for "--- /dev/null", but it does not clear old_name. So "git apply --check" dies on the assertion that 5c47f4c6e7 (builtin-apply: accept patch to an empty file, 2008-05-17) added to check_preimage():

check_preimage: Assertion `patch->is_new <= 0' failed.

The stale new_name causes the same crash when a deletion with such a header is applied with "git apply -R". The stale names also break an empty "diff --git" section that is followed by another section. They do not match the names in the next section, so "git apply" fails with a "bad git-diff" error.

Free the names when the "diff --git" line is not used. A new file is then created, and a deletion can be reversed. An empty section is ignored, as 206de27efa intended. As with any traditional patch, the "new file mode" line is not used, so a new file gets the default mode.

parse_git_diff_header() has also set is_toplevel_relative. Keep it, so that the paths stay relative to the top of the repository, as before.

---
This fixes the crash reported in [1]. While looking into it, I found two more ways to trigger the same assertion: an indented "new file mode" line, which this patch also fixes, and a "---" line before "new file mode", which 2/2 handles. Git 2.43 crashes on all three.

I based this on maint (v2.56.0), since the crash is in released versions. It merges cleanly with master, next and seen.

[1] https://lore.kernel.org/git/CALdwsC1cMKZgRPZA+pBaJe4PPs9q9W0K9CQLmM2bgUCyysTB-w@mail.gmail.com/

 apply.c                    | 10 ++++-
 t/t4133-apply-filenames.sh | 90 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+), 1 deletion(-)

index f00b7ba4d3..b56b363963 100644
@@ -1651,8 +1651,16 @@ static int find_header(struct apply_state *state,
 								size, patch);
 			if (git_hdr_len < 0)
 				return -128;
-			if (git_hdr_len <= len)
+			if (git_hdr_len <= len) {
+				/*
+				 * Not a git header after all. Do not keep the
+				 * names it took from the "diff --git" line.
+				 */
+				FREE_AND_NULL(patch->def_name);
+				FREE_AND_NULL(patch->old_name);
+				FREE_AND_NULL(patch->new_name);
 				continue;
+			}
 			*hdrsize = git_hdr_len;
 			return offset;
 		}

base-commit: a018953688f1b10bddf91bff8747068f5f4746a4

2.43.0

Git writes "new file mode" or "deleted file mode" before the "---" and "+++" lines. The "new file mode" line sets is_new. Then gitdiff_oldname() leaves old_name empty for "--- /dev/null".

When the "---" line comes first, is_new is still 0. So gitdiff_verify_name() takes the name on that line as old_name. For "--- /dev/null" this gives "dev/null", because one leading directory is stripped. The "new file mode" line then sets is_new. The result is a new file that also has an old name. "git apply --check" dies on it, on the same assertion in check_preimage(). In the same way, "+++ /dev/null" before "deleted file mode" gives a deleted file with a new name. "git apply -R" dies on that one.

d70e9c5c8c (apply: check git diffs for mutually exclusive header lines, 2017-06-27) made apply report other conflicting header lines, instead of leaving them to an assertion. Do the same here. After the header is read, reject a new file that has an old name. Reject a deleted file that has a new name too.

Patches made by Git are therefore not affected. A patch with "+++ /dev/null" before "deleted file mode" used to apply without -R. It is now rejected too.

---
I chose to reject these headers, like d70e9c5c8c does for other conflicting lines. Another option would be to accept "--- /dev/null" before "new file mode" as a new file, since "/dev/null" means no file there. I'm happy to change it if that is preferred.

 apply.c                    | 20 ++++++++++++++++++++
 t/t4133-apply-filenames.sh | 31 +++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)

index b56b363963..76cdacff33 100644
@@ -1466,6 +1466,26 @@ int parse_git_diff_header(struct strbuf *root,
 			      *linenr);
 		return -128;
 	}
+	if (patch->is_new > 0 && patch->old_name) {
+		if (patch_input_file)
+			error(_("git diff header for new file %s names an old file %s at %s:%d"),
+			      patch->new_name, patch->old_name,
+			      patch_input_file, *linenr);
+		else
+			error(_("git diff header for new file %s names an old file %s (line %d)"),
+			      patch->new_name, patch->old_name, *linenr);
+		return -128;
+	}
+	if (patch->is_delete > 0 && patch->new_name) {
+		if (patch_input_file)
+			error(_("git diff header for deleted file %s names a new file %s at %s:%d"),
+			      patch->old_name, patch->new_name,
+			      patch_input_file, *linenr);
+		else
+			error(_("git diff header for deleted file %s names a new file %s (line %d)"),
+			      patch->old_name, patch->new_name, *linenr);
+		return -128;
+	}
 	patch->is_toplevel_relative = 1;
 	return offset;
 }

index 727c36e7ea..ac9090d903 100755
@@ -150,4 +150,35 @@ test_expect_success 'blank line after "diff --git" keeps paths relative to the t
 	test_path_is_missing f
 '
 
+for old in /dev/null a/f
+do
+	test_expect_success "reject \"--- $old\" before \"new file mode\"" '
+		cat >old_before_new.diff <<-EOF &&
+		diff --git a/f b/f
+		--- $old
+		+++ b/f
+		new file mode 100644
+		@@ -0,0 +1 @@
+		+1
+		EOF
+		test_must_fail git apply --check old_before_new.diff 2>err &&
+		test_grep "header for new file f names an old file" err
+	'
+done
+
+test_expect_success 'reject "+++ /dev/null" before "deleted file mode"' '
+	cat >new_before_delete.diff <<-\EOF &&
+	diff --git a/f b/f
+	--- a/f
+	+++ /dev/null
+	deleted file mode 100644
+	@@ -1 +0,0 @@
+	-1
+	EOF
+	test_must_fail git apply --check new_before_delete.diff 2>err &&
+	test_grep "header for deleted file f names a new file" err &&
+	test_must_fail git apply --check -R new_before_delete.diff 2>err &&
+	test_grep "header for deleted file f names a new file" err
+'
+
 test_done

2.43.0

Hi Tanga

These sound like corrupt patches which we should be rejecting with a better error message, rather than applying.

Hey Phillip,

Thank you for reviewing my patch.

I now see and agree that accepting invalid patches was the wrong call. Giving a clearer error seems to be a better direction to take.

No, it does not respect the file mode. "new file mode 100755" is ignored and the file gets the default mode.

Yes, I will treat this one the same way as the blank line.

Alright, I will keep accepting this one.

For the next version, I plan to die with an error when a "diff --git" line has no header lines after it, and a "---" and "+++" patch follows before the next "diff --git" line. The error would name the line of the "diff --git". An empty section followed by another "diff --git" line would still be ignored.

One thing I would like to check with you. On master, a blank line after "diff --git" is accepted in a patch that modifies a file. If that patch has "old mode" and "new mode" lines, the mode change is lost. The rule above would reject these patches too. What is your view on this?

Thanks,
Tanga

Hi Tanga

It would have been helpful to retain a bit more context here, I've restored it so anyone reading this could see the patch that I was referring to.

So that is a good reason to reject this, rather than silently applying a corrupt patch.

Do we accept that now, or only with your patch?

Oh, so

index 573541a..d00491f 100644

@@ -1 +1 @@
-0
+1

applies. That looks like a corrupt patch as well to me and if we're ignoring the mode headers I'd say it is definitely a bug and rejecting it is the right thing to do.

Thanks

Phillip

Hey Phillip,

Thanks, and sorry for trimming too much context last time.

Only with my patch. On master your example fails with "error: git
apply: bad git-diff - inconsistent old filename", because the names from the empty "diff --git" line are kept. A new or deleted file after it fails in a similar way. It does work on master when the next section names the same file, or is a rename.

Junio wrote in reply to your first mail:

So I plan to drop this part and leave this case as it is on master. The next version will only turn the corrupt patches into an error.

One thing I noticed while checking this. "git format-patch" can produce a "diff --git" line with nothing after it, when a line in the body of the commit message looks like "diff --git a/x b/x". On master, "git apply" fails on that patch with the same "inconsistent old filename" error, unless the line names the same file as the real patch. This is not new, so I would leave it out of this series, unless you or Junio think otherwise.

Yes, it applies on master. Alright, I will reject this one too in the next version, along with the blank line and the indented "new file mode" cases.

Thanks,
Tanga

A garbage in the commit log message does not count as coming from "git diff". It may take some effort to teach "git am" to avoid feeding such a false "diff --git" line down to "git apply", but an important thing to keep in mind is that it is NOT "git apply"'s job to worry about such a garbage input.

Hello Junio,

That makes sense. I had thought of it as something "git format-patch" produces, but it is only the log message passing through. I will leave it out, and the next version will only reject the corrupt headers.

Thanks,
Tanga

We shouldn't accept anything that "git format-patch" (or "git diff") would not produce, I would have to say.

Thanks.