{"thread":{"id":"32145","subject":"Failure to extra stable@vger.kernel.org addresses","startedAt":"2012-11-19T09:57:47Z","lastAt":"2012-11-27T10:53:08Z","messageCount":34,"participants":["Felipe Balbi","Krzysztof Mazur","Junio C Hamano","Felipe Contreras","Andreas Ericsson","Andreas Schwab"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"203502","messageId":"20121119095747.GA13552@arwen.pp.htv.fi","threadId":"32145","inReplyTo":null,"subject":"Failure to extra stable@vger.kernel.org addresses","fromName":"Felipe Balbi","fromEmail":"balbi@ti.com","sentAt":"2012-11-19T09:57:47Z","receivedAt":"2012-11-19T09:57:47Z","isPatch":false,"sender":{"key":"balbi@ti.com","avatar":null},"body":"Hi guys,\n\nfor whatever reason my git has started acting up with\nstable@vger.kernel.org addresses. It doesn't manage to extract a valid\nadress from the string:\n\n Cc: <stable@vger.kernel.org> # v3.4 v3.5 v3.6\n\nRemoving the comment at the end of the line makes things work again. I\ndo remember, however, seeing this working since few weeks back I sent a\nmail to stable (in fact the same one I'm using to test), so this could\nbe related to some perl updates, who knows ?!?\n\nAnyway, here's output of git-send-email:\n\n> $ git send-email --to linux-usb@vger.kernel.org ../linux/0001-usb-dwc3-gadget-fix-endpoint-always-busy-bug.diff --dry-run\n> ../linux/0001-usb-dwc3-gadget-fix-endpoint-always-busy-bug.diff\n> (mbox) Adding cc: Felipe Balbi <balbi@ti.com> from line 'From: Felipe Balbi <balbi@ti.com>'\n> (body) Adding cc: <stable@vger.kernel.org> #v3.4 v3.5 v3.6 from line 'Cc: <stable@vger.kernel.org> #v3.4 v3.5 v3.6'\n> (body) Adding cc: Felipe Balbi <balbi@ti.com> from line 'Signed-off-by: Felipe Balbi <balbi@ti.com>'\n> Use of uninitialized value $cc in string eq at /usr/libexec/git-core/git-send-email line 997.\n> Use of uninitialized value $cc in quotemeta at /usr/libexec/git-core/git-send-email line 997.\n> W: unable to extract a valid address from: <stable@vger.kernel.org> #v3.4 v3.5 v3.6\n> W: unable to extract a valid address from: <stable@vger.kernel.org> #v3.4 v3.5 v3.6\n> Dry-OK. Log says:\n> Sendmail: /usr/bin/msmtp -i linux-usb@vger.kernel.org balbi@ti.com\n> From: Felipe Balbi <balbi@ti.com>\n> To: linux-usb@vger.kernel.org\n> Cc: Felipe Balbi <balbi@ti.com>\n> Subject: [PATCH] usb: dwc3: gadget: fix 'endpoint always busy' bug\n> Date: Mon, 19 Nov 2012 11:54:16 +0200\n> Message-Id: <1353318856-14987-1-git-send-email-balbi@ti.com>\n> X-Mailer: git-send-email 1.8.0\n> \n> Result: OK\n\n$ perl --version\n\nThis is perl 5, version 14, subversion 2 (v5.14.2) built for x86_64-linux-gnu-thread-multi\n(with 72 registered patches, see perl -V for more detail)\n\nCopyright 1987-2011, Larry Wall\n\nPerl may be copied only under the terms of either the Artistic License or the\nGNU General Public License, which may be found in the Perl 5 source kit.\n\nComplete documentation for Perl, including FAQ lists, should be found on\nthis system using \"man perl\" or \"perldoc perl\".  If you have access to the\nInternet, point your browser at http://www.perl.org/, the Perl Home Page.\n\nAnd attached you can find the patch file which I'm using\n\n-- \nbalbi\n\n\nFrom 041d81f493d90c940ec41f0ec98bc7c4f2fba431 Mon Sep 17 00:00:00 2001\nFrom: Felipe Balbi <balbi@ti.com>\nDate: Thu, 4 Oct 2012 11:58:00 +0300\nSubject: [PATCH] usb: dwc3: gadget: fix 'endpoint always busy' bug\n\nIf a USB transfer has already been started, meaning\nwe have already issued StartTransfer command to that\nparticular endpoint, DWC3_EP_BUSY flag has also\nalready been set.\n\nWhen we try to cancel this transfer which is already\nin controller's cache, we will not receive XferComplete\nevent and we must clear DWC3_EP_BUSY in order to allow\nsubsequent requests to be properly started.\n\nThe best place to clear that flag is right after issuing\nDWC3_DEPCMD_ENDTRANSFER.\n\nCc: <stable@vger.kernel.org> #v3.4 v3.5 v3.6\nReported-by: Moiz Sonasath <m-sonasath@ti.com>\nSigned-off-by: Felipe Balbi <balbi@ti.com>\n---\n drivers/usb/dwc3/gadget.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c\nindex c9e729a..7b7dedd 100644\n--- a/drivers/usb/dwc3/gadget.c\n+++ b/drivers/usb/dwc3/gadget.c\n@@ -1904,7 +1904,7 @@ static void dwc3_stop_active_transfer(struct dwc3 *dwc, u32 epnum)\n \tret = dwc3_send_gadget_ep_cmd(dwc, dep->number, cmd, &params);\n \tWARN_ON_ONCE(ret);\n \tdep->resource_index = 0;\n-\n+\tdep->flags &= ~DWC3_EP_BUSY;\n \tudelay(100);\n }\n \n-- \n1.8.0\n\n"},{"id":"203504","messageId":"20121119151845.GA29678@shrek.podlesie.net","threadId":"32145","inReplyTo":"20121119095747.GA13552@arwen.pp.htv.fi","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-19T15:18:45Z","receivedAt":"2012-11-19T15:18:45Z","isPatch":false,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Mon, Nov 19, 2012 at 11:57:47AM +0200, Felipe Balbi wrote:\n> Hi guys,\n> \n> for whatever reason my git has started acting up with\n> stable@vger.kernel.org addresses. It doesn't manage to extract a valid\n> adress from the string:\n> \n>  Cc: <stable@vger.kernel.org> # v3.4 v3.5 v3.6\n> \n> Removing the comment at the end of the line makes things work again. I\n> do remember, however, seeing this working since few weeks back I sent a\n> mail to stable (in fact the same one I'm using to test), so this could\n> be related to some perl updates, who knows ?!?\n\nYou probably just installed Email::Valid package.\n\nThe current git-send-email works a little better and just prints an error:\n\nW: unable to extract a valid address from: <stable@vger.kernel.org> #v3.4 v3.5 v3.6\n\n\nThis patch should fix the problem, now after <email> any garbage is\nremoved while extracting address.\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5a7c29d..bb659da 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -828,7 +828,7 @@ sub extract_valid_address {\n \t# check for a local address:\n \treturn $address if ($address =~ /^($local_part_regexp)$/);\n \n-\t$address =~ s/^\\s*<(.*)>\\s*$/$1/;\n+\t$address =~ s/^\\s*<(.*)>.*$/$1/;\n \tif ($have_email_valid) {\n \t\treturn scalar Email::Valid->address($address);\n \t} else {\n\nKrzysiek\n"},{"id":"203505","messageId":"20121119153723.GA18697@arwen.pp.htv.fi","threadId":"32145","inReplyTo":"20121119151845.GA29678@shrek.podlesie.net","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Felipe Balbi","fromEmail":"balbi@ti.com","sentAt":"2012-11-19T15:37:23Z","receivedAt":"2012-11-19T15:37:23Z","isPatch":false,"sender":{"key":"balbi@ti.com","avatar":null},"body":"Hi,\n\nOn Mon, Nov 19, 2012 at 04:18:45PM +0100, Krzysztof Mazur wrote:\n> On Mon, Nov 19, 2012 at 11:57:47AM +0200, Felipe Balbi wrote:\n> > Hi guys,\n> > \n> > for whatever reason my git has started acting up with\n> > stable@vger.kernel.org addresses. It doesn't manage to extract a valid\n> > adress from the string:\n> > \n> >  Cc: <stable@vger.kernel.org> # v3.4 v3.5 v3.6\n> > \n> > Removing the comment at the end of the line makes things work again. I\n> > do remember, however, seeing this working since few weeks back I sent a\n> > mail to stable (in fact the same one I'm using to test), so this could\n> > be related to some perl updates, who knows ?!?\n> \n> You probably just installed Email::Valid package.\n> \n> The current git-send-email works a little better and just prints an error:\n> \n> W: unable to extract a valid address from: <stable@vger.kernel.org> #v3.4 v3.5 v3.6\n> \n> \n> This patch should fix the problem, now after <email> any garbage is\n> removed while extracting address.\n\nworked like a charm. When you send as a proper patch, you can add my:\n\nTested-by: Felipe Balbi <balbi@ti.com>\n\nthanks\n\n-- \nbalbi\n"},{"id":"203509","messageId":"7vk3thxuj2.fsf@alter.siamese.dyndns.org","threadId":"32145","inReplyTo":"20121119151845.GA29678@shrek.podlesie.net","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-19T19:27:45Z","receivedAt":"2012-11-19T19:27:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Krzysztof Mazur <krzysiek@podlesie.net> writes:\n\n> On Mon, Nov 19, 2012 at 11:57:47AM +0200, Felipe Balbi wrote:\n>> Hi guys,\n>> \n>> for whatever reason my git has started acting up with\n>> stable@vger.kernel.org addresses. It doesn't manage to extract a valid\n>> adress from the string:\n>> \n>>  Cc: <stable@vger.kernel.org> # v3.4 v3.5 v3.6\n>> \n>> Removing the comment at the end of the line makes things work again. I\n>> do remember, however, seeing this working since few weeks back I sent a\n>> mail to stable (in fact the same one I'm using to test), so this could\n>> be related to some perl updates, who knows ?!?\n>\n> You probably just installed Email::Valid package.\n>\n> The current git-send-email works a little better and just prints an error:\n>\n> W: unable to extract a valid address from: <stable@vger.kernel.org> #v3.4 v3.5 v3.6\n>\n>\n> This patch should fix the problem, now after <email> any garbage is\n> removed while extracting address.\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 5a7c29d..bb659da 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -828,7 +828,7 @@ sub extract_valid_address {\n>  \t# check for a local address:\n>  \treturn $address if ($address =~ /^($local_part_regexp)$/);\n>  \n> -\t$address =~ s/^\\s*<(.*)>\\s*$/$1/;\n> +\t$address =~ s/^\\s*<(.*)>.*$/$1/;\n>  \tif ($have_email_valid) {\n>  \t\treturn scalar Email::Valid->address($address);\n>  \t} else {\n\nGiven that the problematic line\n\n\tStable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y\n\nis not even a valid e-mail address, doesn't this new logic belong to\nsanitize_address() conceptually?\n"},{"id":"203519","messageId":"CAMP44s3SeGgyAq_5G=t3iR8LB0Yo7drh7Hc4poo6eEbsS-i_yA@mail.gmail.com","threadId":"32145","inReplyTo":"7vk3thxuj2.fsf@alter.siamese.dyndns.org","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-19T21:59:28Z","receivedAt":"2012-11-19T21:59:28Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Nov 19, 2012 at 8:27 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Krzysztof Mazur <krzysiek@podlesie.net> writes:\n>\n>> On Mon, Nov 19, 2012 at 11:57:47AM +0200, Felipe Balbi wrote:\n>>> Hi guys,\n>>>\n>>> for whatever reason my git has started acting up with\n>>> stable@vger.kernel.org addresses. It doesn't manage to extract a valid\n>>> adress from the string:\n>>>\n>>>  Cc: <stable@vger.kernel.org> # v3.4 v3.5 v3.6\n>>>\n>>> Removing the comment at the end of the line makes things work again. I\n>>> do remember, however, seeing this working since few weeks back I sent a\n>>> mail to stable (in fact the same one I'm using to test), so this could\n>>> be related to some perl updates, who knows ?!?\n>>\n>> You probably just installed Email::Valid package.\n>>\n>> The current git-send-email works a little better and just prints an error:\n>>\n>> W: unable to extract a valid address from: <stable@vger.kernel.org> #v3.4 v3.5 v3.6\n>>\n>>\n>> This patch should fix the problem, now after <email> any garbage is\n>> removed while extracting address.\n>>\n>> diff --git a/git-send-email.perl b/git-send-email.perl\n>> index 5a7c29d..bb659da 100755\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -828,7 +828,7 @@ sub extract_valid_address {\n>>       # check for a local address:\n>>       return $address if ($address =~ /^($local_part_regexp)$/);\n>>\n>> -     $address =~ s/^\\s*<(.*)>\\s*$/$1/;\n>> +     $address =~ s/^\\s*<(.*)>.*$/$1/;\n>>       if ($have_email_valid) {\n>>               return scalar Email::Valid->address($address);\n>>       } else {\n>\n> Given that the problematic line\n>\n>         Stable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y\n>\n> is not even a valid e-mail address, doesn't this new logic belong to\n> sanitize_address() conceptually?\n\nThat would be great, it would also help the cc-cmd stuff. The\nget_maintainer.pl patch from the Linux kernel outputs something like:\n\nDavid Airlie <airlied@linux.ie> (maintainer:DRM DRIVERS)\nBen Skeggs <bskeggs@redhat.com>\n(commit_signer:17/19=89%,commit_signer:43/46=93%)\nMaxim Levitsky <maximlevitsky@gmail.com> (commit_signer:3/19=16%)\nGreg Kroah-Hartman <gregkh@linuxfoundation.org> (commit_signer:2/19=11%)\nDave Airlie <airlied@redhat.com> (commit_signer:2/19=11%,commit_signer:3/46=7%)\nAlex Deucher <alexander.deucher@amd.com> (commit_signer:1/19=5%)\ndri-devel@lists.freedesktop.org (open list:DRM DRIVERS)\nlinux-kernel@vger.kernel.org (open list)\n\n-- \nFelipe Contreras\n"},{"id":"203522","messageId":"20121119225838.GA23412@shrek.podlesie.net","threadId":"32145","inReplyTo":"7vk3thxuj2.fsf@alter.siamese.dyndns.org","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-19T22:58:38Z","receivedAt":"2012-11-19T22:58:38Z","isPatch":false,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:\n> Given that the problematic line\n> \n> \tStable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y\n> \n> is not even a valid e-mail address, doesn't this new logic belong to\n> sanitize_address() conceptually?\n\nYes, it's much better to do it in the sanitize_address().\n\nFelipe, may you check it?\n\nKrzysiek\n-- >8 --\nSubject: [PATCH] git-send-email: remove garbage after email address\n\nIn some cases it's very useful to add some additional information\nafter email in Cc-list, for instance:\n\n\"Cc: Stable kernel <stable@vger.kernel.org> #v3.4 v3.5 v3.6\"\n\nCurrently the git refuses to add such invalid email to Cc-list,\nwhen the Email::Valid perl module is available or just uses whole line\nas the email address.\n\nNow in sanitize_address() everything after the email address is\nremoved, so the resulting line is correct email address and Email::Valid\nvalidates it correctly.\n\nSigned-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\n---\n git-send-email.perl | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5a7c29d..9840d0a 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -924,6 +924,10 @@ sub quote_subject {\n # use the simplest quoting being able to handle the recipient\n sub sanitize_address {\n \tmy ($recipient) = @_;\n+\n+\t# remove garbage after email address\n+\t$recipient =~ s/(.*>).*$/$1/;\n+\n \tmy ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\\s*(<.*)/);\n \n \tif (not $recipient_name) {\n-- \n1.8.0.283.gc57d856\n"},{"id":"203524","messageId":"CAMP44s0f0zYa1FVf9RhNuwYJbkQ7zPwgJ6=ty3c5knjo5a2TNw@mail.gmail.com","threadId":"32145","inReplyTo":"20121119225838.GA23412@shrek.podlesie.net","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-19T23:12:02Z","receivedAt":"2012-11-19T23:12:02Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -924,6 +924,10 @@ sub quote_subject {\n>  # use the simplest quoting being able to handle the recipient\n>  sub sanitize_address {\n>         my ($recipient) = @_;\n> +\n> +       # remove garbage after email address\n> +       $recipient =~ s/(.*>).*$/$1/;\n> +\n\nLooks fine, but I would do s/(.*?>)(.*)$/$1/, so that 'test\n<foo@bar.com> <#comment>' gets the second comment removed.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203526","messageId":"7vlidxuowf.fsf@alter.siamese.dyndns.org","threadId":"32145","inReplyTo":"CAMP44s0f0zYa1FVf9RhNuwYJbkQ7zPwgJ6=ty3c5knjo5a2TNw@mail.gmail.com","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-19T23:57:36Z","receivedAt":"2012-11-19T23:57:36Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n>\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -924,6 +924,10 @@ sub quote_subject {\n>>  # use the simplest quoting being able to handle the recipient\n>>  sub sanitize_address {\n>>         my ($recipient) = @_;\n>> +\n>> +       # remove garbage after email address\n>> +       $recipient =~ s/(.*>).*$/$1/;\n>> +\n>\n> Looks fine, but I would do s/(.*?>)(.*)$/$1/, so that 'test\n> <foo@bar.com> <#comment>' gets the second comment removed.\n\nYeah, but do you need to capture the second group?  IOW, like\n\"s/(.*?>).*$/$1/\" perhaps?\n"},{"id":"203527","messageId":"7vhaoluos6.fsf@alter.siamese.dyndns.org","threadId":"32145","inReplyTo":"20121119225838.GA23412@shrek.podlesie.net","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-20T00:00:09Z","receivedAt":"2012-11-20T00:00:09Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Krzysztof Mazur <krzysiek@podlesie.net> writes:\n\n> On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:\n>> Given that the problematic line\n>> \n>> \tStable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y\n>> \n>> is not even a valid e-mail address, doesn't this new logic belong to\n>> sanitize_address() conceptually?\n>\n> Yes, it's much better to do it in the sanitize_address().\n\nNote that I did not check that all the addresses that are handled by\nextract-valid-address came through sanitize-address function, so\nunlike your original patch, this change alone may still pass some\ngarbage to Email::Valid->address().  I tend to think that is a\nprogress; we should make sure all the addresses are sanitized before\nusing them for sending messages out.\n"},{"id":"203541","messageId":"20121120071516.GA7206@shrek.podlesie.net","threadId":"32145","inReplyTo":"7vhaoluos6.fsf@alter.siamese.dyndns.org","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-20T07:15:16Z","receivedAt":"2012-11-20T07:15:16Z","isPatch":false,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Mon, Nov 19, 2012 at 04:00:09PM -0800, Junio C Hamano wrote:\n> Krzysztof Mazur <krzysiek@podlesie.net> writes:\n> \n> > On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:\n> >> Given that the problematic line\n> >> \n> >> \tStable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y\n> >> \n> >> is not even a valid e-mail address, doesn't this new logic belong to\n> >> sanitize_address() conceptually?\n> >\n> > Yes, it's much better to do it in the sanitize_address().\n> \n> Note that I did not check that all the addresses that are handled by\n> extract-valid-address came through sanitize-address function, so\n\nBefore sending that patch, I checked that and tested with and without\nEmail::Valid.\n\n> unlike your original patch, this change alone may still pass some\n> garbage to Email::Valid->address().  I tend to think that is a\n> progress; we should make sure all the addresses are sanitized before\n> using them for sending messages out.\n\nI will try to check that.\n\nKrzysiek\n"},{"id":"203542","messageId":"20121120073100.GB7206@shrek.podlesie.net","threadId":"32145","inReplyTo":"7vlidxuowf.fsf@alter.siamese.dyndns.org","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-20T07:31:00Z","receivedAt":"2012-11-20T07:31:00Z","isPatch":false,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Mon, Nov 19, 2012 at 03:57:36PM -0800, Junio C Hamano wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n> \n> > On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n> >\n> >> --- a/git-send-email.perl\n> >> +++ b/git-send-email.perl\n> >> @@ -924,6 +924,10 @@ sub quote_subject {\n> >>  # use the simplest quoting being able to handle the recipient\n> >>  sub sanitize_address {\n> >>         my ($recipient) = @_;\n> >> +\n> >> +       # remove garbage after email address\n> >> +       $recipient =~ s/(.*>).*$/$1/;\n> >> +\n> >\n> > Looks fine, but I would do s/(.*?>)(.*)$/$1/, so that 'test\n> > <foo@bar.com> <#comment>' gets the second comment removed.\n> \n> Yeah, but do you need to capture the second group?  IOW, like\n> \"s/(.*?>).*$/$1/\" perhaps?\n\nI also thought about removing everything after first \">\", but I will\nnot work for addresses like:\n\nCc: \"foo >\" <stable@vger.kernel.org> #v3.4 v3.5 v3.6\n\nWhat about:\n\n\t$recipient =~ s/(.*<[^@]*@[^]]*>).*$/$1/;\n\nor even\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 9840d0a..b988c57 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -925,8 +925,11 @@ sub quote_subject {\n sub sanitize_address {\n \tmy ($recipient) = @_;\n \n+\tmy $local_part_regexp = qr/[^<>\"\\s@]+/;\n+\tmy $domain_regexp = qr/[^.<>\"\\s@]+(?:\\.[^.<>\"\\s@]+)+/;\n+\n \t# remove garbage after email address\n-\t$recipient =~ s/(.*>).*$/$1/;\n+\t$recipient =~ s/(.*<$local_part_regexp\\@$domain_regexp>).*$/$1/;\n \n \tmy ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\\s*(<.*)/);\n\nwhich uses regex used by 99% accurate version of extract_valid_address().\n\nKrzysiek\n"},{"id":"203545","messageId":"20121120075416.GA27690@arwen.pp.htv.fi","threadId":"32145","inReplyTo":"20121119225838.GA23412@shrek.podlesie.net","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Felipe Balbi","fromEmail":"balbi@ti.com","sentAt":"2012-11-20T07:54:16Z","receivedAt":"2012-11-20T07:54:16Z","isPatch":false,"sender":{"key":"balbi@ti.com","avatar":null},"body":"On Mon, Nov 19, 2012 at 11:58:38PM +0100, Krzysztof Mazur wrote:\n> On Mon, Nov 19, 2012 at 11:27:45AM -0800, Junio C Hamano wrote:\n> > Given that the problematic line\n> > \n> > \tStable Kernel Maintainance Track <stable@vger.kernel.org> # vX.Y\n> > \n> > is not even a valid e-mail address, doesn't this new logic belong to\n> > sanitize_address() conceptually?\n> \n> Yes, it's much better to do it in the sanitize_address().\n> \n> Felipe, may you check it?\n> \n> Krzysiek\n> -- >8 --\n> Subject: [PATCH] git-send-email: remove garbage after email address\n> \n> In some cases it's very useful to add some additional information\n> after email in Cc-list, for instance:\n> \n> \"Cc: Stable kernel <stable@vger.kernel.org> #v3.4 v3.5 v3.6\"\n> \n> Currently the git refuses to add such invalid email to Cc-list,\n> when the Email::Valid perl module is available or just uses whole line\n> as the email address.\n> \n> Now in sanitize_address() everything after the email address is\n> removed, so the resulting line is correct email address and Email::Valid\n> validates it correctly.\n> \n> Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\n\nTested-by: Felipe Balbi <balbi@ti.com>\n\n> ---\n>  git-send-email.perl | 4 ++++\n>  1 file changed, 4 insertions(+)\n> \n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 5a7c29d..9840d0a 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -924,6 +924,10 @@ sub quote_subject {\n>  # use the simplest quoting being able to handle the recipient\n>  sub sanitize_address {\n>  \tmy ($recipient) = @_;\n> +\n> +\t# remove garbage after email address\n> +\t$recipient =~ s/(.*>).*$/$1/;\n> +\n>  \tmy ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\\s*(<.*)/);\n>  \n>  \tif (not $recipient_name) {\n> -- \n> 1.8.0.283.gc57d856\n> \n\n-- \nbalbi\n"},{"id":"203544","messageId":"20121120075628.GA7159@shrek.podlesie.net","threadId":"32145","inReplyTo":"20121120073100.GB7206@shrek.podlesie.net","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-20T07:56:28Z","receivedAt":"2012-11-20T07:56:28Z","isPatch":false,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Tue, Nov 20, 2012 at 08:31:00AM +0100, Krzysztof Mazur wrote:\n> On Mon, Nov 19, 2012 at 03:57:36PM -0800, Junio C Hamano wrote:\n> > Felipe Contreras <felipe.contreras@gmail.com> writes:\n> > \n> > > On Mon, Nov 19, 2012 at 11:58 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n> > >\n> > >> --- a/git-send-email.perl\n> > >> +++ b/git-send-email.perl\n> > >> @@ -924,6 +924,10 @@ sub quote_subject {\n> > >>  # use the simplest quoting being able to handle the recipient\n> > >>  sub sanitize_address {\n> > >>         my ($recipient) = @_;\n> > >> +\n> > >> +       # remove garbage after email address\n> > >> +       $recipient =~ s/(.*>).*$/$1/;\n> > >> +\n> > >\n> > > Looks fine, but I would do s/(.*?>)(.*)$/$1/, so that 'test\n> > > <foo@bar.com> <#comment>' gets the second comment removed.\n> > \n> > Yeah, but do you need to capture the second group?  IOW, like\n> > \"s/(.*?>).*$/$1/\" perhaps?\n> \n> I also thought about removing everything after first \">\", but I will\n> not work for addresses like:\n> \n> Cc: \"foo >\" <stable@vger.kernel.org> #v3.4 v3.5 v3.6\n> \n> What about:\n> \n> \t$recipient =~ s/(.*<[^@]*@[^]]*>).*$/$1/;\n> \n> or even\n> \n> which uses regex used by 99% accurate version of extract_valid_address().\n> \n\nOf course, as you suggested earier, only the first email address should\nbe used, so in both cases the first \".*\" should be changed to \".*?\".\nThe second version becomes:\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 9840d0a..dbe520c 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -925,8 +925,11 @@ sub quote_subject {\n sub sanitize_address {\n \tmy ($recipient) = @_;\n \n+\tmy $local_part_regexp = qr/[^<>\"\\s@]+/;\n+\tmy $domain_regexp = qr/[^.<>\"\\s@]+(?:\\.[^.<>\"\\s@]+)+/;\n+\n \t# remove garbage after email address\n-\t$recipient =~ s/(.*>).*$/$1/;\n+\t$recipient =~ s/^(.*?<$local_part_regexp\\@$domain_regexp>).*/$1/;\n \n \tmy ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\\s*(<.*)/);\n \nKrzysiek\n"},{"id":"203548","messageId":"CAMP44s38gTB_3Ao1rFZgMo2EAuiNb+h88-qRFcQPRMJNxo3CAQ@mail.gmail.com","threadId":"32145","inReplyTo":"20121120075628.GA7159@shrek.podlesie.net","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-20T10:28:39Z","receivedAt":"2012-11-20T10:28:39Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Nov 20, 2012 at 8:56 AM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -925,8 +925,11 @@ sub quote_subject {\n>  sub sanitize_address {\n>         my ($recipient) = @_;\n>\n> +       my $local_part_regexp = qr/[^<>\"\\s@]+/;\n> +       my $domain_regexp = qr/[^.<>\"\\s@]+(?:\\.[^.<>\"\\s@]+)+/;\n> +\n>         # remove garbage after email address\n> -       $recipient =~ s/(.*>).*$/$1/;\n> +       $recipient =~ s/^(.*?<$local_part_regexp\\@$domain_regexp>).*/$1/;\n\nI don't think all that extra complexity is warranted, to me\ns/(.*?>)(.*)$/$1/ is just fine.\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203549","messageId":"50AB6494.3070109@op5.se","threadId":"32145","inReplyTo":"CAMP44s38gTB_3Ao1rFZgMo2EAuiNb+h88-qRFcQPRMJNxo3CAQ@mail.gmail.com","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2012-11-20T11:08:04Z","receivedAt":"2012-11-20T11:08:04Z","isPatch":false,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"On 11/20/2012 11:28 AM, Felipe Contreras wrote:\n> On Tue, Nov 20, 2012 at 8:56 AM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n> \n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -925,8 +925,11 @@ sub quote_subject {\n>>   sub sanitize_address {\n>>          my ($recipient) = @_;\n>>\n>> +       my $local_part_regexp = qr/[^<>\"\\s@]+/;\n>> +       my $domain_regexp = qr/[^.<>\"\\s@]+(?:\\.[^.<>\"\\s@]+)+/;\n>> +\n>>          # remove garbage after email address\n>> -       $recipient =~ s/(.*>).*$/$1/;\n>> +       $recipient =~ s/^(.*?<$local_part_regexp\\@$domain_regexp>).*/$1/;\n> \n> I don't think all that extra complexity is warranted, to me\n> s/(.*?>)(.*)$/$1/ is just fine.\n> \n\nIt's intentionally left without the at-sign so one can send mail to a\nlocal account as well as remote ones. Very nifty when debugging, and\nwhen one wants to preview outgoing emails.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n\nConsidering the successes of the wars on alcohol, poverty, drugs and\nterror, I think we should give some serious thought to declaring war\non peace.\n"},{"id":"203554","messageId":"20121120115942.GA6132@shrek.podlesie.net","threadId":"32145","inReplyTo":"CAMP44s38gTB_3Ao1rFZgMo2EAuiNb+h88-qRFcQPRMJNxo3CAQ@mail.gmail.com","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-20T11:59:42Z","receivedAt":"2012-11-20T11:59:42Z","isPatch":false,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Tue, Nov 20, 2012 at 11:28:39AM +0100, Felipe Contreras wrote:\n> On Tue, Nov 20, 2012 at 8:56 AM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n> \n> > --- a/git-send-email.perl\n> > +++ b/git-send-email.perl\n> > @@ -925,8 +925,11 @@ sub quote_subject {\n> >  sub sanitize_address {\n> >         my ($recipient) = @_;\n> >\n> > +       my $local_part_regexp = qr/[^<>\"\\s@]+/;\n> > +       my $domain_regexp = qr/[^.<>\"\\s@]+(?:\\.[^.<>\"\\s@]+)+/;\n> > +\n> >         # remove garbage after email address\n> > -       $recipient =~ s/(.*>).*$/$1/;\n> > +       $recipient =~ s/^(.*?<$local_part_regexp\\@$domain_regexp>).*/$1/;\n> \n> I don't think all that extra complexity is warranted, to me\n> s/(.*?>)(.*)$/$1/ is just fine.\n> \n\nYeah, it's a little bit too complex, but \"s/(.*?>)(.*)$/$1/\"\ncauses small regression - '>' character is no longer allowed\nin \"phrase\" before \"<email address>\". Maybe the initial version,\nthat removes everything after last '>' is better? In this case '>'\nis not allowed in garbage after email.\n\nKrzysiek\n"},{"id":"203567","messageId":"m2lidw11yb.fsf@igel.home","threadId":"32145","inReplyTo":"20121120115942.GA6132@shrek.podlesie.net","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Andreas Schwab","fromEmail":"schwab@linux-m68k.org","sentAt":"2012-11-20T19:58:20Z","receivedAt":"2012-11-20T19:58:20Z","isPatch":false,"sender":{"key":"schwab@linux-m68k.org","avatar":"https://avatars.githubusercontent.com/u/2175493?v=4"},"body":"Krzysztof Mazur <krzysiek@podlesie.net> writes:\n\n> On Tue, Nov 20, 2012 at 11:28:39AM +0100, Felipe Contreras wrote:\n>> On Tue, Nov 20, 2012 at 8:56 AM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n>> \n>> > --- a/git-send-email.perl\n>> > +++ b/git-send-email.perl\n>> > @@ -925,8 +925,11 @@ sub quote_subject {\n>> >  sub sanitize_address {\n>> >         my ($recipient) = @_;\n>> >\n>> > +       my $local_part_regexp = qr/[^<>\"\\s@]+/;\n>> > +       my $domain_regexp = qr/[^.<>\"\\s@]+(?:\\.[^.<>\"\\s@]+)+/;\n>> > +\n>> >         # remove garbage after email address\n>> > -       $recipient =~ s/(.*>).*$/$1/;\n>> > +       $recipient =~ s/^(.*?<$local_part_regexp\\@$domain_regexp>).*/$1/;\n>> \n>> I don't think all that extra complexity is warranted, to me\n>> s/(.*?>)(.*)$/$1/ is just fine.\n>> \n>\n> Yeah, it's a little bit too complex, but \"s/(.*?>)(.*)$/$1/\"\n\nHow about \"s/(.*?<[^>]*>).*$/$1/\"?  That will still fail on \"<foo@bar>\"\n<foo@bar>, but you'll need a full rfc822 parser to handle the general\ncase anyway.\n\nAndreas.\n\n-- \nAndreas Schwab, schwab@linux-m68k.org\nGPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5\n\"And now for something completely different.\"\n"},{"id":"203573","messageId":"20121120212126.GA12656@shrek.podlesie.net","threadId":"32145","inReplyTo":"m2lidw11yb.fsf@igel.home","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-20T21:21:26Z","receivedAt":"2012-11-20T21:21:26Z","isPatch":false,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Tue, Nov 20, 2012 at 08:58:20PM +0100, Andreas Schwab wrote:\n> How about \"s/(.*?<[^>]*>).*$/$1/\"?  That will still fail on \"<foo@bar>\"\n> <foo@bar>, but you'll need a full rfc822 parser to handle the general\n> case anyway.\n\nThat will fail also on \"<something>\" <foo@bar>.\n\n\nI think it's good compromise between complexity and correctness.\n\nFelipe, may you check, it again? This time the change is trivial.\n\nAndreas, may I add you in Thanks-to?\n\nThanks,\n\nKrzysiek\n\n-- >8 --\nSubject: [PATCH] git-send-email: remove garbage after email address\n\nIn some cases it's very useful to add some additional information\nafter email in Cc-list, for instance:\n\n\"Cc: Stable kernel <stable@vger.kernel.org> #v3.4 v3.5 v3.6\"\n\nCurrently the git refuses to add such invalid email to Cc-list,\nwhen the Email::Valid perl module is available or just uses whole line\nas the email address.\n\nNow in sanitize_address() everything after the email address is\nremoved, so the resulting line is correct email address and Email::Valid\nvalidates it correctly.\n\nTo avoid unnecessary complexity this code assumes that in phrase before\nemail address '<something>' never exists.\n\nSigned-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\n---\n git-send-email.perl | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5a7c29d..157eabc 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -924,6 +924,10 @@ sub quote_subject {\n # use the simplest quoting being able to handle the recipient\n sub sanitize_address {\n \tmy ($recipient) = @_;\n+\n+\t# remove garbage after email address\n+\t$recipient =~ s/(.*?<[^>]*>).*$/$1/;\n+\n \tmy ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\\s*(<.*)/);\n \n \tif (not $recipient_name) {\n-- \n1.8.0.283.gc57d856\n"},{"id":"203574","messageId":"CAMP44s3+vnKfhhh=qqU2vuKvWwhii4CQ7=YAuhFiceX1EDaVKQ@mail.gmail.com","threadId":"32145","inReplyTo":"20121120212126.GA12656@shrek.podlesie.net","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2012-11-20T22:06:43Z","receivedAt":"2012-11-20T22:06:43Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"On Tue, Nov 20, 2012 at 10:21 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -924,6 +924,10 @@ sub quote_subject {\n>  # use the simplest quoting being able to handle the recipient\n>  sub sanitize_address {\n>         my ($recipient) = @_;\n> +\n> +       # remove garbage after email address\n> +       $recipient =~ s/(.*?<[^>]*>).*$/$1/;\n\nThat won't work for 'foo@bar.com # test'. I think we should abandon\nhopes of properly parsing an email address and just do:\n\n$recipient =~ s/(.*?) #.*$/$1/;\n\nCheers.\n\n-- \nFelipe Contreras\n"},{"id":"203578","messageId":"7vhaojrjpx.fsf@alter.siamese.dyndns.org","threadId":"32145","inReplyTo":"CAMP44s3+vnKfhhh=qqU2vuKvWwhii4CQ7=YAuhFiceX1EDaVKQ@mail.gmail.com","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-20T22:30:02Z","receivedAt":"2012-11-20T22:30:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Felipe Contreras <felipe.contreras@gmail.com> writes:\n\n> On Tue, Nov 20, 2012 at 10:21 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n>\n>> --- a/git-send-email.perl\n>> +++ b/git-send-email.perl\n>> @@ -924,6 +924,10 @@ sub quote_subject {\n>>  # use the simplest quoting being able to handle the recipient\n>>  sub sanitize_address {\n>>         my ($recipient) = @_;\n>> +\n>> +       # remove garbage after email address\n>> +       $recipient =~ s/(.*?<[^>]*>).*$/$1/;\n>\n> That won't work for 'foo@bar.com # test'. I think we should abandon\n> hopes of properly parsing an email address and just do:\n>\n> $recipient =~ s/(.*?) #.*$/$1/;\n\nWe should probably fix the tools that generate these bogus\nnon-addresses first.  What's wrong with\n\n\tCc: stable kernel (v3.5 v3.6 v3.7) <stable@vger.kernel.org>\n\nwhich should be OK?\n\nAlso I suspect that this should be also deemed valid:\n\n\tCc: stable@vger.kernel.org (Stable kernel - v3.5 v3.6 v3.7)\n"},{"id":"203580","messageId":"20121120230955.GA9686@shrek.podlesie.net","threadId":"32145","inReplyTo":"7vhaojrjpx.fsf@alter.siamese.dyndns.org","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-20T23:09:55Z","receivedAt":"2012-11-20T23:09:55Z","isPatch":false,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Tue, Nov 20, 2012 at 02:30:02PM -0800, Junio C Hamano wrote:\n> Felipe Contreras <felipe.contreras@gmail.com> writes:\n> \n> > On Tue, Nov 20, 2012 at 10:21 PM, Krzysztof Mazur <krzysiek@podlesie.net> wrote:\n> >\n> >> --- a/git-send-email.perl\n> >> +++ b/git-send-email.perl\n> >> @@ -924,6 +924,10 @@ sub quote_subject {\n> >>  # use the simplest quoting being able to handle the recipient\n> >>  sub sanitize_address {\n> >>         my ($recipient) = @_;\n> >> +\n> >> +       # remove garbage after email address\n> >> +       $recipient =~ s/(.*?<[^>]*>).*$/$1/;\n> >\n> > That won't work for 'foo@bar.com # test'. I think we should abandon\n> > hopes of properly parsing an email address and just do:\n> >\n> > $recipient =~ s/(.*?) #.*$/$1/;\n> \n> We should probably fix the tools that generate these bogus\n> non-addresses first.  What's wrong with\n> \n> \tCc: stable kernel (v3.5 v3.6 v3.7) <stable@vger.kernel.org>\n> \n> which should be OK?\n> \n> Also I suspect that this should be also deemed valid:\n> \n> \tCc: stable@vger.kernel.org (Stable kernel - v3.5 v3.6 v3.7)\n\nSo maybe we should just use the original regex:\n\n$recipient =~ s/(.*>).*$/$1/\n\nwhich does not add regression for valid addresses, and just fails\nin some rare cases when '>' is used in garbage. It was sufficient\nfor original issue reported by, and tested by Felipe.\n\nThe problem with '>' would be fixed in separate patch. The same\nproblem exits for invalid address generated by --cc-cmd\n(see [PATCH] git-send-email: don't return undefined value in\nextract_valid_address()). We would report an error in both cases,\nas suggested by Junio.\n\nKrzysiek\n"},{"id":"203581","messageId":"7v8v9vrgc9.fsf@alter.siamese.dyndns.org","threadId":"32145","inReplyTo":"20121120230955.GA9686@shrek.podlesie.net","subject":"Re: Failure to extra stable@vger.kernel.org addresses","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-20T23:43:02Z","receivedAt":"2012-11-20T23:43:02Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Krzysztof Mazur <krzysiek@podlesie.net> writes:\n\n> On Tue, Nov 20, 2012 at 02:30:02PM -0800, Junio C Hamano wrote:\n>\n>> We should probably fix the tools that generate these bogus\n>> non-addresses first.  What's wrong with\n>> \n>> \tCc: stable kernel (v3.5 v3.6 v3.7) <stable@vger.kernel.org>\n>> \n>> which should be OK?\n>> \n>> Also I suspect that this should be also deemed valid:\n>> \n>> \tCc: stable@vger.kernel.org (Stable kernel - v3.5 v3.6 v3.7)\n>\n> So maybe we should just use the original regex:\n>\n> $recipient =~ s/(.*>).*$/$1/\n>\n> which does not add regression for valid addresses, and just fails\n> in some rare cases when '>' is used in garbage. It was sufficient\n> for original issue reported by, and tested by Felipe.\n>\n> The problem with '>' would be fixed in separate patch. The same\n> problem exits for invalid address generated by --cc-cmd\n> (see [PATCH] git-send-email: don't return undefined value in\n> extract_valid_address()). We would report an error in both cases,\n> as suggested by Junio.\n\nOK, sounds like a plan.\n"},{"id":"203631","messageId":"1353607932-10436-1-git-send-email-krzysiek@podlesie.net","threadId":"32145","inReplyTo":"7v8v9vrgc9.fsf@alter.siamese.dyndns.org","subject":"[PATCH 1/5] git-send-email: remove garbage after email address","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-22T18:12:08Z","receivedAt":"2012-11-22T18:12:08Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"In some cases it's very useful to add some additional information\nafter email in Cc-list, for instance:\n\n\"Cc: Stable kernel <stable@vger.kernel.org> #v3.4 v3.5 v3.6\"\n\nCurrently the git refuses to add such invalid email to Cc-list,\nwhen the Email::Valid perl module is available or just uses whole line\nas the email address.\n\nNow in sanitize_address() everything after the email address is\nremoved, so the resulting line is correct email address and Email::Valid\nvalidates it correctly.\n\nSigned-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\nTested-by: Felipe Balbi <balbi@ti.com>\n---\n git-send-email.perl | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5a7c29d..9840d0a 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -924,6 +924,10 @@ sub quote_subject {\n # use the simplest quoting being able to handle the recipient\n sub sanitize_address {\n \tmy ($recipient) = @_;\n+\n+\t# remove garbage after email address\n+\t$recipient =~ s/(.*>).*$/$1/;\n+\n \tmy ($recipient_name, $recipient_addr) = ($recipient =~ /^(.*?)\\s*(<.*)/);\n \n \tif (not $recipient_name) {\n-- \n1.8.0.393.gcc9701d\n"},{"id":"203674","messageId":"1353607932-10436-2-git-send-email-krzysiek@podlesie.net","threadId":"32145","inReplyTo":"1353607932-10436-1-git-send-email-krzysiek@podlesie.net","subject":"[PATCH 2/5] git-send-email: fix fallback code in extract_valid_address()","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-22T18:12:09Z","receivedAt":"2012-11-22T18:12:09Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"In the fallback check, used when Email::Valid is not available, the\nextract_valid_address() uses $1 without checking for success of matching\nregex. The $1 variable may still hold the result of previous match,\nwhich is the address when email address was in '<>' or be undefined\notherwise.\n\nNow if match fails undefined value is always returned to indicate error.\nThe same value is used by Email::Valid->address() in that case.\n\nPreviously 'foo@bar' address was rejected by Email::Valid and fallback,\nbut '<foo@bar>' was rejected by Email::Valid, but accepted by fallback.\n\nSigned-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\n---\n git-send-email.perl | 10 +++++-----\n 1 file changed, 5 insertions(+), 5 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 9840d0a..356f99d 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -831,12 +831,12 @@ sub extract_valid_address {\n \t$address =~ s/^\\s*<(.*)>\\s*$/$1/;\n \tif ($have_email_valid) {\n \t\treturn scalar Email::Valid->address($address);\n-\t} else {\n-\t\t# less robust/correct than the monster regexp in Email::Valid,\n-\t\t# but still does a 99% job, and one less dependency\n-\t\t$address =~ /($local_part_regexp\\@$domain_regexp)/;\n-\t\treturn $1;\n \t}\n+\n+\t# less robust/correct than the monster regexp in Email::Valid,\n+\t# but still does a 99% job, and one less dependency\n+\treturn $1 if $address =~ /($local_part_regexp\\@$domain_regexp)/;\n+\treturn undef;\n }\n \n # Usually don't need to change anything below here.\n-- \n1.8.0.393.gcc9701d\n"},{"id":"203671","messageId":"1353607932-10436-3-git-send-email-krzysiek@podlesie.net","threadId":"32145","inReplyTo":"1353607932-10436-1-git-send-email-krzysiek@podlesie.net","subject":"[PATCH 3/5] git-send-email: remove invalid addresses earlier","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-22T18:12:10Z","receivedAt":"2012-11-22T18:12:10Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"Some addresses are passed twice to unique_email_list() and invalid addresses\nmay be reported twice per send_message. Now we warn about them earlier\nand we also remove invalid addresses.\n\nThis also removes using of undefined values for string comparison\nfor invalid addresses in cc list processing.\n\nSigned-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\n---\n git-send-email.perl | 52 +++++++++++++++++++++++++++++++++++++++-------------\n 1 file changed, 39 insertions(+), 13 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 356f99d..5056fdc 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -786,9 +786,11 @@ sub expand_one_alias {\n }\n \n @initial_to = expand_aliases(@initial_to);\n-@initial_to = (map { sanitize_address($_) } @initial_to);\n+@initial_to = validate_address_list(sanitize_address_list(@initial_to));\n @initial_cc = expand_aliases(@initial_cc);\n+@initial_cc = validate_address_list(sanitize_address_list(@initial_cc));\n @bcclist = expand_aliases(@bcclist);\n+@bcclist = validate_address_list(sanitize_address_list(@bcclist));\n \n if ($thread && !defined $initial_reply_to && $prompting) {\n \t$initial_reply_to = ask(\n@@ -839,6 +841,28 @@ sub extract_valid_address {\n \treturn undef;\n }\n \n+sub extract_valid_address_or_die {\n+\tmy $address = shift;\n+\t$address = extract_valid_address($address);\n+\tdie \"error: unable to extract a valid address from: $address\\n\"\n+\t\tif !$address;\n+\treturn $address;\n+}\n+\n+sub validate_address {\n+\tmy $address = shift;\n+\tif (!extract_valid_address($address)) {\n+\t\tprint STDERR \"W: unable to extract a valid address from: $address\\n\";\n+\t\treturn undef;\n+\t}\n+\treturn $address;\n+}\n+\n+sub validate_address_list {\n+\treturn (grep { defined $_ }\n+\t\tmap { validate_address($_) } @_);\n+}\n+\n # Usually don't need to change anything below here.\n \n # we make a \"fake\" message id by taking the current number\n@@ -955,6 +979,10 @@ sub sanitize_address {\n \n }\n \n+sub sanitize_address_list {\n+\treturn (map { sanitize_address($_) } @_);\n+}\n+\n # Returns the local Fully Qualified Domain Name (FQDN) if available.\n #\n # Tightly configured MTAa require that a caller sends a real DNS\n@@ -1017,14 +1045,13 @@ sub maildomain {\n \n sub send_message {\n \tmy @recipients = unique_email_list(@to);\n-\t@cc = (grep { my $cc = extract_valid_address($_);\n+\t@cc = (grep { my $cc = extract_valid_address_or_die($_);\n \t\t      not grep { $cc eq $_ || $_ =~ /<\\Q${cc}\\E>$/ } @recipients\n \t\t    }\n-\t       map { sanitize_address($_) }\n \t       @cc);\n \tmy $to = join (\",\\n\\t\", @recipients);\n \t@recipients = unique_email_list(@recipients,@cc,@bcclist);\n-\t@recipients = (map { extract_valid_address($_) } @recipients);\n+\t@recipients = (map { extract_valid_address_or_die($_) } @recipients);\n \tmy $date = format_2822_time($time++);\n \tmy $gitversion = '@@GIT_VERSION@@';\n \tif ($gitversion =~ m/..GIT_VERSION../) {\n@@ -1267,7 +1294,7 @@ foreach my $t (@files) {\n \t\t\t\tforeach my $addr (parse_address_line($1)) {\n \t\t\t\t\tprintf(\"(mbox) Adding to: %s from line '%s'\\n\",\n \t\t\t\t\t\t$addr, $_) unless $quiet;\n-\t\t\t\t\tpush @to, sanitize_address($addr);\n+\t\t\t\t\tpush @to, $addr;\n \t\t\t\t}\n \t\t\t}\n \t\t\telsif (/^Cc:\\s+(.*)$/) {\n@@ -1376,6 +1403,9 @@ foreach my $t (@files) {\n \t\t($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1));\n \t$needs_confirm = \"inform\" if ($needs_confirm && $confirm_unconfigured && @cc);\n \n+\t@to = validate_address_list(sanitize_address_list(@to));\n+\t@cc = validate_address_list(sanitize_address_list(@cc));\n+\n \t@to = (@initial_to, @to);\n \t@cc = (@initial_cc, @cc);\n \n@@ -1431,14 +1461,10 @@ sub unique_email_list {\n \tmy @emails;\n \n \tforeach my $entry (@_) {\n-\t\tif (my $clean = extract_valid_address($entry)) {\n-\t\t\t$seen{$clean} ||= 0;\n-\t\t\tnext if $seen{$clean}++;\n-\t\t\tpush @emails, $entry;\n-\t\t} else {\n-\t\t\tprint STDERR \"W: unable to extract a valid address\",\n-\t\t\t\t\t\" from: $entry\\n\";\n-\t\t}\n+\t\tmy $clean = extract_valid_address_or_die($entry))\n+\t\t$seen{$clean} ||= 0;\n+\t\tnext if $seen{$clean}++;\n+\t\tpush @emails, $entry;\n \t}\n \treturn @emails;\n }\n-- \n1.8.0.393.gcc9701d\n"},{"id":"203669","messageId":"1353607932-10436-4-git-send-email-krzysiek@podlesie.net","threadId":"32145","inReplyTo":"1353607932-10436-1-git-send-email-krzysiek@podlesie.net","subject":"[PATCH 4/5] git-send-email: ask what to do with an invalid email address","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-22T18:12:11Z","receivedAt":"2012-11-22T18:12:11Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"We used to warn about invalid emails and just drop them. Such warnings\ncan be unnoticed by user or noticed after sending email when we are not\ngiving the \"final sanity check [Y/n]?\"\n\nNow we quit by default.\n\nSigned-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\nSuggested-by: Junio C Hamano <gitster@pobox.com>\n---\n git-send-email.perl | 12 ++++++++++--\n 1 file changed, 10 insertions(+), 2 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 5056fdc..d42dca2 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -852,8 +852,16 @@ sub extract_valid_address_or_die {\n sub validate_address {\n \tmy $address = shift;\n \tif (!extract_valid_address($address)) {\n-\t\tprint STDERR \"W: unable to extract a valid address from: $address\\n\";\n-\t\treturn undef;\n+\t\tprint STDERR \"error: unable to extract a valid address from: $address\\n\";\n+\t\t$_ = ask(\"What to do with this address? ([q]uit|[d]rop): \",\n+\t\t\tvalid_re => qr/^(?:quit|q|drop|d)/i,\n+\t\t\tdefault => 'q');\n+\t\tif (/^d/i) {\n+\t\t\treturn undef;\n+\t\t} elsif (/^q/i) {\n+\t\t\tcleanup_compose_files();\n+\t\t\texit(0);\n+\t\t}\n \t}\n \treturn $address;\n }\n-- \n1.8.0.393.gcc9701d\n"},{"id":"203633","messageId":"1353607932-10436-5-git-send-email-krzysiek@podlesie.net","threadId":"32145","inReplyTo":"1353607932-10436-1-git-send-email-krzysiek@podlesie.net","subject":"[PATCH 5/5] git-send-email: allow edit invalid email address","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-22T18:12:12Z","receivedAt":"2012-11-22T18:12:12Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"In some cases the user may want to send email with \"Cc:\" line with\nemail address we cannot extract. Now we allow user to extract\nsuch email address for us.\n\nSigned-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\n---\n git-send-email.perl | 9 ++++++---\n 1 file changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex d42dca2..9996735 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -851,10 +851,10 @@ sub extract_valid_address_or_die {\n \n sub validate_address {\n \tmy $address = shift;\n-\tif (!extract_valid_address($address)) {\n+\twhile (!extract_valid_address($address)) {\n \t\tprint STDERR \"error: unable to extract a valid address from: $address\\n\";\n-\t\t$_ = ask(\"What to do with this address? ([q]uit|[d]rop): \",\n-\t\t\tvalid_re => qr/^(?:quit|q|drop|d)/i,\n+\t\t$_ = ask(\"What to do with this address? ([q]uit|[d]rop|[e]dit): \",\n+\t\t\tvalid_re => qr/^(?:quit|q|drop|d|edit|e)/i,\n \t\t\tdefault => 'q');\n \t\tif (/^d/i) {\n \t\t\treturn undef;\n@@ -862,6 +862,9 @@ sub validate_address {\n \t\t\tcleanup_compose_files();\n \t\t\texit(0);\n \t\t}\n+\t\t$address = ask(\"Who should the email be sent to (if any)? \",\n+\t\t\tdefault => \"\",\n+\t\t\tvalid_re => qr/\\@.*\\./, confirm_only => 1);\n \t}\n \treturn $address;\n }\n-- \n1.8.0.393.gcc9701d\n"},{"id":"203899","messageId":"7vsj7wti07.fsf@alter.siamese.dyndns.org","threadId":"32145","inReplyTo":"1353607932-10436-3-git-send-email-krzysiek@podlesie.net","subject":"Re: [PATCH 3/5] git-send-email: remove invalid addresses earlier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T17:02:16Z","receivedAt":"2012-11-26T17:02:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Krzysztof Mazur <krzysiek@podlesie.net> writes:\n\n> Some addresses are passed twice to unique_email_list() and invalid addresses\n> may be reported twice per send_message. Now we warn about them earlier\n> and we also remove invalid addresses.\n>\n> This also removes using of undefined values for string comparison\n> for invalid addresses in cc list processing.\n>\n> Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\n> ---\n\nI think there are three kinds of address-looking things we deal\nwith:\n\n * Possibly invalid but meant for human consumption, e.g.\n\n     Cc: Stable Kernel <stable@k.org> # for v3.5 and upwards\n\n   in the commit log message trailer.\n\n * Meant to be fed to our MSA, without losing the human readable\n   part, e.g.\n\n     Cc: Stable Kernel <stable@k.org>\n\n   in the header of the outgoing message.\n\n * Without the human-readable part, e.g.\n\n     stable@k.org\n\n   that is returned by extract_valid_address.\n\nMy understanding is that our input typically comes from the first\nkind and sanitize_address() is meant to massage it into the second\nkind.  The last kind is to be used to drive the underlying sendmail\nmachinery and meant to go in the envelope (this includes message-id\ngeneration).\n\nI do not think send-email adds the first kind (invalid ones) in its\noutput, even though it reads them from its input and copy them to\nits output in the e-mail body part of the payload, but I think it\nadds new addresses to the e-mail header part of the payload (that is\nwhat $from, @initial_to, @initial_cc and @bcclist are all about).\nWe would want to feed them in the third form (i.e. output from\nextract-valid-address on them) when driving the underlying sendmail\nmachinery to place them in the envelope part, but they should be in\nthe second form when we place them on e-mail header lines.  As far\nas I can tell, the resulting code looks correct in this regard.  The\naddresses are sanitized into the second form upfront and validated\nbefore they are placed in @initial_to and friends, and we carry the\nsecond form around most of the time, until we call unique_email_list\nin send_message to pass them through extract_valid_address to turn\nthem into the third form to drive the underlying sendmail.\n\nI however found it a bit confusing while reading the callers of\nvalidate_address{,_list} functions, which not just validate (and\nwarns) but return the ones that pass the test.  Perhaps we would\nwant a brief comment before validate_address, validate_address_list,\nand extract_valid_address{,_or_die} to clarify what they are doing\n(especially what they return)?\n\nThe result still feels somewhat yucky (the yuckiness comes primarily\nfrom the current code, not from the patch but I am mostly focused on\nthe result after applying the patch), in that extract-valid-address\nthat has problem with invalid email addresses will still die when\nfed an address that is not \"sanitized\" first, so any future patch\nthat adds a new address source may still have to suffer the same\nproblem the part that dealt with the Cc list suffered (which your\n1/5 fixed earlier), but I do not offhand think of a good way to\nreorganize them.  We could of course make validate_address() call\nsanitize_address(), but that would be mostly redundant since the new\ncode sanitizes the input upfront.\n\nSo overall, looks good to me.  Thanks.\n\n>  git-send-email.perl | 52 +++++++++++++++++++++++++++++++++++++++-------------\n>  1 file changed, 39 insertions(+), 13 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 356f99d..5056fdc 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -786,9 +786,11 @@ sub expand_one_alias {\n>  }\n>  \n>  @initial_to = expand_aliases(@initial_to);\n> -@initial_to = (map { sanitize_address($_) } @initial_to);\n> +@initial_to = validate_address_list(sanitize_address_list(@initial_to));\n>  @initial_cc = expand_aliases(@initial_cc);\n> +@initial_cc = validate_address_list(sanitize_address_list(@initial_cc));\n>  @bcclist = expand_aliases(@bcclist);\n> +@bcclist = validate_address_list(sanitize_address_list(@bcclist));\n"},{"id":"203895","messageId":"7vobikthpp.fsf@alter.siamese.dyndns.org","threadId":"32145","inReplyTo":"1353607932-10436-5-git-send-email-krzysiek@podlesie.net","subject":"Re: [PATCH 5/5] git-send-email: allow edit invalid email address","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T17:08:34Z","receivedAt":"2012-11-26T17:08:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Krzysztof Mazur <krzysiek@podlesie.net> writes:\n\n> In some cases the user may want to send email with \"Cc:\" line with\n> email address we cannot extract. Now we allow user to extract\n> such email address for us.\n>\n> Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\n> ---\n>  git-send-email.perl | 9 ++++++---\n>  1 file changed, 6 insertions(+), 3 deletions(-)\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index d42dca2..9996735 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -851,10 +851,10 @@ sub extract_valid_address_or_die {\n>  \n>  sub validate_address {\n>  \tmy $address = shift;\n> -\tif (!extract_valid_address($address)) {\n> +\twhile (!extract_valid_address($address)) {\n>  \t\tprint STDERR \"error: unable to extract a valid address from: $address\\n\";\n> -\t\t$_ = ask(\"What to do with this address? ([q]uit|[d]rop): \",\n> -\t\t\tvalid_re => qr/^(?:quit|q|drop|d)/i,\n> +\t\t$_ = ask(\"What to do with this address? ([q]uit|[d]rop|[e]dit): \",\n> +\t\t\tvalid_re => qr/^(?:quit|q|drop|d|edit|e)/i,\n>  \t\t\tdefault => 'q');\n>  \t\tif (/^d/i) {\n>  \t\t\treturn undef;\n> @@ -862,6 +862,9 @@ sub validate_address {\n>  \t\t\tcleanup_compose_files();\n>  \t\t\texit(0);\n>  \t\t}\n> +\t\t$address = ask(\"Who should the email be sent to (if any)? \",\n> +\t\t\tdefault => \"\",\n> +\t\t\tvalid_re => qr/\\@.*\\./, confirm_only => 1);\n\nNot having this new code inside \"elsif (/^e/) { }\" feels somewhat\nsloppy, even though it is not *too* bad.  Also do we know this\nfunction will never be used for addresses other than recipients' (I\ngave a cursory look to see what is done to the $sender and it does\nnot seem to go through this function, tho)?\n"},{"id":"203893","messageId":"20121126173318.GA12101@shrek.podlesie.net","threadId":"32145","inReplyTo":"7vobikthpp.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/5] git-send-email: allow edit invalid email address","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-26T17:33:18Z","receivedAt":"2012-11-26T17:33:18Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Mon, Nov 26, 2012 at 09:08:34AM -0800, Junio C Hamano wrote:\n> Krzysztof Mazur <krzysiek@podlesie.net> writes:\n> \n> > In some cases the user may want to send email with \"Cc:\" line with\n> > email address we cannot extract. Now we allow user to extract\n> > such email address for us.\n> >\n> > Signed-off-by: Krzysztof Mazur <krzysiek@podlesie.net>\n> > ---\n> >  git-send-email.perl | 9 ++++++---\n> >  1 file changed, 6 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/git-send-email.perl b/git-send-email.perl\n> > index d42dca2..9996735 100755\n> > --- a/git-send-email.perl\n> > +++ b/git-send-email.perl\n> > @@ -851,10 +851,10 @@ sub extract_valid_address_or_die {\n> >  \n> >  sub validate_address {\n> >  \tmy $address = shift;\n> > -\tif (!extract_valid_address($address)) {\n> > +\twhile (!extract_valid_address($address)) {\n> >  \t\tprint STDERR \"error: unable to extract a valid address from: $address\\n\";\n> > -\t\t$_ = ask(\"What to do with this address? ([q]uit|[d]rop): \",\n> > -\t\t\tvalid_re => qr/^(?:quit|q|drop|d)/i,\n> > +\t\t$_ = ask(\"What to do with this address? ([q]uit|[d]rop|[e]dit): \",\n> > +\t\t\tvalid_re => qr/^(?:quit|q|drop|d|edit|e)/i,\n> >  \t\t\tdefault => 'q');\n> >  \t\tif (/^d/i) {\n> >  \t\t\treturn undef;\n> > @@ -862,6 +862,9 @@ sub validate_address {\n> >  \t\t\tcleanup_compose_files();\n> >  \t\t\texit(0);\n> >  \t\t}\n> > +\t\t$address = ask(\"Who should the email be sent to (if any)? \",\n> > +\t\t\tdefault => \"\",\n> > +\t\t\tvalid_re => qr/\\@.*\\./, confirm_only => 1);\n> \n> Not having this new code inside \"elsif (/^e/) { }\" feels somewhat\n> sloppy, even though it is not *too* bad.  Also do we know this\n\nok, I will fix that.\n\n> function will never be used for addresses other than recipients' (I\n> gave a cursory look to see what is done to the $sender and it does\n> not seem to go through this function, tho)?\n\nYes, this function is called only from validate_address_just()\nto filter @initial_to, @initial_cc, @bcc_list as early as possible,\nand filter @to and @cc added in each email.\n\nKrzysiek\n"},{"id":"203945","messageId":"7vhaocotsd.fsf@alter.siamese.dyndns.org","threadId":"32145","inReplyTo":"20121126173318.GA12101@shrek.podlesie.net","subject":"Re: [PATCH 5/5] git-send-email: allow edit invalid email address","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T22:58:58Z","receivedAt":"2012-11-26T22:58:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Krzysztof Mazur <krzysiek@podlesie.net> writes:\n\n>> Not having this new code inside \"elsif (/^e/) { }\" feels somewhat\n>> sloppy, even though it is not *too* bad.  Also do we know this\n>\n> ok, I will fix that.\n>\n>> function will never be used for addresses other than recipients' (I\n>> gave a cursory look to see what is done to the $sender and it does\n>> not seem to go through this function, tho)?\n>\n> Yes, this function is called only from validate_address_just()\n> to filter @initial_to, @initial_cc, @bcc_list as early as possible,\n> and filter @to and @cc added in each email.\n\nThanks; when merged to 'pu', this series seems to break t9001.  I'll\npush the result out with breakages but could you take a look?\n\n\nTest Summary Report\n-------------------\nt9001-send-email.sh                              (Wstat: 256 Tests: 102 Failed: 77)\n  Failed tests:  4-7, 9-10, 12-13, 15, 17-21, 23-29, 31-33\n                35, 37, 39, 41, 43, 45, 47, 49, 51-58, 61-88\n                91, 93-95, 98-102\n  Non-zero exit status: 1\n"},{"id":"203952","messageId":"20121126233337.GA31100@shrek.podlesie.net","threadId":"32145","inReplyTo":"7vhaocotsd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/5] git-send-email: allow edit invalid email address","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-26T23:33:37Z","receivedAt":"2012-11-26T23:33:37Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Mon, Nov 26, 2012 at 02:58:58PM -0800, Junio C Hamano wrote:\n> Krzysztof Mazur <krzysiek@podlesie.net> writes:\n> \n> >> Not having this new code inside \"elsif (/^e/) { }\" feels somewhat\n> >> sloppy, even though it is not *too* bad.  Also do we know this\n> >\n> > ok, I will fix that.\n> >\n> >> function will never be used for addresses other than recipients' (I\n> >> gave a cursory look to see what is done to the $sender and it does\n> >> not seem to go through this function, tho)?\n> >\n> > Yes, this function is called only from validate_address_just()\n> > to filter @initial_to, @initial_cc, @bcc_list as early as possible,\n> > and filter @to and @cc added in each email.\n> \n> Thanks; when merged to 'pu', this series seems to break t9001.  I'll\n> push the result out with breakages but could you take a look?\n> \n\nSorry, I tested final version only on an ancient perl 5.8.8 and it really\nworked there. The third patch is broken:\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 9996735..f3bbc16 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -1472,7 +1472,7 @@ sub unique_email_list {\n \tmy @emails;\n \n \tforeach my $entry (@_) {\n-\t\tmy $clean = extract_valid_address_or_die($entry))\n+\t\tmy $clean = extract_valid_address_or_die($entry);\n \t\t$seen{$clean} ||= 0;\n \t\tnext if $seen{$clean}++;\n \t\tpush @emails, $entry;\n\nKrzysiek\n"},{"id":"203954","messageId":"7v7gp7q5yx.fsf@alter.siamese.dyndns.org","threadId":"32145","inReplyTo":"20121126233337.GA31100@shrek.podlesie.net","subject":"Re: [PATCH 5/5] git-send-email: allow edit invalid email address","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-26T23:50:30Z","receivedAt":"2012-11-26T23:50:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Krzysztof Mazur <krzysiek@podlesie.net> writes:\n\n> On Mon, Nov 26, 2012 at 02:58:58PM -0800, Junio C Hamano wrote:\n>> Krzysztof Mazur <krzysiek@podlesie.net> writes:\n>> \n>> >> Not having this new code inside \"elsif (/^e/) { }\" feels somewhat\n>> >> sloppy, even though it is not *too* bad.  Also do we know this\n>> >\n>> > ok, I will fix that.\n>> >\n>> >> function will never be used for addresses other than recipients' (I\n>> >> gave a cursory look to see what is done to the $sender and it does\n>> >> not seem to go through this function, tho)?\n>> >\n>> > Yes, this function is called only from validate_address_just()\n>> > to filter @initial_to, @initial_cc, @bcc_list as early as possible,\n>> > and filter @to and @cc added in each email.\n>> \n>> Thanks; when merged to 'pu', this series seems to break t9001.  I'll\n>> push the result out with breakages but could you take a look?\n>> \n>\n> Sorry, I tested final version only on an ancient perl 5.8.8 and it really\n> worked there. The third patch is broken:\n>\n> diff --git a/git-send-email.perl b/git-send-email.perl\n> index 9996735..f3bbc16 100755\n> --- a/git-send-email.perl\n> +++ b/git-send-email.perl\n> @@ -1472,7 +1472,7 @@ sub unique_email_list {\n>  \tmy @emails;\n>  \n>  \tforeach my $entry (@_) {\n> -\t\tmy $clean = extract_valid_address_or_die($entry))\n> +\t\tmy $clean = extract_valid_address_or_die($entry);\n\nAh, ok, I wasn't looking closely enough.  Thanks for a quick\nturnaround.  Will requeue and push out.\n\n>  \t\t$seen{$clean} ||= 0;\n>  \t\tnext if $seen{$clean}++;\n>  \t\tpush @emails, $entry;\n>\n> Krzysiek\n"},{"id":"203985","messageId":"20121127105308.GA26626@shrek.podlesie.net","threadId":"32145","inReplyTo":"7v7gp7q5yx.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH 5/5] git-send-email: allow edit invalid email address","fromName":"Krzysztof Mazur","fromEmail":"krzysiek@podlesie.net","sentAt":"2012-11-27T10:53:08Z","receivedAt":"2012-11-27T10:53:08Z","isPatch":true,"sender":{"key":"krzysiek@podlesie.net","avatar":null},"body":"On Mon, Nov 26, 2012 at 03:50:30PM -0800, Junio C Hamano wrote:\n> Krzysztof Mazur <krzysiek@podlesie.net> writes:\n> \n> > On Mon, Nov 26, 2012 at 02:58:58PM -0800, Junio C Hamano wrote:\n> >> Krzysztof Mazur <krzysiek@podlesie.net> writes:\n> >> \n> >> >> Not having this new code inside \"elsif (/^e/) { }\" feels somewhat\n> >> >> sloppy, even though it is not *too* bad.  Also do we know this\n> >> >\n> >> > ok, I will fix that.\n> >> >\n> >> >> function will never be used for addresses other than recipients' (I\n> >> >> gave a cursory look to see what is done to the $sender and it does\n> >> >> not seem to go through this function, tho)?\n> >> >\n> >> > Yes, this function is called only from validate_address_just()\n> >> > to filter @initial_to, @initial_cc, @bcc_list as early as possible,\n> >> > and filter @to and @cc added in each email.\n> >> \n> >> Thanks; when merged to 'pu', this series seems to break t9001.  I'll\n> >> push the result out with breakages but could you take a look?\n> >> \n> >\n> > Sorry, I tested final version only on an ancient perl 5.8.8 and it really\n> > worked there. The third patch is broken:\n> >\n> > diff --git a/git-send-email.perl b/git-send-email.perl\n> > index 9996735..f3bbc16 100755\n> > --- a/git-send-email.perl\n> > +++ b/git-send-email.perl\n> > @@ -1472,7 +1472,7 @@ sub unique_email_list {\n> >  \tmy @emails;\n> >  \n> >  \tforeach my $entry (@_) {\n> > -\t\tmy $clean = extract_valid_address_or_die($entry))\n> > +\t\tmy $clean = extract_valid_address_or_die($entry);\n> \n> Ah, ok, I wasn't looking closely enough.  Thanks for a quick\n> turnaround.  Will requeue and push out.\n\nI rechecked that and I've just sent some older broken version. The\npatch that I've sent had date Date: Thu, 22 Nov 2012 19:00:25 +0100,\nbut on my tree I have commit from Date: Thu Nov 22 19:01:55 2012 +0100,\nwhich is exactly the same as the fixed version in your tree.\n\nThanks,\n\nKrzysiek\n"}]}