On Fri, Sep 18, 2015 at 06:25:24PM -0400, Eric Sunshine wrote:
> On Fri, Sep 18, 2015 at 03:12:50PM -0700, Brian Norris wrote:
> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -1136,7 +1136,7 @@ sub smtp_auth_maybe {
> >
> > # Check mechanism naming as defined in:
> > # https://tools.ietf.org/html/rfc4422#page-8
> > - if ($smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
> > + if ($smtp_auth && $smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
> > die "invalid smtp auth: '${smtp_auth}'";
> > }
>
> Thanks, makes sense. I wonder if moving the check to the point where
> $smtp_auth is actually used (despite the noisier diff) would be cleaner,
> like this:
>
> --- 8< ---
> diff --git a/git-send-email.perl b/git-send-email.perl
> index c5a3f76..2a5ceda 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -1134,12 +1134,6 @@ sub smtp_auth_maybe {
> Authen::SASL->import(qw(Perl));
> };
>
> - # Check mechanism naming as defined in:
> - # https://tools.ietf.org/html/rfc4422#page-8
> - if ($smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
> - die "invalid smtp auth: '${smtp_auth}'";
> - }
> -
> # TODO: Authentication may fail not because credentials were
> # invalid but due to other reasons, in which we should not
> # reject credentials.
> @@ -1154,6 +1148,12 @@ sub smtp_auth_maybe {
> my $cred = shift;
>
> if ($smtp_auth) {
> + # Check mechanism naming as defined in:
> + # https://tools.ietf.org/html/rfc4422#page-8
> + if ($smtp_auth !~ /^(\b[A-Z0-9-_]{1,20}\s*)*$/) {
> + die "invalid smtp auth: '${smtp_auth}'";
> + }
> +
> my $sasl = Authen::SASL->new(
> mechanism => $smtp_auth,
> callback => {
> --- 8< ---
By moving the patch into the sub-subroutine (is this a lambda? I'm not
too familiar with my perl), I think you change the order of the checks.
So, previously, initial password auth would happen after the
$smtp_auth format check. With your patch, it comes first.
My patch:
$ git send-email --smtp-auth="@" ...
...
Send this email? ([y]es|[n]o|[q]uit|[a]ll): y
invalid smtp auth: '@' at ./git-send-email.perl line 1140.
Your patch:
$ git send-email --smtp-auth="@" ...
...
Send this email? ([y]es|[n]o|[q]uit|[a]ll): y
Password for xxxx:
invalid smtp auth: '@' at ./git-send-email.perl line 1155.
Seems like the former is a little better, so you don't have to waste
time with your password too many times.
Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html