Hej,
I'm not sure if this was the origin email ...
Commit e7d04ee147dcbe6af1fa1d2147466696e is OK.
But:
failure on t9010 with commit 195b7ca6f229455da61f9f6b
=============
# test_cmp expect.message actual.message &&
# test_cmp expect.hello1 actual.hello1 &&
# test_cmp expect.hello2 actual.hello2
#
ok 14 - change file mode and reiterate content
ok 15 - deltas not supported
ok 16 - property deltas supported
ok 17 - properties on /
ok 18 - deltas for typechange
ok 19 - set up svn repo
ok 20 - t9135/svn.dump
# still have 3 known breakage(s)
# failed 1 among remaining 17 test(s)
1..20
=====================
Some more info:
b@birne:~/projects/git/git.git> uname -a
Darwin birne.lan 10.7.0 Darwin Kernel Version 10.7.0: Sat Jan 29
15:17:16 PST 2011; root:xnu-1504.9.37~1/RELEASE_I386 i386
tb@birne:~/projects/git/git.git> svn --version
svn, version 1.6.15 (r1038135)
compiled Jan 29 2011, 15:18:15
tb@birne:~/projects/git/git.git> svnadmin --version
svnadmin, version 1.6.15 (r1038135)
compiled Jan 29 2011, 15:18:15
which svn
/usr/bin/svn
I can assist with some more testing
BR
/Torsten
On 03/25/2011 05:07 AM, Jonathan Nieder wrote:
> 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:
======================
On 03/25/2011 05:07 AM, Jonathan Nieder wrote:
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:
--
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