* [PATCH 1/5] fast-import: add input format tests
2011-07-28 5:43 [PATCH 0/5] fixes for committer/author parsing/check Dmitry Ivankov
@ 2011-07-28 5:44 ` Dmitry Ivankov
2011-07-28 5:44 ` [PATCH 2/5] fast-import: don't fail on omitted committer name Dmitry Ivankov
` (3 subsequent siblings)
4 siblings, 0 replies; 11+ messages in thread
From: Dmitry Ivankov @ 2011-07-28 5:44 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Dmitry Ivankov
Documentation/git-fast-import.txt says that git-fast-import is strict
about it's input format. But committer/author field parsing is a bit
loose. Invalid values can be unnoticed and written out to the commit,
either with format-conforming input or with non-format-conforming one.
Add one passing and one failing test for empty/absent committer name
with well-formed input. And a failed test with unnoticed ill-formed
input.
Reported-by: SASAKI Suguru <sss.sonik@gmail.com>
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
t/t9300-fast-import.sh | 99 ++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 99 insertions(+), 0 deletions(-)
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 2a53640..a659dd4 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -324,6 +324,105 @@ test_expect_success \
test `git rev-parse master` = `git rev-parse TEMP_TAG^`'
rm -f .git/TEMP_TAG
+git gc 2>/dev/null >/dev/null
+git prune 2>/dev/null >/dev/null
+
+cat >input <<INPUT_END
+commit refs/heads/empty-committer-1
+committer <> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_success 'B: accept empty committer' '
+ git fast-import <input &&
+ out=$(git fsck) &&
+ echo "$out" &&
+ test -z "$out"
+'
+git update-ref -d refs/heads/empty-committer-1 || true
+
+git gc 2>/dev/null >/dev/null
+git prune 2>/dev/null >/dev/null
+
+cat >input <<INPUT_END
+commit refs/heads/empty-committer-2
+committer <a@b.com> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: accept and fixup committer with no name' '
+ git fast-import <input &&
+ out=$(git fsck) &&
+ echo "$out" &&
+ test -z "$out"
+'
+git update-ref -d refs/heads/empty-committer-2 || true
+
+git gc 2>/dev/null >/dev/null
+git prune 2>/dev/null >/dev/null
+
+cat >input <<INPUT_END
+commit refs/heads/invalid-committer
+committer Name email> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: fail on invalid committer (1)' '
+ test_must_fail git fast-import <input
+'
+git update-ref -d refs/heads/invalid-committer || true
+
+cat >input <<INPUT_END
+commit refs/heads/invalid-committer
+committer Name <e<mail> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: fail on invalid committer (2)' '
+ test_must_fail git fast-import <input
+'
+git update-ref -d refs/heads/invalid-committer || true
+
+cat >input <<INPUT_END
+commit refs/heads/invalid-committer
+committer Name <email>> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: fail on invalid committer (3)' '
+ test_must_fail git fast-import <input
+'
+git update-ref -d refs/heads/invalid-committer || true
+
+cat >input <<INPUT_END
+commit refs/heads/invalid-committer
+committer Name <email $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: fail on invalid committer (4)' '
+ test_must_fail git fast-import <input
+'
+git update-ref -d refs/heads/invalid-committer || true
+
+cat >input <<INPUT_END
+commit refs/heads/invalid-committer
+committer Name<email> $GIT_COMMITTER_DATE
+data <<COMMIT
+empty commit
+COMMIT
+INPUT_END
+test_expect_failure 'B: fail on invalid committer (5)' '
+ test_must_fail git fast-import <input
+'
+git update-ref -d refs/heads/invalid-committer || true
+
###
### series C
###
--
1.7.3.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/5] fast-import: don't fail on omitted committer name
2011-07-28 5:43 [PATCH 0/5] fixes for committer/author parsing/check Dmitry Ivankov
2011-07-28 5:44 ` [PATCH 1/5] fast-import: add input format tests Dmitry Ivankov
@ 2011-07-28 5:44 ` Dmitry Ivankov
2011-08-02 16:53 ` Junio C Hamano
2011-07-28 5:44 ` [PATCH 3/5] fast-import: check committer name more strictly Dmitry Ivankov
` (2 subsequent siblings)
4 siblings, 1 reply; 11+ messages in thread
From: Dmitry Ivankov @ 2011-07-28 5:44 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Dmitry Ivankov
fast-import format declares 'committer_name SP' to be optional. But SP
between empty or not name and a email is obligatory and checked by
git-fsck, so fast-import must prepend the SP if the name is omitted.
Currently it doesn't.
Name cannot contain LT or GT and ident always comes after SP in
fast-import. So reuse that SP as if a valid 'SP <email>' ident was passed.
This fixes a ident parsing bug for a well-formed fast-import input.
Though the parsing is still loose and can accept a ill-formed input.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
fast-import.c | 4 ++++
t/t9300-fast-import.sh | 2 +-
2 files changed, 5 insertions(+), 1 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 9e8d186..3194f4e 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1972,6 +1972,10 @@ static char *parse_ident(const char *buf)
size_t name_len;
char *ident;
+ /* ensure there is a space delimiter even if there is no name */
+ if (*buf == '<')
+ --buf;
+
gt = strrchr(buf, '>');
if (!gt)
die("Missing > in ident string: %s", buf);
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index a659dd4..09ef6ba 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -352,7 +352,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: accept and fixup committer with no name' '
+test_expect_success 'B: accept and fixup committer with no name' '
git fast-import <input &&
out=$(git fsck) &&
echo "$out" &&
--
1.7.3.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/5] fast-import: don't fail on omitted committer name
2011-07-28 5:44 ` [PATCH 2/5] fast-import: don't fail on omitted committer name Dmitry Ivankov
@ 2011-08-02 16:53 ` Junio C Hamano
2011-08-02 17:07 ` Dmitry Ivankov
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2011-08-02 16:53 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, SASAKI Suguru
Dmitry Ivankov <divanorama@gmail.com> writes:
> fast-import format declares 'committer_name SP' to be optional. But SP
> between empty or not name and a email is obligatory and checked by
Sorry, cannot parse this.
> git-fsck, so fast-import must prepend the SP if the name is omitted.
> Currently it doesn't.
>
> Name cannot contain LT or GT and ident always comes after SP in
> fast-import. So reuse that SP as if a valid 'SP <email>' ident was passed.
>
> This fixes a ident parsing bug for a well-formed fast-import input.
> Though the parsing is still loose and can accept a ill-formed input.
>
> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
> ---
> fast-import.c | 4 ++++
> t/t9300-fast-import.sh | 2 +-
> 2 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/fast-import.c b/fast-import.c
> index 9e8d186..3194f4e 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1972,6 +1972,10 @@ static char *parse_ident(const char *buf)
> size_t name_len;
> char *ident;
>
> + /* ensure there is a space delimiter even if there is no name */
> + if (*buf == '<')
> + --buf;
> +
> gt = strrchr(buf, '>');
> if (!gt)
> die("Missing > in ident string: %s", buf);
> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
> index a659dd4..09ef6ba 100755
> --- a/t/t9300-fast-import.sh
> +++ b/t/t9300-fast-import.sh
> @@ -352,7 +352,7 @@ data <<COMMIT
> empty commit
> COMMIT
> INPUT_END
> -test_expect_failure 'B: accept and fixup committer with no name' '
> +test_expect_success 'B: accept and fixup committer with no name' '
> git fast-import <input &&
> out=$(git fsck) &&
> echo "$out" &&
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH 2/5] fast-import: don't fail on omitted committer name
2011-08-02 16:53 ` Junio C Hamano
@ 2011-08-02 17:07 ` Dmitry Ivankov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Ivankov @ 2011-08-02 17:07 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SASAKI Suguru
On Tue, Aug 2, 2011 at 10:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Dmitry Ivankov <divanorama@gmail.com> writes:
>
>> fast-import format declares 'committer_name SP' to be optional. But SP
>> between empty or not name and a email is obligatory and checked by
>
> Sorry, cannot parse this.
Ok, the point is fast-import input format for identities is declared to be
'(name SP)? LT email GT' (followed by a datetime)
where name and email are allowed to be empty (and should not have
LF, LT, GT characters).
While git-fsck checks identities to be in form
'name SP LT email GT' (followed by a datetime)
where name and email are allowed to be empty (and should not have
LF, LT, GT characters).
So fast-import must prepend a space if the name part is omitted. This
patch makes it do so.
>
>> git-fsck, so fast-import must prepend the SP if the name is omitted.
>> Currently it doesn't.
>>
>> Name cannot contain LT or GT and ident always comes after SP in
>> fast-import. So reuse that SP as if a valid 'SP <email>' ident was passed.
>>
>> This fixes a ident parsing bug for a well-formed fast-import input.
>> Though the parsing is still loose and can accept a ill-formed input.
>>
>> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
>> ---
>> fast-import.c | 4 ++++
>> t/t9300-fast-import.sh | 2 +-
>> 2 files changed, 5 insertions(+), 1 deletions(-)
>>
>> diff --git a/fast-import.c b/fast-import.c
>> index 9e8d186..3194f4e 100644
>> --- a/fast-import.c
>> +++ b/fast-import.c
>> @@ -1972,6 +1972,10 @@ static char *parse_ident(const char *buf)
>> size_t name_len;
>> char *ident;
>>
>> + /* ensure there is a space delimiter even if there is no name */
>> + if (*buf == '<')
>> + --buf;
>> +
>> gt = strrchr(buf, '>');
>> if (!gt)
>> die("Missing > in ident string: %s", buf);
>> diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
>> index a659dd4..09ef6ba 100755
>> --- a/t/t9300-fast-import.sh
>> +++ b/t/t9300-fast-import.sh
>> @@ -352,7 +352,7 @@ data <<COMMIT
>> empty commit
>> COMMIT
>> INPUT_END
>> -test_expect_failure 'B: accept and fixup committer with no name' '
>> +test_expect_success 'B: accept and fixup committer with no name' '
>> git fast-import <input &&
>> out=$(git fsck) &&
>> echo "$out" &&
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] fast-import: check committer name more strictly
2011-07-28 5:43 [PATCH 0/5] fixes for committer/author parsing/check Dmitry Ivankov
2011-07-28 5:44 ` [PATCH 1/5] fast-import: add input format tests Dmitry Ivankov
2011-07-28 5:44 ` [PATCH 2/5] fast-import: don't fail on omitted committer name Dmitry Ivankov
@ 2011-07-28 5:44 ` Dmitry Ivankov
2011-08-02 17:01 ` Junio C Hamano
2011-07-28 5:44 ` [PATCH 4/5] fsck: add a few committer name tests Dmitry Ivankov
2011-07-28 5:44 ` [PATCH 5/5] fsck: improve committer/author check Dmitry Ivankov
4 siblings, 1 reply; 11+ messages in thread
From: Dmitry Ivankov @ 2011-07-28 5:44 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Dmitry Ivankov
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
fast-import.c | 29 +++++++++++++++++------------
t/t9300-fast-import.sh | 10 +++++-----
2 files changed, 22 insertions(+), 17 deletions(-)
diff --git a/fast-import.c b/fast-import.c
index 3194f4e..419744f 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -1968,7 +1968,7 @@ static int validate_raw_date(const char *src, char *result, int maxlen)
static char *parse_ident(const char *buf)
{
- const char *gt;
+ const char *ltgt;
size_t name_len;
char *ident;
@@ -1976,28 +1976,33 @@ static char *parse_ident(const char *buf)
if (*buf == '<')
--buf;
- gt = strrchr(buf, '>');
- if (!gt)
+ ltgt = buf + strcspn(buf, "<>");
+ if (*ltgt != '<')
+ die("Missing < in ident string: %s", buf);
+ if (ltgt != buf && ltgt[-1] != ' ')
+ die("Missing space before < in ident string: %s", buf);
+ ltgt = ltgt + 1 + strcspn(ltgt + 1, "<>");
+ if (*ltgt != '>')
die("Missing > in ident string: %s", buf);
- gt++;
- if (*gt != ' ')
+ ltgt++;
+ if (*ltgt != ' ')
die("Missing space after > in ident string: %s", buf);
- gt++;
- name_len = gt - buf;
+ ltgt++;
+ name_len = ltgt - buf;
ident = xmalloc(name_len + 24);
strncpy(ident, buf, name_len);
switch (whenspec) {
case WHENSPEC_RAW:
- if (validate_raw_date(gt, ident + name_len, 24) < 0)
- die("Invalid raw date \"%s\" in ident: %s", gt, buf);
+ if (validate_raw_date(ltgt, ident + name_len, 24) < 0)
+ die("Invalid raw date \"%s\" in ident: %s", ltgt, buf);
break;
case WHENSPEC_RFC2822:
- if (parse_date(gt, ident + name_len, 24) < 0)
- die("Invalid rfc2822 date \"%s\" in ident: %s", gt, buf);
+ if (parse_date(ltgt, ident + name_len, 24) < 0)
+ die("Invalid rfc2822 date \"%s\" in ident: %s", ltgt, buf);
break;
case WHENSPEC_NOW:
- if (strcmp("now", gt))
+ if (strcmp("now", ltgt))
die("Date in ident must be 'now': %s", buf);
datestamp(ident + name_len, 24);
break;
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 09ef6ba..18441f8 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -370,7 +370,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: fail on invalid committer (1)' '
+test_expect_success 'B: fail on invalid committer (1)' '
test_must_fail git fast-import <input
'
git update-ref -d refs/heads/invalid-committer || true
@@ -382,7 +382,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: fail on invalid committer (2)' '
+test_expect_success 'B: fail on invalid committer (2)' '
test_must_fail git fast-import <input
'
git update-ref -d refs/heads/invalid-committer || true
@@ -394,7 +394,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: fail on invalid committer (3)' '
+test_expect_success 'B: fail on invalid committer (3)' '
test_must_fail git fast-import <input
'
git update-ref -d refs/heads/invalid-committer || true
@@ -406,7 +406,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: fail on invalid committer (4)' '
+test_expect_success 'B: fail on invalid committer (4)' '
test_must_fail git fast-import <input
'
git update-ref -d refs/heads/invalid-committer || true
@@ -418,7 +418,7 @@ data <<COMMIT
empty commit
COMMIT
INPUT_END
-test_expect_failure 'B: fail on invalid committer (5)' '
+test_expect_success 'B: fail on invalid committer (5)' '
test_must_fail git fast-import <input
'
git update-ref -d refs/heads/invalid-committer || true
--
1.7.3.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/5] fast-import: check committer name more strictly
2011-07-28 5:44 ` [PATCH 3/5] fast-import: check committer name more strictly Dmitry Ivankov
@ 2011-08-02 17:01 ` Junio C Hamano
0 siblings, 0 replies; 11+ messages in thread
From: Junio C Hamano @ 2011-08-02 17:01 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, SASAKI Suguru
Dmitry Ivankov <divanorama@gmail.com> writes:
> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
> ---
Please describe how you check this field "more strictly" in the body of
the log message, iow, against what rule you are validating, perhaps
something like:
The identifier must be either "<address>" or "Name <address>" where
neither address nor Name can contain '<' nor '>'; otherwise the input
stream is rejected.
As fast-import is used to _create_ new objects, its input is a simple text
file that can be fixed-up as needed, it is a good idea to validate the
input more strictly and rejecting bad ones.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] fsck: add a few committer name tests
2011-07-28 5:43 [PATCH 0/5] fixes for committer/author parsing/check Dmitry Ivankov
` (2 preceding siblings ...)
2011-07-28 5:44 ` [PATCH 3/5] fast-import: check committer name more strictly Dmitry Ivankov
@ 2011-07-28 5:44 ` Dmitry Ivankov
2011-07-28 5:44 ` [PATCH 5/5] fsck: improve committer/author check Dmitry Ivankov
4 siblings, 0 replies; 11+ messages in thread
From: Dmitry Ivankov @ 2011-07-28 5:44 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Dmitry Ivankov
fsck reports "missing space before <email>" for committer string equal
to "name email>". It'd be nicer to say "missing < in email" or "name is
bad" (has > in it). The second option looks a bit better, add such a
failing test.
For "name> <email>" no error is reported. Looks like a bug, so add
such a failing test."
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
t/t1450-fsck.sh | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index bb01d5a..fc7ee8e 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -110,6 +110,30 @@ test_expect_success 'email with embedded > is not okay' '
grep "error in commit $new" out
'
+test_expect_failure 'missing < email delimiter is reported nicely' '
+ git cat-file commit HEAD >basis &&
+ sed "s/<//" basis >bad-email-2 &&
+ new=$(git hash-object -t commit -w --stdin <bad-email-2) &&
+ test_when_finished "remove_object $new" &&
+ git update-ref refs/heads/bogus "$new" &&
+ test_when_finished "git update-ref -d refs/heads/bogus" &&
+ git fsck 2>out &&
+ cat out &&
+ grep "error in commit $new.* - bad name" out
+'
+
+test_expect_failure '> in name is reported' '
+ git cat-file commit HEAD >basis &&
+ sed "s/ </> </" basis >bad-email-3 &&
+ new=$(git hash-object -t commit -w --stdin <bad-email-3) &&
+ test_when_finished "remove_object $new" &&
+ git update-ref refs/heads/bogus "$new" &&
+ test_when_finished "git update-ref -d refs/heads/bogus" &&
+ git fsck 2>out &&
+ cat out &&
+ grep "error in commit $new" out
+'
+
test_expect_success 'tag pointing to nonexistent' '
cat >invalid-tag <<-\EOF &&
object ffffffffffffffffffffffffffffffffffffffff
--
1.7.3.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/5] fsck: improve committer/author check
2011-07-28 5:43 [PATCH 0/5] fixes for committer/author parsing/check Dmitry Ivankov
` (3 preceding siblings ...)
2011-07-28 5:44 ` [PATCH 4/5] fsck: add a few committer name tests Dmitry Ivankov
@ 2011-07-28 5:44 ` Dmitry Ivankov
2011-08-02 17:00 ` Junio C Hamano
4 siblings, 1 reply; 11+ messages in thread
From: Dmitry Ivankov @ 2011-07-28 5:44 UTC (permalink / raw)
To: git; +Cc: SASAKI Suguru, Dmitry Ivankov
Neither name nor email should contain < or >, so split the string with
these and check they come in that order and < is preceeded with a space.
If < is missing don't say a confusing "missing space", say "bad name" if
> isn't missing and "missing email" if both < and > are missing.
Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
---
fsck.c | 10 ++++++----
t/t1450-fsck.sh | 4 ++--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/fsck.c b/fsck.c
index 60bd4bb..6c855f8 100644
--- a/fsck.c
+++ b/fsck.c
@@ -224,13 +224,15 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)
{
- if (**ident == '<' || **ident == '\n')
- return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing space before email");
- *ident += strcspn(*ident, "<\n");
- if ((*ident)[-1] != ' ')
+ if (**ident == '<')
return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing space before email");
+ *ident += strcspn(*ident, "<>\n");
+ if (**ident == '>')
+ return error_func(obj, FSCK_ERROR, "invalid author/committer line - bad name");
if (**ident != '<')
return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing email");
+ if ((*ident)[-1] != ' ')
+ return error_func(obj, FSCK_ERROR, "invalid author/committer line - missing space before email");
(*ident)++;
*ident += strcspn(*ident, "<>\n");
if (**ident != '>')
diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh
index fc7ee8e..0b92d0f 100755
--- a/t/t1450-fsck.sh
+++ b/t/t1450-fsck.sh
@@ -110,7 +110,7 @@ test_expect_success 'email with embedded > is not okay' '
grep "error in commit $new" out
'
-test_expect_failure 'missing < email delimiter is reported nicely' '
+test_expect_success 'missing < email delimiter is reported nicely' '
git cat-file commit HEAD >basis &&
sed "s/<//" basis >bad-email-2 &&
new=$(git hash-object -t commit -w --stdin <bad-email-2) &&
@@ -122,7 +122,7 @@ test_expect_failure 'missing < email delimiter is reported nicely' '
grep "error in commit $new.* - bad name" out
'
-test_expect_failure '> in name is reported' '
+test_expect_success '> in name is reported' '
git cat-file commit HEAD >basis &&
sed "s/ </> </" basis >bad-email-3 &&
new=$(git hash-object -t commit -w --stdin <bad-email-3) &&
--
1.7.3.4
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 5/5] fsck: improve committer/author check
2011-07-28 5:44 ` [PATCH 5/5] fsck: improve committer/author check Dmitry Ivankov
@ 2011-08-02 17:00 ` Junio C Hamano
2011-08-02 19:50 ` Dmitry Ivankov
0 siblings, 1 reply; 11+ messages in thread
From: Junio C Hamano @ 2011-08-02 17:00 UTC (permalink / raw)
To: Dmitry Ivankov; +Cc: git, SASAKI Suguru
Dmitry Ivankov <divanorama@gmail.com> writes:
> Neither name nor email should contain < or >, so split the string with
> these and check they come in that order and < is preceeded with a space.
>
> If < is missing don't say a confusing "missing space", say "bad name" if
>> isn't missing and "missing email" if both < and > are missing.
>
> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
> ---
Same comment as 3/5; before starting to talk about how you implemented
your validation, please state what rules you are enforcing.
Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] fsck: improve committer/author check
2011-08-02 17:00 ` Junio C Hamano
@ 2011-08-02 19:50 ` Dmitry Ivankov
0 siblings, 0 replies; 11+ messages in thread
From: Dmitry Ivankov @ 2011-08-02 19:50 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, SASAKI Suguru
On Tue, Aug 2, 2011 at 11:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Dmitry Ivankov <divanorama@gmail.com> writes:
>
>> Neither name nor email should contain < or >, so split the string with
>> these and check they come in that order and < is preceeded with a space.
>>
>> If < is missing don't say a confusing "missing space", say "bad name" if
>>> isn't missing and "missing email" if both < and > are missing.
>>
>> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
>> ---
>
> Same comment as 3/5; before starting to talk about how you implemented
> your validation, please state what rules you are enforcing.
Thanks, will apply and reroll.
But before this, is it ok to reject "Name> <email>" idents in fsck and
fast-import?
fsck already denies '<' and '>' inside email, and '<' in name. But
accepts '>' in name.
It just hit me that Documentation/fast-import.c doesn't deny '>' in
name and it is
consistent with fsck, so there may be a reason behind it.
>
> Thanks.
>
^ permalink raw reply [flat|nested] 11+ messages in thread