[PATCH 2/2] apply: handle assertion failure gracefully

From

Vegard Nossum <vegard.nossum@oracle.com>

Date

Feb 25, 2017, 10:13 UTC

Message-ID

<20170225101307.24067-2-vegard.nossum@oracle.com>

In-Reply-To

<20170225101307.24067-1-vegard.nossum@oracle.com>

For the patches in the added testcases, we were crashing with:

    git-apply: apply.c:3665: check_preimage: Assertion `patch->is_new <= 0' failed.

Found using AFL.

Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>

---

(I'm fully aware of how it looks to just delete an assertion to "fix" a
bug without any other changes to accomodate the condition that was
being tested for. I am definitely not an expert on this code, but as far
as I can tell -- both by reviewing and testing the code -- the function
really is prepared to handle the case where patch->is_new == 1, as it
will always hit another error condition if that is true. I've tried to
add more test cases to show what errors you can expect to see instead of
the assertion failure when trying to apply these nonsensical patches. If
you don't want to remove the assertion for whatever reason, please feel
free to take the testcases and add "# TODO: known breakage" or whatever.)
---
 apply.c                     |  1 -
 t/t4154-apply-git-header.sh | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/apply.c b/apply.c
index cbf7cc7f2..9219d2737 100644
--- a/apply.c
+++ b/apply.c
@@ -3652,7 +3652,6 @@ static int check_preimage(struct apply_state *state,
 	if (!old_name)
 		return 0;
 
-	assert(patch->is_new <= 0);
 	previous = previous_patch(state, patch, &status);
 
 	if (status)
diff --git a/t/t4154-apply-git-header.sh b/t/t4154-apply-git-header.sh
index d651af4a2..c440c48ad 100755
--- a/t/t4154-apply-git-header.sh
+++ b/t/t4154-apply-git-header.sh
@@ -12,4 +12,40 @@ rename new 0
 EOF
 '
 
+test_expect_success 'apply deleted file mode / new file mode / wrong mode' '
+	test_must_fail git apply << EOF
+diff --git a/. b/.
+deleted file mode 
+new file mode 
+EOF
+'
+
+test_expect_success 'apply deleted file mode / new file mode / wrong type' '
+	mkdir x &&
+	chmod 755 x &&
+	test_must_fail git apply << EOF
+diff --git a/x b/x
+deleted file mode 160755
+new file mode 
+EOF
+'
+
+test_expect_success 'apply deleted file mode / new file mode / already exists' '
+	touch 1 &&
+	chmod 644 1 &&
+	test_must_fail git apply << EOF
+diff --git a/1 b/1
+deleted file mode 100644
+new file mode 
+EOF
+'
+
+test_expect_success 'apply new file mode / copy from / nonexistant file' '
+	test_must_fail git apply << EOF
+diff --git a/. b/.
+new file mode 
+copy from  
+EOF
+'
+
 test_done

-- 
2.12.0.rc0