A caller to buffer_read_string cannot easily tell the difference
between the string "foo" followed by an early end of file and the
string "foo\0bar\0baz". In a half-hearted attempt to catch early EOF,
c9d1c8ba (2010-12-28) introduced a safety strlen(val) == len for
property keys and values, to at least keep svn-fe from reading
uninitialized data when a property list ends early due to EOF.
But it is permissible for both keys and values to contain null
characters, so in handling revision 59151 of the ASF repository svn-fe
encounters a null byte and produces the following message:
fatal: invalid dump: unexpected end of file
Fix it by using buffer_read_binary to read to a strbuf (and keep track
of the actual length read). Most consumers of properties still use
C-style strings, so in practice we still can't use an author or log
message with embedded nuls, but at least this way svn-fe won't error
out.
Reported-by: David Barr <david.barr@xxxxxxxxxxxx>
Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
---
t/t9010-svn-fe.sh | 27 +++++++++++++++++++++++++++
vcs-svn/svndump.c | 24 ++++++++++--------------
2 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/t/t9010-svn-fe.sh b/t/t9010-svn-fe.sh
index 5a6a4b9..47f1e4f 100755
--- a/t/t9010-svn-fe.sh
+++ b/t/t9010-svn-fe.sh
@@ -370,6 +370,33 @@ test_expect_failure 'change file mode but keep old content' '
test_cmp hello actual.target
'
+test_expect_success 'null byte in property value' '
+ reinit_git &&
+ echo "commit message" >expect.message &&
+ {
+ properties \
+ unimportant "something with a null byte (Q)" \
+ svn:log "commit message"&&
+ echo PROPS-END
+ } |
+ q_to_nul >props &&
+ {
+ cat <<-\EOF &&
+ SVN-fs-dump-format-version: 3
+
+ Revision-number: 1
+ EOF
+ echo Prop-content-length: $(wc -c <props) &&
+ echo Content-length: $(wc -c <props) &&
+ echo &&
+ cat props
+ } >nullprop.dump &&
+ test-svn-fe nullprop.dump >stream &&
+ git fast-import <stream &&
+ git diff-tree --always -s --format=%s HEAD >actual.message &&
+ test_cmp expect.message actual.message
+'
+
test_expect_success 'change file mode and reiterate content' '
reinit_git &&
cat >expect <<-\EOF &&
diff --git a/vcs-svn/svndump.c b/vcs-svn/svndump.c
index ea5b128..c00f031 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:
--
1.7.4.1
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html