threads / patch / 10955

patchAuthenticate only once in git-send-email

Subject: [PATCH] Authenticate only once in git-send-email

## tl;dr

3 messages between Nov 21, 2007 and Nov 22, 2007. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Wincent Colaiuta· Nov 21, 2007, 12:35 UTC · lore

When using git-send-email with SMTP authentication sending a patch series would redundantly authenticate multiple times, once for each patch. In the worst case, this would actually prevent the series from being sent because the server would reply with a "5.5.0 Already Authenticated" status code which would derail the process.

This commit teaches git-send-email to authenticate once and only once at the beginning of the series.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
---
 git-send-email.perl |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Show changes to git-send-email.perl +2 −1
diff --git a/git-send-email.perl b/git-send-email.perl
index 9c6fa64..76baa8e 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -145,6 +145,7 @@ sub format_2822_time {
 
 my $have_email_valid = eval { require Email::Valid; 1 };
 my $smtp;
+my $auth;
 
 sub unique_email_list(@);
 sub cleanup_compose_files();
@@ -635,7 +636,7 @@ X-Mailer: git-send-email $gitversion
 		}
 
 		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
-			$smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
+			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
 		}
 		$smtp->mail( $raw_from ) or die $smtp->message;
 		$smtp->to( @recipients ) or die $smtp->message;

-- 
1.5.3.5.737.gdee1b
Junio C Hamano· Nov 22, 2007, 08:48 UTC · re: Wincent Colaiuta · lore

Re: [PATCH] Authenticate only once in git-send-email

Wincent Colaiuta <win@wincent.com> writes:
> This commit teaches git-send-email to authenticate once and only once at
> the beginning of the series.

Ok. What does $smtp->auth() return? Presumably a true value, but I do not find it the best coding style to hide a call made primarily for its effects not for its return value behind a conditional assignment to a boolean. Eek.

>  		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
> -			$smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> +			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
>  		}
Perhaps something along the lines of...
>  		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
> 			$smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp->message;
> +			undef $smtp_authpass;
>  		}

... or using a separate boolean variable "my $auth_happened" may be more appropriate.

But I am just saying this; I do not care _too_ deeply about it.
Will apply as-is.
Wincent Colaiuta· Nov 22, 2007, 10:07 UTC · re: Junio C Hamano · lore

Re: [PATCH] Authenticate only once in git-send-email

El 22/11/2007, a las 9:48, Junio C Hamano escribió:
Show 7 quoted lines
> Wincent Colaiuta <win@wincent.com> writes:
>
>> This commit teaches git-send-email to authenticate once and only  
>> once at
>> the beginning of the series.
>
> Ok.  What does $smtp->auth() return?  Presumably a true value,
True on success, false on failure.
Show 21 quoted lines
> but I do not find it the best coding style to hide a call made
> primarily for its effects not for its return value behind a
> conditional assignment to a boolean.  Eek.
>
>> 		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
>> -			$smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp- 
>> >message;
>> +			$auth ||= $smtp->auth( $smtp_authuser, $smtp_authpass ) or die  
>> $smtp->message;
>> 		}
>
> Perhaps something along the lines of...
>
>> 		if ((defined $smtp_authuser) && (defined $smtp_authpass)) {
>> 			$smtp->auth( $smtp_authuser, $smtp_authpass ) or die $smtp- 
>> >message;
>> +			undef $smtp_authpass;
>> 		}
>
> ... or using a separate boolean variable "my $auth_happened"
> may be more appropriate.

Of the alternatives you suggest, I think an "$auth_happened" or "$auth_done" flag is probably the nicest/cleanest.

> But I am just saying this; I do not care _too_ deeply about it.
> Will apply as-is.

Yes, I don't really care either. My reasoning for doing it the way I did is that I'm not much of a Perl hacker so I basically wanted to make the change as minimally invasive as possible (in this case prepending "$auth ||= "). But above all, all that I really care about is that the problem gets fixed.

Cheers, Wincent

← back to recent threads