{"thread":{"id":"24029","subject":"[PATCH] mailinfo: parse From header more strictly when run from rebase","startedAt":"2010-06-07T04:45:45Z","lastAt":"2010-06-15T17:01:40Z","messageCount":4,"participants":["Jay Soffian","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"143131","messageId":"1275885945-29241-1-git-send-email-jaysoffian@gmail.com","threadId":"24029","inReplyTo":null,"subject":"[PATCH] mailinfo: parse From header more strictly when run from rebase","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2010-06-07T04:45:45Z","receivedAt":"2010-06-07T04:45:45Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"Given an email address of the form:\n\n author@example.com \n\nresults in \"author@example.com\" being used as both the name and the\naddress. This is due to an assumption in handle_from() that the\nfirst '@' denotes the address portion. The remainder of the string\nis then assumed to be the name, which is rejected by\nget_sane_name(), resulting in address being used for the name as\nwell.\n\nI considered making handle_from() smarter by instead looking first\nfor an '@' contained by brackets. But get_sane_name() would still\nreject \"author@example.com\" for the name, resulting in\n\"author@example.com@11B4E7C6-762E-4CF5-B2EB-3F7BD596981D\" being used\nfor both the name and the address. Hardly an improvement.\n\nIn practice, I've only encountered this issue when rebasing commits\nthat were generated by git svn. So instead of adding more heuristics\nto handle_from(), just make it stricter when run from rebase.\n\nWe use undocumented --no-inbody-headers to know when we're running\nfrom rebase and then assume the address is of the form:\n\n name \n\nin which case email is extracted purely by brackets and name is\ntrimmed only for whitespace.\n\nSigned-off-by: Jay Soffian \n---\n builtin/mailinfo.c | 15 ++++++++++++---\n t/t5100-mailinfo.sh | 2 +-\n t/t5100/0017 | 17 +++++++++++++++++\n t/t5100/info0017 | 5 +++++\n t/t5100/info0017--no-inbody-headers | 5 +++++\n t/t5100/patch0017 | 12 ++++++++++++\n t/t5100/patch0017--no-inbody-headers | 12 ++++++++++++\n t/t5100/sample.mbox | 17 +++++++++++++++++\n 8 files changed, 81 insertions(+), 4 deletions(-)\n create mode 100644 t/t5100/0017\n create mode 100644 t/t5100/info0017\n create mode 100644 t/t5100/info0017--no-inbody-headers\n create mode 100644 t/t5100/msg0017\n create mode 100644 t/t5100/msg0017--no-inbody-headers\n create mode 100644 t/t5100/patch0017\n create mode 100644 t/t5100/patch0017--no-inbody-headers\n\ndiff --git a/builtin/mailinfo.c b/builtin/mailinfo.c\nindex 4a9729b..0cb5521 100644\n--- a/builtin/mailinfo.c\n+++ b/builtin/mailinfo.c\n@@ -47,7 +47,7 @@ static void get_sane_name(struct strbuf *out, struct strbuf *name, struct strbuf\n \tstrbuf_addbuf(out, src);\n }\n \n-static void parse_bogus_from(const struct strbuf *line)\n+static void parse_bracketed_from(const struct strbuf *line, int strict)\n {\n \t/* John Doe */\n \n@@ -71,7 +71,8 @@ static void parse_bogus_from(const struct strbuf *line)\n \tstrbuf_reset(&name);\n \tstrbuf_add(&name, line->buf, bra - line->buf);\n \tstrbuf_trim(&name);\n-\tget_sane_name(&name, &name, &email);\n+\tif (!strict)\n+\t\tget_sane_name(&name, &name, &email);\n }\n \n static void handle_from(const struct strbuf *from)\n@@ -80,12 +81,20 @@ static void handle_from(const struct strbuf *from)\n \tsize_t el;\n \tstruct strbuf f;\n \n+\tif (!use_inbody_headers) {\n+\t\t/* we're running from rebase, so we can assume the from header\n+\t\t * is well-formed and use the stricter parse_bracketed_from\n+\t\t */\n+\t\tparse_bracketed_from(from, 1);\n+\t\treturn;\n+\t}\n+\n \tstrbuf_init(&f, from->len);\n \tstrbuf_addbuf(&f, from);\n \n \tat = strchr(f.buf, '@');\n \tif (!at) {\n-\t\tparse_bogus_from(from);\n+\t\tparse_bracketed_from(from, 0);\n \t\treturn;\n \t}\n \ndiff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh\nindex ebc36c1..d8bbad8 100755\n--- a/t/t5100-mailinfo.sh\n+++ b/t/t5100-mailinfo.sh\n@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \\\n \t'git mailsplit -o. \"$TEST_DIRECTORY\"/t5100/sample.mbox >last &&\n \tlast=`cat last` &&\n \techo total is $last &&\n-\ttest `cat last` = 16'\n+\ttest `cat last` = 17'\n \n check_mailinfo () {\n \tmail=$1 opt=$2\ndiff --git a/t/t5100/0017 b/t/t5100/0017\nnew file mode 100644\nindex 0000000..085d414\n--- /dev/null\n+++ b/t/t5100/0017\n@@ -0,0 +1,17 @@\n+From 9251ac1cdbf624c31ec0a9996f0b38dc7bcdcd60 Mon Sep 17 00:00:00 2001\n+From: author@example.com \n+Date: Sun, 6 Jun 2010 22:14:37 -0400\n+Subject: initial\n+\n+---\n+ hello | 1 +\n+ 1 files changed, 1 insertions(+), 0 deletions(-)\n+ create mode 100644 hello\n+\n+diff --git a/hello b/hello\n+new file mode 100644\n+index 0000000..ce01362\n+--- /dev/null\n++++ b/hello\n+@@ -0,0 +1 @@\n++hello\ndiff --git a/t/t5100/info0017 b/t/t5100/info0017\nnew file mode 100644\nindex 0000000..992cfe0\n--- /dev/null\n+++ b/t/t5100/info0017\n@@ -0,0 +1,5 @@\n+Author: author@example.com\n+Email: author@example.com\n+Subject: initial\n+Date: Sun, 6 Jun 2010 22:14:37 -0400\n+\ndiff --git a/t/t5100/info0017--no-inbody-headers b/t/t5100/info0017--no-inbody-headers\nnew file mode 100644\nindex 0000000..9d74dce\n--- /dev/null\n+++ b/t/t5100/info0017--no-inbody-headers\n@@ -0,0 +1,5 @@\n+Author: author@example.com\n+Email: author@example.com@11B4E7C6-762E-4CF5-B2EB-3F7BD596981D\n+Subject: initial\n+Date: Sun, 6 Jun 2010 22:14:37 -0400\n+\ndiff --git a/t/t5100/msg0017 b/t/t5100/msg0017\nnew file mode 100644\nindex 0000000..e69de29\ndiff --git a/t/t5100/msg0017--no-inbody-headers b/t/t5100/msg0017--no-inbody-headers\nnew file mode 100644\nindex 0000000..e69de29\ndiff --git a/t/t5100/patch0017 b/t/t5100/patch0017\nnew file mode 100644\nindex 0000000..d5640e6\n--- /dev/null\n+++ b/t/t5100/patch0017\n@@ -0,0 +1,12 @@\n+---\n+ hello | 1 +\n+ 1 files changed, 1 insertions(+), 0 deletions(-)\n+ create mode 100644 hello\n+\n+diff --git a/hello b/hello\n+new file mode 100644\n+index 0000000..ce01362\n+--- /dev/null\n++++ b/hello\n+@@ -0,0 +1 @@\n++hello\ndiff --git a/t/t5100/patch0017--no-inbody-headers b/t/t5100/patch0017--no-inbody-headers\nnew file mode 100644\nindex 0000000..d5640e6\n--- /dev/null\n+++ b/t/t5100/patch0017--no-inbody-headers\n@@ -0,0 +1,12 @@\n+---\n+ hello | 1 +\n+ 1 files changed, 1 insertions(+), 0 deletions(-)\n+ create mode 100644 hello\n+\n+diff --git a/hello b/hello\n+new file mode 100644\n+index 0000000..ce01362\n+--- /dev/null\n++++ b/hello\n+@@ -0,0 +1 @@\n++hello\ndiff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox\nindex de10312..50f71ea 100644\n--- a/t/t5100/sample.mbox\n+++ b/t/t5100/sample.mbox\n@@ -683,3 +683,20 @@ index e69de29..d95f3ad 100644\n @@ -0,0 +1 @@\n +content\n \n+From 9251ac1cdbf624c31ec0a9996f0b38dc7bcdcd60 Mon Sep 17 00:00:00 2001\n+From: author@example.com \n+Date: Sun, 6 Jun 2010 22:14:37 -0400\n+Subject: initial\n+\n+---\n+ hello | 1 +\n+ 1 files changed, 1 insertions(+), 0 deletions(-)\n+ create mode 100644 hello\n+\n+diff --git a/hello b/hello\n+new file mode 100644\n+index 0000000..ce01362\n+--- /dev/null\n++++ b/hello\n+@@ -0,0 +1 @@\n++hello\n-- \n1.7.1\n"},{"id":"143757","messageId":"7vzkyw6zo2.fsf@alter.siamese.dyndns.org","threadId":"24029","inReplyTo":"1275885945-29241-1-git-send-email-jaysoffian@gmail.com","subject":"Re: [PATCH] mailinfo: parse From header more strictly when run from rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-06-15T16:14:37Z","receivedAt":"2010-06-15T16:14:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jay Soffian writes:\n\n> Given an email address of the form:\n>\n> author@example.com \n>\n> results in \"author@example.com\" being used as both the name and the\n> address. This is due to an assumption in handle_from() that the\n> first '@' denotes the address portion. The remainder of the string\n> is then assumed to be the name, which is rejected by\n> get_sane_name(), resulting in address being used for the name as\n> well.\n\nSorry, but it is unclear from your message what problem you are trying to\nsolve, what your desired outcome is, and why you think that outcome is\ndesired. The bracketed one shown above does _not_ look like a valid email\naddress, so using\n\n name = \"author@example.com\"\n email = \"author@example.com@11B4...\"\n\ndoes not sound like a good thing to do to begin with.\n"},{"id":"143763","messageId":"AANLkTina5G9P-OkGAoia0DeJhPygRaoaiI7tXDRHgjtF@mail.gmail.com","threadId":"24029","inReplyTo":"7vzkyw6zo2.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] mailinfo: parse From header more strictly when run from rebase","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2010-06-15T16:41:58Z","receivedAt":"2010-06-15T16:41:58Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, Jun 15, 2010 at 12:14 PM, Junio C Hamano wrote:\n> Jay Soffian writes:\n>\n>> Given an email address of the form:\n>>\n>>  author@example.com \n>>\n>> results in \"author@example.com\" being used as both the name and the\n>> address. This is due to an assumption in handle_from() that the\n>> first '@' denotes the address portion. The remainder of the string\n>> is then assumed to be the name, which is rejected by\n>> get_sane_name(), resulting in address being used for the name as\n>> well.\n>\n> Sorry, but it is unclear from your message what problem you are trying to\n> solve, what your desired outcome is, and why you think that outcome is\n> desired.  The bracketed one shown above does _not_ look like a valid email\n> address, so using\n>\n>    name = \"author@example.com\"\n>    email = \"author@example.com@11B4...\"\n>\n> does not sound like a good thing to do to begin with.\n\nYou left out this part of my commit message:\n\n\"In practice, I've only encountered this issue when rebasing commits\nthat were generated by git svn.\"\n\nWhich is exactly what I was doing. I'm rebasing commits from this repo:\n\nhttp://src.chromium.org/cgi-bin/gitweb.cgi?p=chromium.git\n\nThe rebase operation is munging the address and transforming an author of\n\n user@chromium.org \n\ninto\n\n user@chromium.org \n\nRegardless of whether the address is valid or not, rebase shouldn't munge it.\n\nj.\n"},{"id":"143768","messageId":"AANLkTimmJEGf8UeGxJHhTn1BPiCQEoqhz4KkIiP5qewS@mail.gmail.com","threadId":"24029","inReplyTo":"AANLkTina5G9P-OkGAoia0DeJhPygRaoaiI7tXDRHgjtF@mail.gmail.com","subject":"Re: [PATCH] mailinfo: parse From header more strictly when run from rebase","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2010-06-15T17:01:40Z","receivedAt":"2010-06-15T17:01:40Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, Jun 15, 2010 at 12:41 PM, Jay Soffian wrote:\n> On Tue, Jun 15, 2010 at 12:14 PM, Junio C Hamano wrote:\n>> Sorry, but it is unclear from your message what problem you are trying to\n>> solve, what your desired outcome is, and why you think that outcome is\n>> desired.  The bracketed one shown above does _not_ look like a valid email\n>> address, so using\n>>\n>>    name = \"author@example.com\"\n>>    email = \"author@example.com@11B4...\"\n>>\n>> does not sound like a good thing to do to begin with.\n\nIn fact, that is an invalid email address (I thought it might be valid\nper historical source-routed syntax, but I just checked the RFCs and\nit's not even valid for that). Nonetheless, my goal is that I can\nrebase a commit without the address (even an invalid one) being\nmunged.\n\nj.\n"}]}