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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).