git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Drew DeVault <sir@cmpwn.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption
Date: Sun, 11 Apr 2021 16:20:16 +0200	[thread overview]
Message-ID: <87blakgaxr.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20210411125431.28971-3-sir@cmpwn.com>


On Sun, Apr 11 2021, Drew DeVault wrote:

> Signed-off-by: Drew DeVault <sir@cmpwn.com>
> ---
>  Documentation/git-send-email.txt | 4 ++--
>  git-send-email.perl              | 3 +++
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index c17c3b400a..520b355e50 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -171,8 +171,8 @@ Sending
>  	Specify the encryption to use, either 'ssl' or 'tls'. 'ssl' enables
>  	generic SSL/TLS support and is typically used on port 465.  'tls'
>  	enables in-band STARTTLS support and is typically used on port 25 or
> -	587.  Use whichever option is recommended by your mail provider.  Any
> -	other value reverts to plain SMTP.  Default is the value of
> +	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`.
>  
>  --smtp-domain=<FQDN>::
> diff --git a/git-send-email.perl b/git-send-email.perl
> index f5bbf1647e..bda5211f0d 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -495,6 +495,9 @@ 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");
> +}

Having not tested this but just eyeballed the code, I'm fairly sure
you're adding a logic error here, or is $smtp_encryption guaranteed to
be defined at this point?

We start with it as undef, then read the config, then the CLI
options. If we've got neither it'll still be undef here, no?

Thus the string comparison will emit a warning.

Maybe I'm overly used to regexen in Perl, but I'd also find this more
readable as:

    $smtp_encryption !~ /^(?:|ssl|tls)$/s

Or something like:

    my @valid_smtp_encryption = ('', qw(ssl tls));
    if (!grep { $_ eq $smtp_encryption } @valid_smtp_encryption) {
        ....
    }


  reply	other threads:[~2021-04-11 14:20 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-11 12:54 [PATCH v2 0/3] git-send-email: improve SSL configuration Drew DeVault
2021-04-11 12:54 ` [PATCH v2 1/3] git-send-email(1): improve smtp-encryption docs Drew DeVault
2021-04-11 14:11   ` Ævar Arnfjörð Bjarmason
2021-04-11 12:54 ` [PATCH v2 2/3] git-send-email: die on invalid smtp_encryption Drew DeVault
2021-04-11 14:20   ` Ævar Arnfjörð Bjarmason [this message]
2021-04-11 14:21     ` Drew DeVault
2021-04-11 14:30       ` Ævar Arnfjörð Bjarmason
2021-04-11 15:06         ` Ævar Arnfjörð Bjarmason
2021-04-11 15:18           ` Drew DeVault
2021-04-11 19:56             ` Ævar Arnfjörð Bjarmason
2021-04-12 12:33               ` Drew DeVault
2021-04-12 13:16                 ` Ævar Arnfjörð Bjarmason
2021-04-13 12:12                   ` Drew DeVault
2021-04-13 14:22                     ` Ævar Arnfjörð Bjarmason
2021-04-13 21:39                     ` Junio C Hamano
2021-04-11 12:54 ` [PATCH v2 3/3] git-send-email: rename 'tls' to 'starttls' Drew DeVault
2021-04-11 14:17   ` Ævar Arnfjörð Bjarmason
2021-04-11 14:22     ` Drew DeVault
2021-04-11 14:43 ` [PATCH 0/2] send-email: simplify smtp.{smtpssl,smtpencryption} parsing Ævar Arnfjörð Bjarmason
2021-04-11 14:43   ` [PATCH 1/2] send-email: remove non-working support for "sendemail.smtpssl" Ævar Arnfjörð Bjarmason
2021-04-11 19:08     ` Junio C Hamano
2021-04-11 19:51       ` Ævar Arnfjörð Bjarmason
2021-05-01  9:15         ` Ævar Arnfjörð Bjarmason
2021-04-11 14:43   ` [PATCH 2/2] send-email: refactor sendemail.smtpencryption config parsing Ævar Arnfjörð Bjarmason

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87blakgaxr.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=sir@cmpwn.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).