{"thread":{"id":"23583","subject":"[PATCH] fast-import docs: LT is valid in email, GT is not","startedAt":"2010-04-24T00:45:44Z","lastAt":"2010-05-04T17:11:13Z","messageCount":13,"participants":["Mark Lodato","Jonathan Nieder","Shawn O. Pearce","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"140249","messageId":"1272069944-20626-1-git-send-email-lodatom@gmail.com","threadId":"23583","inReplyTo":null,"subject":"[PATCH] fast-import docs: LT is valid in email, GT is not","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-04-24T00:45:44Z","receivedAt":"2010-04-24T00:45:44Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"In git-fast-import(1), fix a mistake that said LT and LF were the\ninvalid characters in email addresses.  This should have been GT and LF,\nsince the GT ends the email address and LF ends the command.\n\nSigned-off-by: Mark Lodato <lodatom@gmail.com>\n---\n Documentation/git-fast-import.txt |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 19082b0..6dcd583 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -394,7 +394,7 @@ Here `<name>` is the person's display name (for example\n and greater-than (\\x3e) symbols.  These are required to delimit\n the email address from the other fields in the line.  Note that\n `<name>` is free-form and may contain any sequence of bytes, except\n-`LT` and `LF`.  It is typically UTF-8 encoded.\n+`GT` and `LF`.  It is typically UTF-8 encoded.\n \n The time of the change is specified by `<when>` using the date format\n that was selected by the \\--date-format=<fmt> command line option.\n-- \n1.7.0.2\n"},{"id":"140282","messageId":"20100424160608.GA14690@progeny.tock","threadId":"23583","inReplyTo":"1272069944-20626-1-git-send-email-lodatom@gmail.com","subject":"[PATCH] fsck: check ident lines in commit objects","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-24T16:06:08Z","receivedAt":"2010-04-24T16:06:08Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Check that email addresses do not contain <, >, or newline so they can\nbe quickly scanned without trouble.  The copy() function in ident.c\nalready ensures that ordinary git commands will not write email\naddresses without this property.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nThoughts?  Should some of these errors be warnings?\n\ngit fast-import is capable of producing commits with some of these\nproblems: for example, it is fine with\n\n\tcommitter C O Mitter <foo@b>ar.net> 005 -    +5\n\n fsck.c          |   47 +++++++++++++++++++++++++++++++++++++++++++++++\n t/t1450-fsck.sh |   25 +++++++++++++++++++++++++\n 2 files changed, 72 insertions(+), 0 deletions(-)\n\ndiff --git a/fsck.c b/fsck.c\nindex 89278c1..ae9ae1a 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -222,12 +222,47 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n \treturn retval;\n }\n \n+static int fsck_ident(char **ident, struct object *obj, fsck_error error_func)\n+{\n+\tif (**ident == '<' || **ident == '\\n')\n+\t\treturn error_func(obj, FSCK_ERROR, \"invalid author/committer line - missing space before email\");\n+\t*ident += strcspn(*ident, \"<\\n\");\n+\tif ((*ident)[-1] != ' ')\n+\t\treturn error_func(obj, FSCK_ERROR, \"invalid author/committer line - missing space before email\");\n+\tif (**ident != '<')\n+\t\treturn error_func(obj, FSCK_ERROR, \"invalid author/committer line - missing email\");\n+\t(*ident)++;\n+\t*ident += strcspn(*ident, \"<>\\n\");\n+\tif (**ident != '>')\n+\t\treturn error_func(obj, FSCK_ERROR, \"invalid author/committer line - bad email\");\n+\t(*ident)++;\n+\tif (**ident != ' ')\n+\t\treturn error_func(obj, FSCK_ERROR, \"invalid author/committer line - missing space before date\");\n+\t(*ident)++;\n+\tif (**ident == '0' && (*ident)[1] != ' ')\n+\t\treturn error_func(obj, FSCK_ERROR, \"invalid author/committer line - zero-padded date\");\n+\t*ident += strspn(*ident, \"0123456789\");\n+\tif (**ident != ' ')\n+\t\treturn error_func(obj, FSCK_ERROR, \"invalid author/committer line - bad date\");\n+\t(*ident)++;\n+\tif ((**ident != '+' && **ident != '-') ||\n+\t    !isdigit((*ident)[1]) ||\n+\t    !isdigit((*ident)[2]) ||\n+\t    !isdigit((*ident)[3]) ||\n+\t    !isdigit((*ident)[4]) ||\n+\t    ((*ident)[5] != '\\n'))\n+\t\treturn error_func(obj, FSCK_ERROR, \"invalid author/committer line - bad time zone\");\n+\t(*ident) += 6;\n+\treturn 0;\n+}\n+\n static int fsck_commit(struct commit *commit, fsck_error error_func)\n {\n \tchar *buffer = commit->buffer;\n \tunsigned char tree_sha1[20], sha1[20];\n \tstruct commit_graft *graft;\n \tint parents = 0;\n+\tint err;\n \n \tif (commit->date == ULONG_MAX)\n \t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid author/committer line\");\n@@ -266,6 +301,18 @@ static int fsck_commit(struct commit *commit, fsck_error error_func)\n \t}\n \tif (memcmp(buffer, \"author \", 7))\n \t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid format - expected 'author' line\");\n+\tbuffer += 7;\n+\terr = fsck_ident(&buffer, &commit->object, error_func);\n+\tif (err)\n+\t\treturn err;\n+\tif (memcmp(buffer, \"committer \", strlen(\"committer \")))\n+\t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid format - expected 'committer' line\");\n+\tbuffer += strlen(\"committer \");\n+\terr = fsck_ident(&buffer, &commit->object, error_func);\n+\tif (err)\n+\t\treturn err;\n+\tif (*buffer != '\\n')\n+\t\treturn error_func(&commit->object, FSCK_ERROR, \"invalid format - expected blank line\");\n \tif (!commit->tree)\n \t\treturn error_func(&commit->object, FSCK_ERROR, \"could not load commit's tree %s\", sha1_to_hex(tree_sha1));\n \ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex 49cae3e..d8eed9b 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -57,6 +57,31 @@ test_expect_success 'branch pointing to non-commit' '\n \tgit update-ref -d refs/heads/invalid\n '\n \n+new=nothing\n+test_expect_success 'email without @ is okay' '\n+\tgit cat-file commit HEAD >basis &&\n+\tsed \"s/@/AT/\" basis >okay &&\n+\tnew=$(git hash-object -t commit -w --stdin <okay) &&\n+\techo \"$new\" &&\n+\tgit update-ref refs/heads/bogus \"$new\" &&\n+\tgit fsck\n+'\n+git update-ref -d refs/heads/bogus\n+rm -f \".git/objects/$new\"\n+\n+new=nothing\n+test_expect_success 'email with embedded > is not okay' '\n+\tgit cat-file commit HEAD >basis &&\n+\tsed \"s/@[a-z]/&>/\" basis >bad-email &&\n+\tnew=$(git hash-object -t commit -w --stdin <bad-email) &&\n+\techo \"$new\" &&\n+\tgit update-ref refs/heads/bogus \"$new\" &&\n+\tgit fsck 2>out &&\n+\tgrep \"error in commit $new\" out\n+'\n+git update-ref -d refs/heads/bogus\n+rm -f \".git/objects/$new\"\n+\n cat > invalid-tag <<EOF\n object ffffffffffffffffffffffffffffffffffffffff\n type commit\n-- \n1.7.0.6.2.g02f3f0.dirty\n"},{"id":"140284","messageId":"20100424161236.GB14690@progeny.tock","threadId":"23583","inReplyTo":"1272069944-20626-1-git-send-email-lodatom@gmail.com","subject":"Re: [PATCH] fast-import docs: LT is valid in email, GT is not","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-24T16:12:36Z","receivedAt":"2010-04-24T16:12:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi Mark,\n\nMark Lodato wrote:\n\n> +++ b/Documentation/git-fast-import.txt\n> @@ -394,7 +394,7 @@ Here `<name>` is the person's display name (for example\n>  and greater-than (\\x3e) symbols.  These are required to delimit\n>  the email address from the other fields in the line.  Note that\n>  `<name>` is free-form and may contain any sequence of bytes, except\n> -`LT` and `LF`.  It is typically UTF-8 encoded.\n> +`GT` and `LF`.  It is typically UTF-8 encoded.\n\n\tHere <name> is the person’s display name (for example\n\t“Com M Itter”)\n\nSo the original text is correct --- a <name> cannot contain LT because\na less-than sign marks the boundary between a name and email address.\n\nMaybe you were wondering what characters are valid in an e-mail address?\nThe comments in fast-import.c and code in ident.c are consistent about\nthis: the forbidden characters are <, >, and LF, though no one seems to\ncheck (see also my other reply).  A patch to explain this (including a\nreference to git-commit-tree(1), I guess) might be useful.\n\ngit won’t understand an email with embedded > or LF.  I’m not sure a <\nwould cause problems, but I don’t mind that it is disallowed.\n\nHope that helps,\nJonathan\n"},{"id":"140287","messageId":"20100424165900.GC14690@progeny.tock","threadId":"23583","inReplyTo":"20100424160608.GA14690@progeny.tock","subject":"Re: [PATCH] fsck: check ident lines in commit objects","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-24T16:59:00Z","receivedAt":"2010-04-24T16:59:00Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> Check that email addresses do not contain <, >, or newline so they can\n> be quickly scanned without trouble.\n\nTest was bogus: the object format tests in fsck do not affect its exit\ncode.  Here’s a fixup.\n\nSorry for the trouble,\nJonathan\n\ndiff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh\nindex d8eed9b..22a80c8 100755\n--- a/t/t1450-fsck.sh\n+++ b/t/t1450-fsck.sh\n@@ -64,7 +64,9 @@ test_expect_success 'email without @ is okay' '\n \tnew=$(git hash-object -t commit -w --stdin <okay) &&\n \techo \"$new\" &&\n \tgit update-ref refs/heads/bogus \"$new\" &&\n-\tgit fsck\n+\tgit fsck 2>out &&\n+\tcat out &&\n+\t! grep \"error in commit $new\" out\n '\n git update-ref -d refs/heads/bogus\n rm -f \".git/objects/$new\"\n@@ -77,6 +79,7 @@ test_expect_success 'email with embedded > is not okay' '\n \techo \"$new\" &&\n \tgit update-ref refs/heads/bogus \"$new\" &&\n \tgit fsck 2>out &&\n+\tcat out &&\n \tgrep \"error in commit $new\" out\n '\n git update-ref -d refs/heads/bogus\n"},{"id":"140288","messageId":"i2hca433831004240959s227a96as2caf523da94dff51@mail.gmail.com","threadId":"23583","inReplyTo":"20100424161236.GB14690@progeny.tock","subject":"Re: [PATCH] fast-import docs: LT is valid in email, GT is not","fromName":"Mark Lodato","fromEmail":"lodatom@gmail.com","sentAt":"2010-04-24T16:59:01Z","receivedAt":"2010-04-24T16:59:01Z","isPatch":true,"sender":{"key":"lodatom@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58860?v=4"},"body":"On Sat, Apr 24, 2010 at 12:12 PM, Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Mark Lodato wrote:\n>> +++ b/Documentation/git-fast-import.txt\n>> @@ -394,7 +394,7 @@ Here `<name>` is the person's display name (for example\n>>  and greater-than (\\x3e) symbols.  These are required to delimit\n>>  the email address from the other fields in the line.  Note that\n>>  `<name>` is free-form and may contain any sequence of bytes, except\n>> -`LT` and `LF`.  It is typically UTF-8 encoded.\n>> +`GT` and `LF`.  It is typically UTF-8 encoded.\n>\n>        Here <name> is the person’s display name (for example\n>        “Com M Itter”)\n>\n> So the original text is correct --- a <name> cannot contain LT because\n> a less-than sign marks the boundary between a name and email address.\n\nAh, you're right, sorry.  I thought it was <email>, not <name>.\n\n> Maybe you were wondering what characters are valid in an e-mail address?\n> The comments in fast-import.c and code in ident.c are consistent about\n> this: the forbidden characters are <, >, and LF, though no one seems to\n> check (see also my other reply).  A patch to explain this (including a\n> reference to git-commit-tree(1), I guess) might be useful.\n>\n> git won’t understand an email with embedded > or LF.  I’m not sure a <\n> would cause problems, but I don’t mind that it is disallowed.\n\nIt seems like it would be good to disallow <, >, and LF in both name\nand email.  With your other patch, > is allowed in the name.\n\nThanks for the clarification,\nMark\n"},{"id":"140294","messageId":"20100424190419.GA7502@spearce.org","threadId":"23583","inReplyTo":"20100424160608.GA14690@progeny.tock","subject":"Re: [PATCH] fsck: check ident lines in commit objects","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2010-04-24T19:04:19Z","receivedAt":"2010-04-24T19:04:19Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> wrote:\n> Check that email addresses do not contain <, >, or newline so they can\n> be quickly scanned without trouble.  The copy() function in ident.c\n> already ensures that ordinary git commands will not write email\n> addresses without this property.\n> \n> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>\n> ---\n> Thoughts?  Should some of these errors be warnings?\n\nThese should be errors.  We should never see this sort of thing\noccur in a live repository.\n \n> git fast-import is capable of producing commits with some of these\n> problems: for example, it is fine with\n> \n> \tcommitter C O Mitter <foo@b>ar.net> 005 -    +5\n\nYuck.  We probably should tighten up the parser in fast-import a\nbit more.  The above is pretty insane for it to produce into the\nrepository.  I can't even begin to count how many ways the above\nline is just wrong...  :-)\n\n-- \nShawn.\n"},{"id":"140300","messageId":"20100424203827.GA24948@progeny.tock","threadId":"23583","inReplyTo":"20100424190419.GA7502@spearce.org","subject":"[PATCH 0/2] fast-import: tighten up parsing ident line","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-24T20:38:27Z","receivedAt":"2010-04-24T20:38:27Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Shawn O. Pearce wrote:\n> Jonathan Nieder <jrnieder@gmail.com> wrote:\n\n>> git fast-import is capable of producing commits with some of these\n>> problems: for example, it is fine with\n>> \n>> \tcommitter C O Mitter <foo@b>ar.net> 005 -    +5\n>\n> Yuck.  We probably should tighten up the parser in fast-import a\n> bit more.\n\nHow about this?\n\nJonathan Nieder (2):\n  fast-import: be strict about formatting of dates\n  fast-import: validate entire ident string\n\n Documentation/git-fast-import.txt |    9 ++--\n fast-import.c                     |   63 ++++++++++++++--------\n t/t9300-fast-import.sh            |  105 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 150 insertions(+), 27 deletions(-)\n"},{"id":"140301","messageId":"20100424205036.GB24948@progeny.tock","threadId":"23583","inReplyTo":"20100424203827.GA24948@progeny.tock","subject":"[PATCH 1/2] fast-import: be strict about formatting of raw dates","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-24T20:50:36Z","receivedAt":"2010-04-24T20:50:36Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"git proper never zero- or space-pads its dates and its time zones are\nalways 4 digits long.  Require fast-import front-ends to behave\nlikewise to avoid generating puzzling objects.\n\nSince this makes the input format more strict, some front-ends may not\nbe happy.  But they were warned:\n\n   This is the Git native format and is <time> SP <offutc>. It is also\n   fast-import’s default format, if --date-format was not specified.\n\n   Unlike the rfc2822 format, this format is very strict. Any variation\n   in formatting will cause fast-import to reject the value.\n\nAside from ensuring the format is predictable so tools like git can\nhandle it, making the date format this strict ensures that there is\nonly one valid representation for a given date and time zone, which\nwould be useful for round-trip conversion of objects to and from other\nformats (for storage by other version control systems, for example).\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nIs -0000 the same time zone as +0000?  I wasn’t sure so I erred on the\nside of not worrying about it.\n\n fast-import.c          |    9 +++++++--\n t/t9300-fast-import.sh |   30 ++++++++++++++++++++++++++++++\n 2 files changed, 37 insertions(+), 2 deletions(-)\n\ndiff --git a/fast-import.c b/fast-import.c\nindex 74f08bd..1701cf1 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -1906,6 +1906,8 @@ static int validate_raw_date(const char *src, char *result, int maxlen)\n \n \terrno = 0;\n \n+\tif ((*src == '0' && isdigit(src[1])) || !isdigit(*src))\n+\t\treturn -1;\n \tnum = strtoul(src, &endp, 10);\n \t/* NEEDSWORK: perhaps check for reasonable values? */\n \tif (errno || endp == src || *endp != ' ')\n@@ -1915,8 +1917,11 @@ static int validate_raw_date(const char *src, char *result, int maxlen)\n \tif (*src != '-' && *src != '+')\n \t\treturn -1;\n \n-\tnum = strtoul(src + 1, &endp, 10);\n-\tif (errno || endp == src + 1 || *endp || (endp - orig_src) >= maxlen ||\n+\tsrc++;\n+\tif (*src != '0' && *src != '1')\n+\t\treturn -1;\n+\tnum = strtoul(src, &endp, 10);\n+\tif (errno || endp != src + 4 || *endp || (endp - orig_src) >= maxlen ||\n \t    1400 < num)\n \t\treturn -1;\n \ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 131f032..ed653a7 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -348,6 +348,36 @@ test_expect_success \\\n \n cat >input <<INPUT_END\n commit refs/heads/branch\n+author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> 1170783301 -  0500\n+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 117078330 -0500\n+data <<COMMIT\n+Malformed time zone\n+COMMIT\n+\n+from refs/heads/branch^0\n+\n+INPUT_END\n+test_expect_success 'E: blanks in raw time zone' '\n+    test_must_fail git fast-import --date-format=raw <input\n+'\n+\n+cat >input <<INPUT_END\n+commit refs/heads/branch\n+author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> 01170783301 -0500\n+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 117078330 -0500\n+data <<COMMIT\n+Malformed date\n+COMMIT\n+\n+from refs/heads/branch^0\n+\n+INPUT_END\n+test_expect_success 'E: leading zero in raw date' '\n+    test_must_fail git fast-import --date-format=raw <input\n+'\n+\n+cat >input <<INPUT_END\n+commit refs/heads/branch\n author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> Tue Feb 6 11:22:18 2007 -0500\n committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> Tue Feb 6 12:35:02 2007 -0500\n data <<COMMIT\n-- \n1.7.1.rc1\n"},{"id":"140302","messageId":"20100424211042.GC24948@progeny.tock","threadId":"23583","inReplyTo":"20100424203827.GA24948@progeny.tock","subject":"[PATCH 2/2] fast-import: validate entire ident string","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-24T21:10:42Z","receivedAt":"2010-04-24T21:10:42Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"The author, committer, and tagger name and email should not include\nany embedded <, >, or newline characters.  The format of the\nidentification string is\n\n  ('author'|'committer'|'tagger') sp name sp < email > sp date\n\nIf an object has no name attached, then git expects to find two spaces\nin a row.\n\nHelped-by: Mark Lodato <lodatom@gmail.com>\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\nFor malformed input, the parser in pretty.c and ‘git commit --amend’\ntend to end up with different ideas of who the author is.  A lot of\nthe time, commit --amend gives up with \"fatal: invalid commit\".\n\n Documentation/git-fast-import.txt |    9 ++--\n fast-import.c                     |   54 ++++++++++++++++----------\n t/t9300-fast-import.sh            |   75 +++++++++++++++++++++++++++++++++++++\n 3 files changed, 113 insertions(+), 25 deletions(-)\n\ndiff --git a/Documentation/git-fast-import.txt b/Documentation/git-fast-import.txt\nindex 19082b0..ee725c6 100644\n--- a/Documentation/git-fast-import.txt\n+++ b/Documentation/git-fast-import.txt\n@@ -337,8 +337,8 @@ change to the project.\n ....\n \t'commit' SP <ref> LF\n \tmark?\n-\t('author' (SP <name>)? SP LT <email> GT SP <when> LF)?\n-\t'committer' (SP <name>)? SP LT <email> GT SP <when> LF\n+\t('author' SP <name>? SP LT <email> GT SP <when> LF)?\n+\t'committer' SP <name>? SP LT <email> GT SP <when> LF\n \tdata\n \t('from' SP <committish> LF)?\n \t('merge' SP <committish> LF)?\n@@ -393,8 +393,9 @@ Here `<name>` is the person's display name (for example\n (``cm@example.com'').  `LT` and `GT` are the literal less-than (\\x3c)\n and greater-than (\\x3e) symbols.  These are required to delimit\n the email address from the other fields in the line.  Note that\n-`<name>` is free-form and may contain any sequence of bytes, except\n-`LT` and `LF`.  It is typically UTF-8 encoded.\n+`<name>` and `<email>` are free-form and may contain any sequence\n+of bytes that are not `LT`, `GT`, or `LF`.  Both are typically UTF-8\n+encoded.\n \n The time of the change is specified by `<when>` using the date format\n that was selected by the \\--date-format=<fmt> command line option.\ndiff --git a/fast-import.c b/fast-import.c\nindex 1701cf1..d919168 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -19,8 +19,8 @@ Format of STDIN stream:\n \n   new_commit ::= 'commit' sp ref_str lf\n     mark?\n-    ('author' (sp name)? sp '<' email '>' sp when lf)?\n-    'committer' (sp name)? sp '<' email '>' sp when lf\n+    ('author' sp name? sp '<' email '>' sp when lf)?\n+    'committer' sp name? sp '<' email '>' sp when lf\n     commit_msg\n     ('from' sp committish lf)?\n     ('merge' sp committish lf)*\n@@ -47,7 +47,7 @@ Format of STDIN stream:\n \n   new_tag ::= 'tag' sp tag_str lf\n     'from' sp committish lf\n-    ('tagger' (sp name)? sp '<' email '>' sp when lf)?\n+    ('tagger' sp name? sp '<' email '>' sp when lf)?\n     tag_msg;\n   tag_msg ::= data;\n \n@@ -123,9 +123,8 @@ Format of STDIN stream:\n   sha1exp ::= # Any valid GIT SHA1 expression;\n   hexsha1 ::= # SHA1 in hexadecimal format;\n \n-     # note: name and email are UTF8 strings, however name must not\n-     # contain '<' or lf and email must not contain any of the\n-     # following: '<', '>', lf.\n+     # note: name and email are UTF8 strings, however name and email\n+     # must not contain any of the following: '<', '>', lf.\n      #\n   name  ::= # valid GIT author/committer name;\n   email ::= # valid GIT author/committer email;\n@@ -1929,34 +1928,47 @@ static int validate_raw_date(const char *src, char *result, int maxlen)\n \treturn 0;\n }\n \n-static char *parse_ident(const char *buf)\n+static size_t parse_name_and_email(const char *src, char **result, size_t extra)\n {\n-\tconst char *gt;\n+\tconst char *lt, *gt;\n \tsize_t name_len;\n-\tchar *ident;\n \n-\tgt = strrchr(buf, '>');\n-\tif (!gt)\n-\t\tdie(\"Missing > in ident string: %s\", buf);\n+\tlt = src + strcspn(src, \"<>\\n\");\n+\tif (lt == src || lt[-1] != ' ' || *lt != '<')\n+\t\tdie(\"Invalid name in ident string: %s\", src);\n+\tgt = lt + 1 + strcspn(lt + 1, \"<>\\n\");\n+\tif (*gt != '>')\n+\t\tdie(\"Invalid email in ident string: %s\", src);\n \tgt++;\n \tif (*gt != ' ')\n-\t\tdie(\"Missing space after > in ident string: %s\", buf);\n+\t\tdie(\"Missing space after > in ident string: %s\", src);\n \tgt++;\n-\tname_len = gt - buf;\n-\tident = xmalloc(name_len + 24);\n-\tstrncpy(ident, buf, name_len);\n+\tname_len = gt - src;\n+\t*result = xmalloc(name_len + extra);\n+\tmemcpy(*result, src, name_len);\n+\treturn name_len;\n+}\n+\n+static char *parse_ident(const char *buf)\n+{\n+\tconst char *date;\n+\tsize_t name_len;\n+\tchar *ident;\n+\n+\tname_len = parse_name_and_email(buf, &ident, 24);\n+\tdate = buf + name_len;\n \n \tswitch (whenspec) {\n \tcase WHENSPEC_RAW:\n-\t\tif (validate_raw_date(gt, ident + name_len, 24) < 0)\n-\t\t\tdie(\"Invalid raw date \\\"%s\\\" in ident: %s\", gt, buf);\n+\t\tif (validate_raw_date(date, ident + name_len, 24) < 0)\n+\t\t\tdie(\"Invalid raw date \\\"%s\\\" in ident: %s\", date, buf);\n \t\tbreak;\n \tcase WHENSPEC_RFC2822:\n-\t\tif (parse_date(gt, ident + name_len, 24) < 0)\n-\t\t\tdie(\"Invalid rfc2822 date \\\"%s\\\" in ident: %s\", gt, buf);\n+\t\tif (parse_date(date, ident + name_len, 24) < 0)\n+\t\t\tdie(\"Invalid rfc2822 date \\\"%s\\\" in ident: %s\", date, buf);\n \t\tbreak;\n \tcase WHENSPEC_NOW:\n-\t\tif (strcmp(\"now\", gt))\n+\t\tif (strcmp(\"now\", date))\n \t\t\tdie(\"Date in ident must be 'now': %s\", buf);\n \t\tdatestamp(ident + name_len, 24);\n \t\tbreak;\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex ed653a7..a7e379f 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -348,6 +348,81 @@ test_expect_success \\\n \n cat >input <<INPUT_END\n commit refs/heads/branch\n+author <$GIT_AUTHOR_EMAIL> Sat, 24 Apr 2010 14:49:52 -0500\n+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> Tue Feb 6 12:35:01 2007 -0500\n+data <<COMMIT\n+Nameless author, first attempt\n+COMMIT\n+\n+from refs/heads/branch^0\n+\n+INPUT_END\n+test_expect_success 'E: require space after author name' '\n+    test_must_fail git fast-import --date-format=rfc2822 <input\n+'\n+\n+cat >input <<INPUT_END\n+commit refs/heads/branch\n+author  <$GIT_AUTHOR_EMAIL> Sat, 24 Apr 2010 14:49:52 -0500\n+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> Tue Feb 6 12:35:01 2007 -0500\n+data <<COMMIT\n+Nameless author\n+COMMIT\n+\n+from refs/heads/branch^0\n+\n+INPUT_END\n+test_expect_success 'E: do not require author name, though' '\n+    git fast-import --date-format=rfc2822 <input\n+'\n+\n+cat >input <<INPUT_END\n+commit refs/heads/branch\n+author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> Sat, 24 Apr 2010 14:49:52 -0500\n+committer C O >Mitter <$GIT_COMMITTER_EMAIL> Tue Feb 6 12:35:01 2007 -0500\n+data <<COMMIT\n+Odd committer\n+COMMIT\n+\n+from refs/heads/branch^0\n+\n+INPUT_END\n+test_expect_success 'E: unparsable committer' '\n+    test_must_fail git fast-import --date-format=rfc2822 <input\n+'\n+\n+cat >input <<INPUT_END\n+commit refs/heads/branch\n+author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> Sat, 24 Apr 2010 15:05:27 -0500\n+committer $GIT_COMMITTER_NAME <aggh@<example.com> Tue Feb 6 12:35:01 2007 -0500\n+data <<COMMIT\n+Odd email\n+COMMIT\n+\n+from refs/heads/branch^0\n+\n+INPUT_END\n+test_expect_success 'E: unparsable email' '\n+    test_must_fail git fast-import --date-format=rfc2822 <input\n+'\n+\n+cat >input <<INPUT_END\n+commit refs/heads/branch\n+author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> Sat, 24 Apr 2010 15:05:27 -0500\n+committer $GIT_COMMITTER_NAME <äggh!some!other!machine!example> Tue Feb 6 12:35:01 2007 -0500\n+data <<COMMIT\n+Bang path\n+COMMIT\n+\n+from refs/heads/branch^0\n+\n+INPUT_END\n+test_expect_success 'E: okay email' '\n+    git fast-import --date-format=rfc2822 <input\n+'\n+\n+cat >input <<INPUT_END\n+commit refs/heads/branch\n author $GIT_AUTHOR_NAME <$GIT_AUTHOR_EMAIL> 1170783301 -  0500\n committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> 117078330 -0500\n data <<COMMIT\n-- \n1.7.1.rc1\n"},{"id":"140422","messageId":"20100426160247.GD7502@spearce.org","threadId":"23583","inReplyTo":"20100424211042.GC24948@progeny.tock","subject":"Re: [PATCH 2/2] fast-import: validate entire ident string","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2010-04-26T16:02:47Z","receivedAt":"2010-04-26T16:02:47Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> wrote:\n> The author, committer, and tagger name and email should not include\n> any embedded <, >, or newline characters.  The format of the\n> identification string is\n> \n>   ('author'|'committer'|'tagger') sp name sp < email > sp date\n> \n> If an object has no name attached, then git expects to find two spaces\n> in a row.\n\nThis is going to be a problem I think.  Some importers are probably\nwriting \"committer <bob> ....\" when pulling from systems that don't\nhave a concept of name vs. email (e.g. CVS or SVN).  I highly suspect\nthat requiring two spaces here will cause a lot of importers to fail.\n\nIf we really need to require two spaces, I think we need to honor\nthe documented input format but rewrite the line inside of the\nimport process to match the two space convention.\n \n-- \nShawn.\n"},{"id":"140423","messageId":"20100426162422.GA10859@progeny.tock","threadId":"23583","inReplyTo":"20100426160247.GD7502@spearce.org","subject":"Re: [PATCH 2/2] fast-import: validate entire ident string","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T16:24:22Z","receivedAt":"2010-04-26T16:24:22Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Shawn O. Pearce wrote:\n\n> Some importers are probably\n> writing \"committer <bob> ....\" when pulling from systems that don't\n> have a concept of name vs. email (e.g. CVS or SVN).  I highly suspect\n> that requiring two spaces here will cause a lot of importers to fail.\n> \n> If we really need to require two spaces,\n\nIt is not a huge deal, but ‘git commit --amend’ will die with \"invalid\ncommit\" if it does not find a “ <” sequence after the “author ”\nstring.  Maybe that should be changed.  Patch below.\n\n> I think we need to honor\n> the documented input format but rewrite the line inside of the\n> import process to match the two space convention.\n\nYes, that’s doable.\n\nThanks for the feedback,\nJonathan\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex c5ab683..c56f2c9 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -449,19 +449,23 @@ static void determine_author_info(void)\n \tdate = getenv(\"GIT_AUTHOR_DATE\");\n \n \tif (use_message && !renew_authorship) {\n-\t\tconst char *a, *lb, *rb, *eol;\n+\t\tconst char *a, *n, *lb, *rb, *eol;\n \n \t\ta = strstr(use_message_buffer, \"\\nauthor \");\n \t\tif (!a)\n \t\t\tdie(\"invalid commit: %s\", use_message);\n \n-\t\tlb = strstr(a + 8, \" <\");\n-\t\trb = strstr(a + 8, \"> \");\n-\t\teol = strchr(a + 8, '\\n');\n+\t\tn = a + strlen(\"\\nauthor\");\n+\t\tlb = strstr(n, \" <\");\n+\t\trb = strstr(lb + 2, \"> \");\n+\t\teol = strchr(rb + 2, '\\n');\n \t\tif (!lb || !rb || !eol)\n \t\t\tdie(\"invalid commit: %s\", use_message);\n \n-\t\tname = xstrndup(a + 8, lb - (a + 8));\n+\t\tif (lb == n)\n+\t\t\tname = xstrndup(\"\", 0);\n+\t\telse\n+\t\t\tname = xstrndup(n + 1, lb - (n + 1));\n \t\temail = xstrndup(lb + 2, rb - (lb + 2));\n \t\tdate = xstrndup(rb + 2, eol - (rb + 2));\n \t}\n@@ -470,7 +474,7 @@ static void determine_author_info(void)\n \t\tconst char *lb = strstr(force_author, \" <\");\n \t\tconst char *rb = strchr(force_author, '>');\n \n-\t\tif (!lb || !rb)\n+\t\tif (!lb || !rb || rb < lb)\n \t\t\tdie(\"malformed --author parameter\");\n \t\tname = xstrndup(force_author, lb - force_author);\n \t\temail = xstrndup(lb + 2, rb - (lb + 2));\n-- \n"},{"id":"140424","messageId":"20100426163032.GB10859@progeny.tock","threadId":"23583","inReplyTo":"20100426162422.GA10859@progeny.tock","subject":"Re: [PATCH 2/2] fast-import: validate entire ident string","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2010-04-26T16:30:33Z","receivedAt":"2010-04-26T16:30:33Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Jonathan Nieder wrote:\n\n> -\t\tlb = strstr(a + 8, \" <\");\n> -\t\trb = strstr(a + 8, \"> \");\n> -\t\teol = strchr(a + 8, '\\n');\n> +\t\tn = a + strlen(\"\\nauthor\");\n> +\t\tlb = strstr(n, \" <\");\n> +\t\trb = strstr(lb + 2, \"> \");\n> +\t\teol = strchr(rb + 2, '\\n');\n>  \t\tif (!lb || !rb || !eol)\n>  \t\t\tdie(\"invalid commit: %s\", use_message);\n\nErr, this will segv when it fails; better to use\n\n\tlb = a + strlen(\"\\nauthor \");\n\tlb = strchrnul(lb, '<');\n\trb = strchrnul(lb, '>');\n\teol = strchrnul(rb, '\\n');\n\tif (!*lb || !*rb || !*eol)\n\t\tdie(\"invalid commit: %s\", use_message);\n\nThis is even more permissive, but I think that’s okay.\n\nSorry for the noise.\nJonathan\n"},{"id":"140924","messageId":"7vk4rjmw26.fsf@alter.siamese.dyndns.org","threadId":"23583","inReplyTo":"20100426160247.GD7502@spearce.org","subject":"Re: [PATCH 2/2] fast-import: validate entire ident string","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-05-04T17:11:13Z","receivedAt":"2010-05-04T17:11:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Shawn O. Pearce\" <spearce@spearce.org> writes:\n\n> Jonathan Nieder <jrnieder@gmail.com> wrote:\n>> The author, committer, and tagger name and email should not include\n>> any embedded <, >, or newline characters.  The format of the\n>> identification string is\n>> \n>>   ('author'|'committer'|'tagger') sp name sp < email > sp date\n>> \n>> If an object has no name attached, then git expects to find two spaces\n>> in a row.\n>\n> This is going to be a problem I think.  Some importers are probably\n> writing \"committer <bob> ....\" when pulling from systems that don't\n> have a concept of name vs. email (e.g. CVS or SVN).  I highly suspect\n> that requiring two spaces here will cause a lot of importers to fail.\n>\n> If we really need to require two spaces, I think we need to honor\n> the documented input format but rewrite the line inside of the\n> import process to match the two space convention.\n\nIt probably should document [sp name] (or [name sp]) an \"zero or one\"\nitem, if we want to be lenient.\n\nI also think it may not be such a bad idea to allow fast-import to add a\nphoney \"name\" by taking everything in e-mail before the first '@', just\nlike how the git-cvsimport and git-svn does, when the input stream does\nnot have a \"name\".  I am not sure how usable \"shortlog\" output would be\notherwise, without any \"name\" field.\n"}]}