From: D. Ben Knoble Date: Wed, 25 Feb 2026 16:37:57 GMT Subject: Re: [PATCH v2] send-email: validate charset name in 8bit encoding prompt Message-ID: In-Reply-To: <20260224213932.92364-1-shreyanshpaliwalcmsmn@gmail.com> On Tue, Feb 24, 2026 at 4:39 PM Shreyansh Paliwal wrote: > > 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 [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 [y/N]? > > [1]- https://github.com/git/git/commit/852a15d748034eec87adbee73a72689c8936fb8b > > Signed-off-by: Shreyansh Paliwal > --- > 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; > + } I think this is asking "ask" to do too much, since only encoding askers can use warn_invalid. What I rather meant was to extract relevant helper procedures so that open-coding ask around the encoding question would be easier to maintain. > } > 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 -- D. Ben Knoble