[Outreachy PATCH 2/2] apply: reject new and deleted files that have both names

[Tanga Joris <tangajoris@gmail.com>]

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.

Signed-off-by: Tanga Joris <tangajoris@gmail.com>
---
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(+)

diff --git a/apply.c b/apply.c
index b56b363963..76cdacff33 100644
--- a/apply.c
+++ b/apply.c
@@ -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;
 }
diff --git a/t/t4133-apply-filenames.sh b/t/t4133-apply-filenames.sh
index 727c36e7ea..ac9090d903 100755
--- a/t/t4133-apply-filenames.sh
+++ b/t/t4133-apply-filenames.sh
@@ -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