Re: [PATCH v2] send-email: validate charset name in 8bit encoding prompt

>
> When a non-ASCII character is detected in the body or subject of the email
> the user is prompted with,
>
>   Which 8bit encoding should I declare [UTF-8]? foo
>
> After this the input string is validated by the regex, based on the fact
> that the charset string will be minimum 4 characters [1]. If the string is
> more than 4 letters the email is sent, if not then a second prompt to
> confirm is asked to the user,
>
>   Are you sure you want to use <foo> [y/N]? y
>
> This relies on a length based regex heuristic check to validate the user
> input, and can allow clearly invalid charset names to pass if the input is
> greater than 4 characters.
>
> Add a semantic validation of the charset name using the
> Encode::find_encoding() module of perl. If the encoding is not recognized,
> warn the user and ask for confirmation before proceeding. After this
> validation the lenght based validation becomes redundant and also breaks
> flow, so change the regex of valid input to any non blank string.
>
> Additionally, the wording of the first prompt can confuse the user if not
> read properly or under any default assumptions for a yes/no prompt. Change
> the wording to make it explicitly clear to the user that the prompt needs a
> string input, UTF-8 being the default.
>
> The intended flow is,
>
>   Declare which 8bit encoding to use [default: UTF-8]? foobar
>   warning: 'foobar' does not appear to be a valid charset name.
>   Are you sure you want to use <foobar> [y/N]?
>
> [1]- https://github.com/git/git/commit/852a15d748034eec87adbee73a72689c8936fb8b
>
> Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
> ---
> Changes in v2:
>  - Added braces in if-else block.
>
>  git-send-email.perl   | 17 ++++++++++++++---
>  t/t9001-send-email.sh |  2 +-
>  2 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index cd4b316ddc..15387ac377 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -23,6 +23,7 @@
>  use Git::LoadCPAN::Error qw(:try);
>  use Git;
>  use Git::I18N;
> +use Encode qw(find_encoding);
>
>  Getopt::Long::Configure qw/ pass_through /;
>
> @@ -987,6 +988,7 @@ sub get_patch_subject {
>  sub ask {
>         my ($prompt, %arg) = @_;
>         my $valid_re = $arg{valid_re};
> +       my $warn_invalid = $arg{warn_invalid};
>         my $default = $arg{default};
>         my $confirm_only = $arg{confirm_only};
>         my $resp;
> @@ -1005,7 +1007,15 @@ sub ask {
>                         return $default;
>                 }
>                 if (!defined $valid_re or $resp =~ /$valid_re/) {
> -                       return $resp;
> +                       if ($warn_invalid) {
> +                               if (find_encoding($resp)) {
> +                                       return $resp;
> +                               } else {
> +                                       printf STDERR __("warning: '%s' does not appear to be a valid charset name.\n"), $resp;
> +                               }
> +                       } else {
> +                               return $resp;
> +                       }

>                 }
>                 if ($confirm_only) {
>                         my $yesno = $term->readline(
> @@ -1044,8 +1054,9 @@ sub file_declares_8bit_cte {
>         foreach my $f (sort keys %broken_encoding) {
>                 print "    $f\n";
>         }
> -       $auto_8bit_encoding = ask(__("Which 8bit encoding should I declare [UTF-8]? "),
> -                                 valid_re => qr/.{4}/, confirm_only => 1,
> +       $auto_8bit_encoding = ask(__("Declare which 8bit encoding to use [default: UTF-8]? "),
> +                                 valid_re => qr/^\S+$/, confirm_only => 1,
> +                                 warn_invalid => 1,
>                                   default => "UTF-8");
>  }
>
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index e56e0c8d77..24f6c76aee 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -1691,7 +1691,7 @@ test_expect_success $PREREQ 'asks about and fixes 8bit encodings' '
>                         email-using-8bit >stdout &&
>         grep "do not declare a Content-Transfer-Encoding" stdout &&
>         grep email-using-8bit stdout &&
> -       grep "Which 8bit encoding" stdout &&
> +       grep "Declare which 8bit encoding to use" stdout &&
>         grep -E "Content|MIME" msgtxt1 >actual &&
>         test_cmp content-type-decl actual
>  '
>
> Range-diff against v1:
> 1:  70fa4d2899 ! 1:  954c1dae9f send-email: validate charset name in 8bit encoding prompt
>     @@ git-send-email.perl: sub ask {
>                 if (!defined $valid_re or $resp =~ /$valid_re/) {
>      -                  return $resp;
>      +                  if ($warn_invalid) {
>     -+                          if (find_encoding($resp))
>     ++                          if (find_encoding($resp)) {
>      +                                  return $resp;
>     -+                          else
>     ++                          } else {
>      +                                  printf STDERR __("warning: '%s' does not appear to be a valid charset name.\n"), $resp;
>     -+                  } else
>     ++                          }
>     ++                  } else {
>      +                          return $resp;
>     ++                  }
>                 }
>                 if ($confirm_only) {
>                         my $yesno = $term->readline(
> --
> 2.53.0