git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 2/4] apply: make update_pre_post_images() sanity check the given postlen

From
Junio C Hamano <gitster@pobox.com>
Date
Jan 22, 2015, 22:58 UTC
Message-ID
<1421967505-16879-3-git-send-email-gitster@pobox.com>
In-Reply-To
<1421967505-16879-1-git-send-email-gitster@pobox.com>

"git apply --whitespace=fix" used to be able to assume that fixing errors will always reduce the size by e.g. stripping whitespaces at the end of lines or collapsing runs of spaces into tabs at the beginning of lines. An update to accomodate fixes that lengthens the result by e.g. expanding leading tabs into spaces were made long time ago but the logic miscounted the necessary space after such whitespace fixes, leading to either under-allocation or over-usage of already allocated space.

Illustrate this with a runtime sanity-check to protect us from future breakage. The test was stolen from Kyle McKay who helped to identify the problem.

Helped-by: "Kyle J. McKay" <mackyle@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/apply.c               |   6 +++
 t/t4138-apply-ws-expansion.sh | 121 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+)
 create mode 100755 t/t4138-apply-ws-expansion.sh
diff --git a/builtin/apply.c b/builtin/apply.c
index 31f8733..da6fb35 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -2171,6 +2171,12 @@ static void update_pre_post_images(struct image *preimage,
 		ctx++;
 	}
 
+	if (postlen
+	    ? postlen < new - postimage->buf
+	    : postimage->len < new - postimage->buf)
+		die("BUG: caller miscounted postlen: asked %d, orig = %d, used = %d",
+		    (int)postlen, (int) postimage->len, (int)(new - postimage->buf));
+
 	/* Fix the length of the whole thing */
 	postimage->len = new - postimage->buf;
 	postimage->nr -= reduced;
