diff mbox series

[v2,3/3] git-send-email: rename 'tls' to 'starttls'

Message ID 20210411125431.28971-4-sir@cmpwn.com (mailing list archive)
State New
Headers show
Series git-send-email: improve SSL configuration | expand

Commit Message

Drew DeVault April 11, 2021, 12:54 p.m. UTC
The name 'tls' is misleading. The 'ssl' option enables a generic
"modern" encryption stack which might very well use TLS; but the 'tls'
option enables STARTTLS support, which works entirely differently.

This renames the canonical option to 'starttls', to make this
distinction more obvious, and adds 'tls' as an alias for starttls, to
avoid breaking config files.

Signed-off-by: Drew DeVault <sir@cmpwn.com>
---
 Documentation/git-send-email.txt |  6 ++++--
 git-send-email.perl              | 10 +++++++---
 2 files changed, 11 insertions(+), 5 deletions(-)

Comments

Ævar Arnfjörð Bjarmason April 11, 2021, 2:17 p.m. UTC | #1
On Sun, Apr 11 2021, Drew DeVault wrote:

> The name 'tls' is misleading. The 'ssl' option enables a generic
> "modern" encryption stack which might very well use TLS; but the 'tls'
> option enables STARTTLS support, which works entirely differently.
>
> This renames the canonical option to 'starttls', to make this
> distinction more obvious, and adds 'tls' as an alias for starttls, to
> avoid breaking config files.
>
> Signed-off-by: Drew DeVault <sir@cmpwn.com>
> ---
>  Documentation/git-send-email.txt |  6 ++++--
>  git-send-email.perl              | 10 +++++++---
>  2 files changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 520b355e50..f8cea9e1f9 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -168,12 +168,14 @@ Sending
>  	unspecified, choosing the envelope sender is left to your MTA.
>  
>  --smtp-encryption=<encryption>::
> -	Specify the encryption to use, either 'ssl' or 'tls'. 'ssl' enables
> -	generic SSL/TLS support and is typically used on port 465.  'tls'
> +	Specify the encryption to use, either 'ssl' or 'starttls'. 'ssl' enables
> +	generic SSL/TLS support and is typically used on port 465.  'starttls'
>  	enables in-band STARTTLS support and is typically used on port 25 or
>  	587.  Use whichever option is recommended by your mail provider.  Leave
>  	empty to disable encryption and use plain SMTP.  Default is the value of
>  	`sendemail.smtpEncryption`.
> ++
> +'tls' is an alias for 'starttls' for legacy reasons.
>  
>  --smtp-domain=<FQDN>::
>  	Specifies the Fully Qualified Domain Name (FQDN) used in the
> diff --git a/git-send-email.perl b/git-send-email.perl
> index bda5211f0d..3f125bc2b8 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -495,8 +495,12 @@ sub read_config {
>  
>  # 'default' encryption is none -- this only prevents a warning
>  $smtp_encryption = '' unless (defined $smtp_encryption);
> -if ($smtp_encryption ne "" && $smtp_encryption ne "ssl" && $smtp_encryption ne "tls") {
> -	die __("Invalid smtp_encryption configuration: expected 'ssl', 'tls', or nothing.\n");
> +if ($smtp_encryption eq "tls") {
> +	# "tls" is an alias for starttls for legacy reasons
> +	$smtp_encryption = "starttls";
> +};

Needless trailing ";".

This and the preceding patch would be more readable if it was
re-arranged in some way as to not rewrite the newly introduced lines
between 2 and 3, maybe:

{
	my $tls_name = "tls";
        if (....)
}

Then you'd only need to change "tls" to "starttls" there.

> +if ($smtp_encryption ne "" && $smtp_encryption ne "ssl" && $smtp_encryption ne "starttls") {
> +	die __("Invalid smtp_encryption configuration: expected 'ssl', 'starttls', or nothing.\n");
>  }
>  
>  # Set CC suppressions
> @@ -1541,7 +1545,7 @@ sub send_message {
>  						 Hello => $smtp_domain,
>  						 Debug => $debug_net_smtp,
>  						 Port => $smtp_server_port);
> -			if ($smtp_encryption eq 'tls' && $smtp) {
> +			if ($smtp_encryption eq 'starttls' && $smtp) {

And this could use the same variable.
Drew DeVault April 11, 2021, 2:22 p.m. UTC | #2
On Sun Apr 11, 2021 at 10:17 AM EDT, Ævar Arnfjörð Bjarmason wrote:
> >  # 'default' encryption is none -- this only prevents a warning
> >  $smtp_encryption = '' unless (defined $smtp_encryption);
> > -if ($smtp_encryption ne "" && $smtp_encryption ne "ssl" && $smtp_encryption ne "tls") {
> > -	die __("Invalid smtp_encryption configuration: expected 'ssl', 'tls', or nothing.\n");
> > +if ($smtp_encryption eq "tls") {
> > +	# "tls" is an alias for starttls for legacy reasons
> > +	$smtp_encryption = "starttls";
> > +};
>
> Needless trailing ";".
>
> This and the preceding patch would be more readable if it was
> re-arranged in some way as to not rewrite the newly introduced lines
> between 2 and 3, maybe:
>
> {
> my $tls_name = "tls";
> if (....)
> }

I disagree that this would be an improvement. It would make the patches
a bit more readlable on their own, but the resulting code would
introduce this bizzare variable which doesn't make sense out of context.
diff mbox series

Patch

diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
index 520b355e50..f8cea9e1f9 100644
--- a/Documentation/git-send-email.txt
+++ b/Documentation/git-send-email.txt
@@ -168,12 +168,14 @@  Sending
 	unspecified, choosing the envelope sender is left to your MTA.
 
 --smtp-encryption=<encryption>::
-	Specify the encryption to use, either 'ssl' or 'tls'. 'ssl' enables
-	generic SSL/TLS support and is typically used on port 465.  'tls'
+	Specify the encryption to use, either 'ssl' or 'starttls'. 'ssl' enables
+	generic SSL/TLS support and is typically used on port 465.  'starttls'
 	enables in-band STARTTLS support and is typically used on port 25 or
 	587.  Use whichever option is recommended by your mail provider.  Leave
 	empty to disable encryption and use plain SMTP.  Default is the value of
 	`sendemail.smtpEncryption`.
++
+'tls' is an alias for 'starttls' for legacy reasons.
 
 --smtp-domain=<FQDN>::
 	Specifies the Fully Qualified Domain Name (FQDN) used in the
diff --git a/git-send-email.perl b/git-send-email.perl
index bda5211f0d..3f125bc2b8 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -495,8 +495,12 @@  sub read_config {
 
 # 'default' encryption is none -- this only prevents a warning
 $smtp_encryption = '' unless (defined $smtp_encryption);
-if ($smtp_encryption ne "" && $smtp_encryption ne "ssl" && $smtp_encryption ne "tls") {
-	die __("Invalid smtp_encryption configuration: expected 'ssl', 'tls', or nothing.\n");
+if ($smtp_encryption eq "tls") {
+	# "tls" is an alias for starttls for legacy reasons
+	$smtp_encryption = "starttls";
+};
+if ($smtp_encryption ne "" && $smtp_encryption ne "ssl" && $smtp_encryption ne "starttls") {
+	die __("Invalid smtp_encryption configuration: expected 'ssl', 'starttls', or nothing.\n");
 }
 
 # Set CC suppressions
@@ -1541,7 +1545,7 @@  sub send_message {
 						 Hello => $smtp_domain,
 						 Debug => $debug_net_smtp,
 						 Port => $smtp_server_port);
-			if ($smtp_encryption eq 'tls' && $smtp) {
+			if ($smtp_encryption eq 'starttls' && $smtp) {
 				if ($use_net_smtp_ssl) {
 					$smtp->command('STARTTLS');
 					$smtp->response();