From: tb <tboegi@web.de>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: David Barr <david.barr@cordelta.com>,
Git Mailing List <git@vger.kernel.org>,
Ramkumar Ramachandra <artagnon@gmail.com>,
Junio C Hamano <gitster@pobox.com>,
tboegi@web.de
Subject: Re: [PATCH 1/4] vcs-svn: make reading of properties binary-safe
Date: Mon, 28 Mar 2011 17:34:02 +0200 [thread overview]
Message-ID: <4D90AA6A.1090904@web.de> (raw)
In-Reply-To: <20110325040730.GB3007@elie>
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@cordelta.com>
> Signed-off-by: Jonathan Nieder<jrnieder@gmail.com>
> ---
> 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@cordelta.com>
> Signed-off-by: Jonathan Nieder<jrnieder@gmail.com>
> ---
> 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:
next prev parent reply other threads:[~2011-03-28 15:34 UTC|newest]
Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-03-19 7:03 vcs-svn: purge obsolete data structures and code David Barr
2011-03-19 7:03 ` [PATCH 1/9] vcs-svn: pass paths through to fast-import David Barr
2011-03-19 7:50 ` Jonathan Nieder
2011-03-19 7:03 ` [PATCH 2/9] vcs-svn: avoid using ls command twice David Barr
2011-03-19 8:01 ` Jonathan Nieder
2011-03-19 7:03 ` [PATCH 3/9] vcs-svn: implement perfect hash for node-prop keys David Barr
2011-03-19 8:51 ` Jonathan Nieder
2011-03-21 1:26 ` [PATCH 1/3] " David Barr
2011-03-21 1:26 ` [PATCH 2/3] vcs-svn: implement perfect hash for top-level keys David Barr
2011-03-21 1:26 ` [PATCH 3/3] vcs-svn: use switch rather than cascading ifs David Barr
2011-03-21 1:38 ` [PATCHv2] " David Barr
2011-03-19 7:03 ` [PATCH 4/9] vcs-svn: implement perfect hash for top-level keys David Barr
2011-03-19 8:57 ` Jonathan Nieder
2011-03-19 7:03 ` [PATCH 5/9] vcs-svn: factor out usage of string_pool David Barr
2011-03-19 9:08 ` Jonathan Nieder
2011-03-19 7:03 ` [PATCH 6/9] vcs-svn: drop string_pool David Barr
2011-03-19 7:03 ` [PATCH 7/9] vcs-svn: drop trp.h David Barr
2011-03-19 7:03 ` [PATCH 8/9] vcs-svn: drop obj_pool.h David Barr
2011-03-19 7:03 ` [PATCH 9/9] vcs-svn: use strchr to find RFC822 delimiter David Barr
2011-03-19 9:10 ` Jonathan Nieder
2011-03-19 7:20 ` vcs-svn: integrate support for text deltas David Barr
2011-03-19 7:20 ` [PATCH 01/16] vcs-svn: improve support for reading large files David Barr
2011-03-19 7:20 ` [PATCH 02/16] vcs-svn: make buffer_skip_bytes return length read David Barr
2011-03-19 7:20 ` [PATCH 03/16] vcs-svn: make buffer_copy_bytes " David Barr
2011-03-19 7:20 ` [PATCH 04/16] vcs-svn: improve reporting of input errors David Barr
2011-03-19 7:20 ` [PATCH 05/16] vcs-svn: learn to maintain a sliding view of a file David Barr
2011-03-19 7:20 ` [PATCH 06/16] vcs-svn: skeleton of an svn delta parser David Barr
2011-03-28 3:30 ` Jonathan Nieder
2011-03-19 7:20 ` [PATCH 07/16] vcs-svn: parse svndiff0 window header David Barr
2011-03-19 7:20 ` [PATCH 08/16] vcs-svn: read the preimage when applying deltas David Barr
2011-03-19 7:20 ` [PATCH 09/16] vcs-svn: read inline data from deltas David Barr
2011-03-19 7:20 ` [PATCH 10/16] vcs-svn: read instructions " David Barr
2011-03-19 7:20 ` [PATCH 11/16] vcs-svn: implement copyfrom_data delta instruction David Barr
2011-03-19 7:20 ` [PATCH 12/16] vcs-svn: verify that deltas consume all inline data David Barr
2011-03-19 7:20 ` [PATCH 13/16] vcs-svn: let deltas use data from postimage David Barr
2011-03-19 7:20 ` [PATCH 14/16] vcs-svn: let deltas use data from preimage David Barr
2011-03-19 7:20 ` [PATCH 15/16] vcs-svn: microcleanup in svndiff0 window-reading code David Barr
2011-03-19 7:20 ` [PATCH 16/16] vcs-svn: implement text-delta handling David Barr
2011-03-28 7:00 ` vcs-svn: integrate support for text deltas Jonathan Nieder
2011-03-28 11:56 ` David Barr
2011-03-21 23:49 ` [PATCHv2 00/11] vcs-svn: purge obsolete data structures and code David Barr
2011-03-21 23:49 ` [PATCH 01/11] vcs-svn: use strbuf for revision log David Barr
2011-03-21 23:49 ` [PATCH 02/11] vcs-svn: pass paths through to fast-import David Barr
2011-03-21 23:49 ` [PATCH 03/11] vcs-svn: avoid using ls command twice David Barr
2011-03-21 23:49 ` [PATCH 04/11] vcs-svn: implement perfect hash for node-prop keys David Barr
2011-03-21 23:49 ` [PATCH 05/11] vcs-svn: implement perfect hash for top-level keys David Barr
2011-03-21 23:49 ` [PATCH 06/11] vcs-svn: use switch rather than cascading ifs David Barr
2011-03-21 23:49 ` [PATCH 07/11] vcs-svn: factor out usage of string_pool David Barr
2011-03-21 23:49 ` [PATCH 08/11] vcs-svn: drop string_pool David Barr
2011-03-21 23:49 ` =?^[?q?=5BPATCH=2009/11=5D=20vcs-svn=3A=20drop=20trp=2Eh?= David Barr
2011-03-21 23:49 ` [PATCH 10/11] vcs-svn: drop obj_pool.h David Barr
2011-03-21 23:50 ` [PATCH 11/11] vcs-svn: use strchr to find RFC822 delimiter David Barr
2011-03-23 0:32 ` [PULL svn-fe] vcs-svn: simplifications, error handling improvements Jonathan Nieder
2011-03-23 5:46 ` Junio C Hamano
2011-03-23 6:03 ` Junio C Hamano
2011-03-26 6:42 ` Jonathan Nieder
2011-03-26 9:49 ` t0081-line-buffer.sh hangs (Re: [PULL svn-fe] vcs-svn: simplifications, error handling improvements) Jonathan Nieder
2011-03-23 7:11 ` [PULL svn-fe] vcs-svn: simplifications, error handling improvements David Barr
2011-03-24 12:43 ` [PATCH] fixup! vcs-svn: improve reporting of input errors David Barr
2011-03-25 1:12 ` Jonathan Nieder
2011-03-25 3:34 ` [PATCH svn-fe 0/4] vcs-svn: null bytes in properties Jonathan Nieder
2011-03-25 4:07 ` [PATCH 1/4] vcs-svn: make reading of properties binary-safe Jonathan Nieder
2011-03-28 15:34 ` tb [this message]
2011-03-28 19:41 ` Jonathan Nieder
2011-03-28 20:30 ` Torsten Bögershausen
2011-03-28 20:44 ` Jonathan Nieder
2011-03-25 4:09 ` [PATCH 2/4] vcs-svn: remove buffer_read_string Jonathan Nieder
2011-03-25 4:10 ` [PATCH 3/4] vcs-svn: avoid unnecessary copying of log message and author Jonathan Nieder
2011-03-25 4:11 ` [PATCH 4/4] vcs-svn: handle log message with embedded null bytes Jonathan Nieder
2011-03-26 6:46 ` [PULL svn-fe] vcs-svn: simplifications, error handling improvements Jonathan Nieder
2011-03-26 18:36 ` Junio C Hamano
2011-03-28 0:38 ` [PATCH svn-fe] vcs-svn: add missing cast to printf argument Jonathan Nieder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4D90AA6A.1090904@web.de \
--to=tboegi@web.de \
--cc=artagnon@gmail.com \
--cc=david.barr@cordelta.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.