diff --git a/t/t4138-apply-ws-expansion.sh b/t/t4138-apply-ws-expansion.sh
new file mode 100755
index 0000000..0ffe33f
--- /dev/null
+++ b/t/t4138-apply-ws-expansion.sh
@@ -0,0 +1,121 @@
+#!/bin/sh
+#
+# Copyright (C) 2015 Kyle J. McKay
+#
+
+test_description='git apply test patches with whitespace expansion.'
+
+. ./test-lib.sh
+
+test_expect_success setup '
+	#
+	## create test-N, patchN.patch, expect-N files
+	#
+
+	# test 1
+	printf "\t%s\n" 1 2 3 4 5 6 >before &&
+	printf "\t%s\n" 1 2 3 >after &&
+	printf "%64s\n" a b c >>after &&
+	printf "\t%s\n" 4 5 6 >>after &&
+	git diff --no-index before after |
+		sed -e "s/before/test-1/" -e "s/after/test-1/" >patch1.patch &&
+	printf "%64s\n" 1 2 3 4 5 6 >test-1 &&
+	printf "%64s\n" 1 2 3 a b c 4 5 6 >expect-1 &&
+
+	# test 2
+	printf "\t%s\n" a b c d e f >before &&
+	printf "\t%s\n" a b c >after &&
+	n=10 &&
+	x=1 &&
+	while test $x -lt $n
+	do
+		printf "%63s%d\n" "" $x >>after
+		x=$(( $x + 1 ))
+	done &&
+	printf "\t%s\n" d e f >>after &&
+	git diff --no-index before after |
+		sed -e "s/before/test-2/" -e "s/after/test-2/" >patch2.patch &&
+	printf "%64s\n" a b c d e f >test-2 &&
+	printf "%64s\n" a b c >expect-2 &&
+	x=1 &&
+	while test $x -lt $n
+	do
+		printf "%63s%d\n" "" $x >>expect-2
+		x=$(( $x + 1 ))
+	done &&
+	printf "%64s\n" d e f >>expect-2 &&
+
+	# test 3
+	printf "\t%s\n" a b c d e f >before &&
+	printf "\t%s\n" a b c >after &&
+	n=100 &&
+	x=0 &&
+	while test $x -lt $n
+	do
+		printf "%63s%02d\n" "" $x >>after
+		x=$(( $x + 1 ))
+	done &&
+	printf "\t%s\n" d e f >>after &&
+	git diff --no-index before after |
+	sed -e "s/before/test-3/" -e "s/after/test-3/" >patch3.patch &&
+	printf "%64s\n" a b c d e f >test-3 &&
+	printf "%64s\n" a b c >expect-3 &&
+	x=0 &&
+	while test $x -lt $n
+	do
+		printf "%63s%02d\n" "" $x >>expect-3
+		x=$(( $x + 1 ))
+	done &&
+	printf "%64s\n" d e f >>expect-3 &&
+
+	# test 4
+	>before &&
+	x=0 &&
+	while test $x -lt 50
+	do
+		printf "\t%02d\n" $x >>before
+		x=$(( $x + 1 ))
+	done &&
+	cat before >after &&
+	printf "%64s\n" a b c >>after &&
+	while test $x -lt 100
+	do
+		printf "\t%02d\n" $x >>before
+		printf "\t%02d\n" $x >>after
+		x=$(( $x + 1 ))
+	done &&
+	git diff --no-index before after |
+	sed -e "s/before/test-4/" -e "s/after/test-4/" >patch4.patch &&
+	>test-4 &&
+	x=0 &&
+	while test $x -lt 50
+	do
+		printf "%63s%02d\n" "" $x >>test-4
+		x=$(( $x + 1 ))
+	done &&
+	cat test-4 >expect-4 &&
+	printf "%64s\n" a b c >>expect-4 &&
+	while test $x -lt 100
+	do
+		printf "%63s%02d\n" "" $x >>test-4
+		printf "%63s%02d\n" "" $x >>expect-4
+		x=$(( $x + 1 ))
+	done &&
+
+	git config core.whitespace tab-in-indent,tabwidth=63 &&
+	git config apply.whitespace fix
+
+'
+
+# Note that `patch` can successfully apply all patches when run
+# with the --ignore-whitespace option.
+
+for t in 1 2 3 4
+do
+	test_expect_success 'apply with ws expansion (t=$t)' '
+		git apply patch$t.patch &&
+		test_cmp test-$t expect-$t
+	'
+done
+
+test_done
-- 
2.3.0-rc1-116-g84c5016
Previous: Stefan BellerNext: Junio C Hamano
Message 23 of 26 in “Segmentation fault in git apply”
  1. Michael BlumeJan 14, 2015
  2. Michael BlumeJan 14, 2015
  3. Michael BlumeJan 14, 2015
  4. Michael BlumeJan 14, 2015
  5. Michael BlumeJan 14, 2015
  6. Michael BlumeJan 14, 2015
  7. Kyle J. McKayJan 15, 2015
  8. Kyle J. McKayJan 15, 2015
  9. Junio C HamanoJan 16, 2015
  10. apply: count the size of postimage correctlyJunio C Hamano, Jan 16, 2015
  11. test: add git apply whitespace expansion testsKyle J. McKay, Jan 18, 2015
  12. Junio C HamanoJan 18, 2015
  13. Kyle J. McKayJan 19, 2015
  14. Junio C HamanoJan 21, 2015
  15. Kyle J. McKayJan 22, 2015
  16. Junio C HamanoJan 22, 2015
  17. Kyle J. McKayJan 23, 2015
  18. 0/4 apply --whitespace=fix buffer corruption fixJunio C Hamano, Jan 22, 2015
  19. 1/4 apply.c: typofixJunio C Hamano, Jan 22, 2015
  20. Stefan BellerJan 22, 2015
  21. Junio C HamanoJan 22, 2015
  22. Stefan BellerJan 22, 2015
  23. 2/4 apply: make update_pre_post_images() sanity check the given postlenJunio C Hamano, Jan 22, 2015
  24. 3/4 apply: count the size of postimage correctlyJunio C Hamano, Jan 22, 2015
  25. 4/4 apply: detect and mark whitespace errors in context lines when fixingJunio C Hamano, Jan 22, 2015
  26. Junio C HamanoJan 14, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.