Re: [PATCH v2] send-email: validate charset name in 8bit encoding prompt
- From
Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
- Date
- Feb 26, 2026, 17:32 UTC
- Message-ID
- <20260226173336.194601-1-shreyanshpaliwalcmsmn@gmail.com>
- In-Reply-To
- <CALnO6CDSJPnVi-1RUsr7tFMwa0_xTJkiQmzTL_b-BGq=6PSz0A@mail.gmail.com>
Show 88 quoted lines
> On Tue, Feb 24, 2026 at 4:39 PM Shreyansh Paliwal
> <shreyanshpaliwalcmsmn@gmail.com> 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 <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;
> > + }
>
> 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.Hi,
I have sent a v3 on this, in which I introduced a helper for the confirmation prompt and also made validation logic specific to the 8bit prompt. Do you think it would also be better to move the encoding validation into a separate helper, or does the current split look reasonable?
Best, Shreyansh