Subject: Re: [PATCH] fixup! vcs-svn: improve reporting of input errors

From: Jonathan Nieder <[email-masked]>
Date: Thu, 24 Mar 2011 20:12:50 -0500

Hi David,

David Barr wrote:

> An excessive constraint was introduced in c9d1c8ba; when reading
> svn props, it is permissible for both keys and values to contain
> nul characters.

Yes, that will work.

buffer_read_string returns a '\0'-terminated string and on early EOF,
the part after the end-of-file will be gibberish (and probably
uninitialized).  But it lives in an 1000-char buffer and errors out
when it doesn't fit, so at least with the fix it wouldn't crash.

Sorry for the breakage.  I suppose a test like the following would
catch future problems of this kind?

Signed-off-by: Jonathan Nieder <[email-masked]>
---
 t/t9010-svn-fe.sh |   79 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 79 insertions(+), 0 deletions(-)

diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index 5a6a4b9..a91b59c 100755
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -370,6 +370,85 @@ test_expect_failure 'change file mode but keep old content' '
 	test_cmp hello actual.target
 '
 
+test_expect_failure 'null bytes' '
+	# Caveat: svnadmin 1.6.16 (r1073529) truncates at \0 in the
+	# svn:specialQnotreally example.
+	reinit_git &&
+	cat >expect <<-\EOF &&
+	OBJID
+	:100644 100644 OBJID OBJID M	greeting
+	OBJID
+	:000000 100644 OBJID OBJID A	greeting
+	EOF
+	printf "%s\n" "something with a null byte (Q)" |
+		q_to_nul >expect.message &&
+	printf "%s\n" "helQo" |
+		q_to_nul >expect.hello1 &&
+	printf "%s\n" "link hello" >expect.hello2 &&
+	{
+		properties svn:log "something with a null byte (Q)" &&
+		echo PROPS-END
+	} |
+	q_to_nul >props &&
+	{
+		q_to_nul <<-\EOF &&
+		SVN-fs-dump-format-version: 3
+
+		Revision-number: 1
+		Prop-content-length: 10
+		Content-length: 10
+
+		PROPS-END
+
+		Node-path: greeting
+		Node-kind: file
+		Node-action: add
+		Prop-content-length: 10
+		Text-content-length: 6
+		Content-length: 16
+
+		PROPS-END
+		helQo
+
+		Revision-number: 2
+		EOF
+		echo Prop-content-length: $(wc -c <props) &&
+		echo Content-length: $(wc -c <props) &&
+		echo &&
+		cat props &&
+		q_to_nul <<-\EOF
+
+		Node-path: greeting
+		Node-kind: file
+		Node-action: change
+		Prop-content-length: 43
+		Text-content-length: 11
+		Content-length: 54
+
+		K 21
+		svn:specialQnotreally
+		V 1
+		*
+		PROPS-END
+		link hello
+		EOF
+	} >8bitclean.dump &&
+	test-svn-fe 8bitclean.dump >stream &&
+	git fast-import <stream &&
+	{
+		git rev-list HEAD |
+		git diff-tree --root --stdin |
+		sed "s/$_x40/OBJID/g"
+	} >actual &&
+	git diff-tree --always -s --format=%s HEAD >actual.message &&
+	git cat-file blob HEAD^:greeting >actual.hello1 &&
+	git cat-file blob HEAD:greeting >actual.hello2 &&
+	test_cmp expect actual &&
+	test_cmp expect.message actual.message &&
+	test_cmp expect.hello1 actual.hello1 &&
+	test_cmp expect.hello2 actual.hello2
+'
+
 test_expect_success 'change file mode and reiterate content' '
 	reinit_git &&
 	cat >expect <<-\EOF &&
-- 
1.7.4.1