[GSoC,v3,1/1] Unify SMTP auth error handling

Commit Message

Zheng Yuting March 12, 2025, 6:46 a.m. UTC

Refactored SMTP authentication to use a unified error capture block for
both SASL and plain methods. Errors are now handled by parsing SMTP status
codes (4yz for transient, 5yz for permanent) instead of relying on regex
matching. This change improves clarity .


Signed-off-by: Zheng Yuting <[e-mail]>
---
 git-send-email.perl | 72 +++++++++++++++++++++++++++------------------
 1 file changed, 43 insertions(+), 29 deletions(-)

--
2.49.0.rc0.57.gdb91954e18

Patch

diff --git a/git-send-email.perl b/git-send-email.perl
index a012d61abb..532dda264c 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -1411,7 +1411,7 @@  sub smtp_auth_maybe {
 	eval {
 		require Authen::SASL;
 		Authen::SASL->import(qw(Perl));
-	};
+	}

 	# Check mechanism naming as defined in:
 	# https://tools.ietf.org/html/rfc4422#page-8
@@ -1426,42 +1426,56 @@  sub smtp_auth_maybe {
 		'protocol' => 'smtp',
 		'host' => smtp_host_string(),
 		'username' => $smtp_authuser,
+		# if there's no password, "git credential fill" will
+		# give us one, otherwise it'll just pass this one.
 		'password' => $smtp_authpass
-
 	}, sub {
 		my $cred = shift;
 		my $result;
 		my $error;
-		if ($smtp_auth) {
-			my $sasl = Authen::SASL->new(
-				mechanism => $smtp_auth,
-				callback => {
-					user => $cred->{'username'},
-					pass => $cred->{'password'},
-					authname => $cred->{'username'},
-				}
-			);
-			return !!$smtp->auth($sasl);
-		} else {
-			# Handle plain authentication errors
-			eval {
+
+		# catch all SMTP auth error
+		eval {
+			if ($smtp_auth) {
+				my $sasl = Authen::SASL->new(
+					mechanism => $smtp_auth,
+					callback => {
+						user => $cred->{'username'},
+						pass => $cred->{'password'},
+						authname => $cred->{'username'},
+					}
+				);
+				$result = $smtp->auth($sasl);
+			} else {
 				$result = $smtp->auth($cred->{'username'}, $cred->{'password'});
-				1; # Ensure true value is returned
-			} or do {
-				$error = $@ || 'Unknown error';
-			};
-		}
-		# Unified error handling logic
+			}
+			1; # Ensure true value is returned if no exception is thrown.
+		} or do {
+			$error = $@ || 'Unknown error';
+		};
+
+		#check if an error was captured
 		if ($error) {
-			# Match temporary errors
-			if ($error =~ /timeout|temporary|greylist|throttled|quota\s+exceeded|queue|overload|try\s+again|connection\s+lost|network\s+error/i) {
-				warn "SMTP temporary error: $error";
-				return 1;
+			#Parse SMTP status code from error message in:
+			#https://www.rfc-editor.org/rfc/rfc5321.html
+			if ($error =~ /\b(\d{3})\b/) {
+				my $status_code = $1;
+				if ($status_code =~ /^4/) {
+					# 4yz: Transient Negative Completion reply
+					warn "SMTP temporary error (status code $status_code): $error";
+					return 1;
+				} elsif ($status_code =~ /^5/) {
+					# 5yz: Permanent Negative Completion reply
+					warn "SMTP permanent error (status code $status_code): $error";
+					return 0;
+				}
+				# If no status code is found, treat as permanent error
+				warn "SMTP unknown error: $error";
+				return 0;
 			}
-			return 0;
-		}
-		return !!$result;
-	});
+			return $result ? 1 : 0;
+		});
+
 	return $auth;
 }