Re: [PATCH] send-email: improve RFC2047 quote parsing

2012-07-31 Thread Thomas Rast
Junio C Hamano  writes:

> Thomas Rast  writes:
>
>> This patch fixes the first two by doing a more careful decoding of the
>> =AB outer quoting.  Fixing the fundamental issues is left for a
>> future, more intrusive, patch.
>
> What is this =AB thing?

The two-hex-digits quoting in the style of MIME quoted-printable.  I
called it the "outer quoting" (RFC2047: "encoding") because it serves to
protect the bytes from transport damage; there is another encoding
(RFC2047: "character set") "inside" which is specified by the
=?utf-8?...?= wrapper.

BTW, note that we also only handle the Q outer quoting
(quoted-printable).  There is a B encoding, which your email in fact
used in the Cc: header:

Cc: [...] =?utf-8?B?SsO8cmdlbiBSw7xobGU=?= 

B means Base64, as you can probably guess from the looks of it.

>> This is the easy part, fixed as per Junio's comment that it needs to
>> use a .*? match for the contents, and with a test.
>
> What's the hard part?  Do you mean the "fundamentally cannot" part?

Yes, and by "fundamentally" I meant "not without fixing something
outside of this function, which I am too lazy to do at this time". ;-)

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] send-email: improve RFC2047 quote parsing

2012-07-30 Thread Junio C Hamano
Thomas Rast  writes:

> The RFC2047 unquoting, used to parse email addresses in From and Cc
> headers, is broken in several ways:
>
> * It erroneously substitutes ' ' for '_' in *the whole* header, even
>   outside the quoted field. [Noticed by Christoph.]
>
> * It is too liberal in its matching, and happily matches the start of
>   one quoted chunk against the end of another, or even just something
>   that looks like such an end. [Noticed by Junio.]
>
> * It fundamentally cannot cope with encodings that are not a superset
>   of ASCII, nor several (incompatible) encodings in the same header.
>
> This patch fixes the first two by doing a more careful decoding of the
> =AB outer quoting.  Fixing the fundamental issues is left for a
> future, more intrusive, patch.

What is this =AB thing?

>
> Noticed-by: Christoph Miebach 
> Helped-by: Junio C Hamano 
> Signed-off-by: Thomas Rast 
> ---
>
> This is the easy part, fixed as per Junio's comment that it needs to
> use a .*? match for the contents, and with a test.

What's the hard part?  Do you mean the "fundamentally cannot" part?

Thanks.

>  git-send-email.perl   | 10 ++
>  t/t9001-send-email.sh | 13 +
>  2 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index ef30c55..6647137 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -862,11 +862,13 @@ sub make_message_id {
>  sub unquote_rfc2047 {
>   local ($_) = @_;
>   my $encoding;
> - if (s/=\?([^?]+)\?q\?(.*)\?=/$2/g) {
> + s{=\?([^?]+)\?q\?(.*?)\?=}{
>   $encoding = $1;
> - s/_/ /g;
> - s/=([0-9A-F]{2})/chr(hex($1))/eg;
> - }
> + my $e = $2;
> + $e =~ s/_/ /g;
> + $e =~ s/=([0-9A-F]{2})/chr(hex($1))/eg;
> + $e;
> + }eg;
>   return wantarray ? ($_, $encoding) : $_;
>  }
>  
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index 8c12c65..0351228 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -841,6 +841,19 @@ test_expect_success $PREREQ '--compose adds MIME for 
> utf8 subject' '
>   grep "^Subject: =?UTF-8?q?utf8-s=C3=BCbj=C3=ABct?=" msgtxt1
>  '
>  
> +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é " &&
> + git format-patch --stdout -1 >funny_name.patch &&
> + git send-email --from="Example " \
> +   --to=nob...@example.com \
> +   --smtp-server="$(pwd)/fake.sendmail" \
> +   funny_name.patch &&
> + grep "^From: Füñný Nâmé " msgtxt1
> +'
> +
>  test_expect_success $PREREQ 'detects ambiguous reference/file conflict' '
>   echo master > master &&
>   git add master &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] send-email: improve RFC2047 quote parsing

2012-07-30 Thread Thomas Rast
The RFC2047 unquoting, used to parse email addresses in From and Cc
headers, is broken in several ways:

* It erroneously substitutes ' ' for '_' in *the whole* header, even
  outside the quoted field. [Noticed by Christoph.]

* It is too liberal in its matching, and happily matches the start of
  one quoted chunk against the end of another, or even just something
  that looks like such an end. [Noticed by Junio.]

* It fundamentally cannot cope with encodings that are not a superset
  of ASCII, nor several (incompatible) encodings in the same header.

This patch fixes the first two by doing a more careful decoding of the
=AB outer quoting.  Fixing the fundamental issues is left for a
future, more intrusive, patch.

Noticed-by: Christoph Miebach 
Helped-by: Junio C Hamano 
Signed-off-by: Thomas Rast 
---

This is the easy part, fixed as per Junio's comment that it needs to
use a .*? match for the contents, and with a test.

 git-send-email.perl   | 10 ++
 t/t9001-send-email.sh | 13 +
 2 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index ef30c55..6647137 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -862,11 +862,13 @@ sub make_message_id {
 sub unquote_rfc2047 {
local ($_) = @_;
my $encoding;
-   if (s/=\?([^?]+)\?q\?(.*)\?=/$2/g) {
+   s{=\?([^?]+)\?q\?(.*?)\?=}{
$encoding = $1;
-   s/_/ /g;
-   s/=([0-9A-F]{2})/chr(hex($1))/eg;
-   }
+   my $e = $2;
+   $e =~ s/_/ /g;
+   $e =~ s/=([0-9A-F]{2})/chr(hex($1))/eg;
+   $e;
+   }eg;
return wantarray ? ($_, $encoding) : $_;
 }
 
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 8c12c65..0351228 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -841,6 +841,19 @@ test_expect_success $PREREQ '--compose adds MIME for utf8 
subject' '
grep "^Subject: =?UTF-8?q?utf8-s=C3=BCbj=C3=ABct?=" msgtxt1
 '
 
+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é " &&
+   git format-patch --stdout -1 >funny_name.patch &&
+   git send-email --from="Example " \
+ --to=nob...@example.com \
+ --smtp-server="$(pwd)/fake.sendmail" \
+ funny_name.patch &&
+   grep "^From: Füñný Nâmé " msgtxt1
+'
+
 test_expect_success $PREREQ 'detects ambiguous reference/file conflict' '
echo master > master &&
git add master &&
-- 
1.7.12.rc0.434.gd809d0f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html