Re: [PATCH] send-email: handle adjacent RFC 2047-encoded words properly

[Junio C Hamano <gitster@pobox.com>]

On Sun, Nov 23, 2014 at 3:50 PM, Роман Донченко <dpb@corrigendum.ru> wrote:
> The RFC says that they are to be concatenated after decoding (i.e. the
> intervening whitespace is ignored).
>
> I change the sender's name to an all-Cyrillic string in the tests so that
> its encoded form goes over the 76 characters in a line limit, forcing
> format-patch to split it into multiple encoded words.
>
> Since I have to modify the regular expression for an encoded word anyway,
> I take the opportunity to bring it closer to the spec, most notably
> disallowing embedded spaces and making it case-insensitive (thus allowing
> the encoding to be specified as both "q" and "Q").
>
> Signed-off-by: Роман Донченко <dpb@corrigendum.ru>

This sounds like a worthy thing to do in general.

I wonder if the C implementation we have for mailinfo needs similar
update, though. I vaguely recall that we have case-insensitive start for
q/b segments, but do not remember the details offhand.

Was the change to the test to use Cyrillic really necessary, or did it
suffice if you simply extended the existsing "Funny Name" spelled with
strange accents, but you substituted the whole string anyway?

Until I found out what the new string says by running web-based
translation on it, I felt somewhat uneasy. As I do not read
Cyrillic/Russian, we may have been adding some profanity without
knowing. It turns out that the string just says "Cyrillic Name", so I am
not against using the new string, but it simply looked odd to replace the
string whole-sale when you merely need a longer string. It made it look
as if a bug was specific to Cyrillic when it wasn't.

As you may notice by reading "git log --no-merges" from recent history,
we tend not to say "I did X, I did Y". If the tone of the above message
were more similar to them, it may have been easier to read.

But other than these minor nits, the change looks good from
a cursory read.

Thanks.

> ---
>  git-send-email.perl   | 21 +++++++++++++++------
>  t/t9001-send-email.sh | 18 +++++++++---------
>  2 files changed, 24 insertions(+), 15 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 9949db0..4bb9f6f 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -913,13 +913,22 @@ $time = time - scalar $#files;
>
>  sub unquote_rfc2047 {
>         local ($_) = @_;
> +
> +       my $et = qr/[!->@-~]+/; # encoded-text from RFC 2047
> +       my $sep = qr/[ \t]+/;
> +       my $encoded_word = qr/=\?($et)\?q\?($et)\?=/i;
> +
>         my $encoding;
> -       s{=\?([^?]+)\?q\?(.*?)\?=}{
> -               $encoding = $1;
> -               my $e = $2;
> -               $e =~ s/_/ /g;
> -               $e =~ s/=([0-9A-F]{2})/chr(hex($1))/eg;
> -               $e;
> +       s{$encoded_word(?:$sep$encoded_word)+}{
> +               my @words = split $sep, $&;
> +               foreach (@words) {
> +                       m/$encoded_word/;
> +                       $encoding = $1;
> +                       $_ = $2;
> +                       s/_/ /g;
> +                       s/=([0-9A-F]{2})/chr(hex($1))/eg;
> +               }
> +               join '', @words;
>         }eg;
>         return wantarray ? ($_, $encoding) : $_;
>  }
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 19a3ced..318b870 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -236,7 +236,7 @@ test_expect_success $PREREQ 'self name with dot is suppressed' "
>  "
>
>  test_expect_success $PREREQ 'non-ascii self name is suppressed' "
> -       test_suppress_self_quoted 'Füñný Nâmé' 'odd_?=mail@example.com' \
> +       test_suppress_self_quoted 'Кириллическое Имя' 'odd_?=mail@example.com' \
>                 'non_ascii_self_suppressed'
>  "
>
> @@ -946,25 +946,25 @@ test_expect_success $PREREQ 'utf8 author is correctly passed on' '
>         clean_fake_sendmail &&
>         test_commit weird_author &&
>         test_when_finished "git reset --hard HEAD^" &&
> -       git commit --amend --author "Füñný Nâmé <odd_?=mail@example.com>" &&
> -       git format-patch --stdout -1 >funny_name.patch &&
> +       git commit --amend --author "Кириллическое Имя <odd_?=mail@example.com>" &&
> +       git format-patch --stdout -1 >nonascii_name.patch &&
>         git send-email --from="Example <nobody@example.com>" \
>           --to=nobody@example.com \
>           --smtp-server="$(pwd)/fake.sendmail" \
> -         funny_name.patch &&
> -       grep "^From: Füñný Nâmé <odd_?=mail@example.com>" msgtxt1
> +         nonascii_name.patch &&
> +       grep "^From: Кириллическое Имя <odd_?=mail@example.com>" msgtxt1
>  '
>
>  test_expect_success $PREREQ 'utf8 sender is not duplicated' '
>         clean_fake_sendmail &&
>         test_commit weird_sender &&
>         test_when_finished "git reset --hard HEAD^" &&
> -       git commit --amend --author "Füñný Nâmé <odd_?=mail@example.com>" &&
> -       git format-patch --stdout -1 >funny_name.patch &&
> -       git send-email --from="Füñný Nâmé <odd_?=mail@example.com>" \
> +       git commit --amend --author "Кириллическое Имя <odd_?=mail@example.com>" &&
> +       git format-patch --stdout -1 >nonascii_name.patch &&
> +       git send-email --from="Кириллическое Имя <odd_?=mail@example.com>" \
>           --to=nobody@example.com \
>           --smtp-server="$(pwd)/fake.sendmail" \
> -         funny_name.patch &&
> +         nonascii_name.patch &&
>         grep "^From: " msgtxt1 >msgfrom &&
>         test_line_count = 1 msgfrom
>  '
> --
> 2.1.1
>