vcs-svn: make reading of properties binary-safe

svn-fe errors out on revision 59151 of the ASF repository:

 fatal: invalid dump: unexpected end of file

The proximate cause is a property with an embedded NUL character.
Previously such anomalies were ignored but commit c9d1c8ba
(2010-12-28) introduced a check strlen(val) == len to avoid reading
uninitialized data when a property list ends early and unfortunately
this test does not distinguish between "foo" followed by EOF and the
string "foo\0bar\0baz".

Fix it by using buffer_read_binary to read to a strbuf and checking
the actual length read.  Most consumers of properties still use
C-style strings, so in practice an author or log message with embedded
NULs will be truncated, but a least this way svn-fe won't error out
(fixing the regression).

Reported-by: David Barr <david.barr@cordelta.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

1 files changed, 10 insertions, 14 deletions

diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index ea5b128e4f..c00f031179 100644
--- a/vcs-svn/svndump.c
+++ b/vcs-svn/svndump.c
@@ -147,6 +147,7 @@ static void die_short_read(void)
 static void read_props(void)
 {
 	static struct strbuf key = STRBUF_INIT;
+	static struct strbuf val = STRBUF_INIT;
 	const char *t;
 	/*
 	 * NEEDSWORK: to support simple mode changes like
@@ -163,15 +164,15 @@ static void read_props(void)
 	uint32_t type_set = 0;
 	while ((t = buffer_read_line(&input)) && strcmp(t, "PROPS-END")) {
 		uint32_t len;
-		const char *val;
 		const char type = t[0];
 		int ch;
 
 		if (!type || t[1] != ' ')
 			die("invalid property line: %s\n", t);
 		len = atoi(&t[2]);
-		val = buffer_read_string(&input, len);
-		if (!val || strlen(val) != len)
+		strbuf_reset(&val);
+		buffer_read_binary(&input, &val, len);
+		if (val.len < len)
 			die_short_read();
 
 		/* Discard trailing newline. */
@@ -179,22 +180,17 @@ static void read_props(void)
 		if (ch == EOF)
 			die_short_read();
 		if (ch != '\n')
-			die("invalid dump: expected newline after %s", val);
+			die("invalid dump: expected newline after %s", val.buf);
 
 		switch (type) {
 		case 'K':
+			strbuf_swap(&key, &val);
+			continue;
 		case 'D':
-			strbuf_reset(&key);
-			if (val)
-				strbuf_add(&key, val, len);
-			if (type == 'K')
-				continue;
-			assert(type == 'D');
-			val = NULL;
-			len = 0;
-			/* fall through */
+			handle_property(&val, NULL, 0, &type_set);
+			continue;
 		case 'V':
-			handle_property(&key, val, len, &type_set);
+			handle_property(&key, val.buf, len, &type_set);
 			strbuf_reset(&key);
 			continue;
 		default: