{"thread":{"id":"38132","subject":"[PATCH v2 2/2] send-email: handle adjacent RFC 2047-encoded words properly","startedAt":"2014-12-06T19:36:22Z","lastAt":"2014-12-10T21:51:54Z","messageCount":7,"participants":["Роман Донченко","Jeff King","Philip Oakley"],"isPatch":true,"patchVersion":2,"patchTotal":2},"messages":[{"id":"253252","messageId":"1417894583-2352-1-git-send-email-dpb@corrigendum.ru","threadId":"38132","inReplyTo":null,"subject":"[PATCH v2 1/2] send-email: align RFC 2047 decoding more closely with the spec","fromName":"Роман Донченко","fromEmail":"dpb@corrigendum.ru","sentAt":"2014-12-06T19:36:22Z","receivedAt":"2014-12-06T19:36:22Z","isPatch":true,"sender":{"key":"dpb@corrigendum.ru","avatar":"https://avatars.githubusercontent.com/u/2391761?v=4"},"body":"More specifically:\n\n* Add \"\\\" to the list of characters not allowed in a token (see RFC 2047\n errata).\n\n* Share regexes between unquote_rfc2047 and is_rfc2047_quoted. Besides\n removing duplication, this also makes unquote_rfc2047 more stringent.\n\n* Allow both \"q\" and \"Q\" to identify the encoding.\n\n* Allow lowercase hexadecimal digits in the \"Q\" encoding.\n\nAnd, more on the cosmetic side:\n\n* Change the \"encoded-text\" regex to exclude rather than include characters,\n for clarity and consistency with \"token\".\n\nSigned-off-by: Роман Донченко \n---\n git-send-email.perl | 30 +++++++++++++++++++-----------\n 1 file changed, 19 insertions(+), 11 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex 9949db0..d461ffb 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -145,6 +145,11 @@ my $have_mail_address = eval { require Mail::Address; 1 };\n my $smtp;\n my $auth;\n \n+# Regexes for RFC 2047 productions.\n+my $re_token = qr/[^][()<>@,;:\\\\\"\\/?.= \\000-\\037\\177-\\377]+/;\n+my $re_encoded_text = qr/[^? \\000-\\037\\177-\\377]+/;\n+my $re_encoded_word = qr/=\\?($re_token)\\?($re_token)\\?($re_encoded_text)\\?=/;\n+\n # Variables we fill in automatically, or via prompting:\n my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@bcclist,$no_bcc,@xh,\n \t$initial_reply_to,$initial_subject,@files,\n@@ -913,15 +918,20 @@ $time = time - scalar $#files;\n \n sub unquote_rfc2047 {\n \tlocal ($_) = @_;\n-\tmy $encoding;\n-\ts{=\\?([^?]+)\\?q\\?(.*?)\\?=}{\n-\t\t$encoding = $1;\n-\t\tmy $e = $2;\n-\t\t$e =~ s/_/ /g;\n-\t\t$e =~ s/=([0-9A-F]{2})/chr(hex($1))/eg;\n-\t\t$e;\n+\tmy $charset;\n+\ts{$re_encoded_word}{\n+\t\t$charset = $1;\n+\t\tmy $encoding = $2;\n+\t\tmy $text = $3;\n+\t\tif ($encoding eq 'q' || $encoding eq 'Q') {\n+\t\t\t$text =~ s/_/ /g;\n+\t\t\t$text =~ s/=([0-9A-F]{2})/chr(hex($1))/egi;\n+\t\t\t$text;\n+\t\t} else {\n+\t\t\t$&; # other encodings not supported yet\n+\t\t}\n \t}eg;\n-\treturn wantarray ? ($_, $encoding) : $_;\n+\treturn wantarray ? ($_, $charset) : $_;\n }\n \n sub quote_rfc2047 {\n@@ -934,10 +944,8 @@ sub quote_rfc2047 {\n \n sub is_rfc2047_quoted {\n \tmy $s = shift;\n-\tmy $token = qr/[^][()<>@,;:\"\\/?.= \\000-\\037\\177-\\377]+/;\n-\tmy $encoded_text = qr/[!->@-~]+/;\n \tlength($s) <= 75 &&\n-\t$s =~ m/^(?:\"[[:ascii:]]*\"|=\\?$token\\?$token\\?$encoded_text\\?=)$/o;\n+\t$s =~ m/^(?:\"[[:ascii:]]*\"|$re_encoded_word)$/o;\n }\n \n sub subject_needs_rfc2047_quoting {\n-- \n2.1.1\n"},{"id":"253251","messageId":"1417894583-2352-2-git-send-email-dpb@corrigendum.ru","threadId":"38132","inReplyTo":"1417894583-2352-1-git-send-email-dpb@corrigendum.ru","subject":"[PATCH v2 2/2] send-email: handle adjacent RFC 2047-encoded words properly","fromName":"Роман Донченко","fromEmail":"dpb@corrigendum.ru","sentAt":"2014-12-06T19:36:23Z","receivedAt":"2014-12-06T19:36:23Z","isPatch":true,"sender":{"key":"dpb@corrigendum.ru","avatar":"https://avatars.githubusercontent.com/u/2391761?v=4"},"body":"The RFC says that they are to be concatenated after decoding (i.e. the\nintervening whitespace is ignored).\n---\n git-send-email.perl | 26 ++++++++++++++++----------\n t/t9001-send-email.sh | 7 +++++++\n 2 files changed, 23 insertions(+), 10 deletions(-)\n\ndiff --git a/git-send-email.perl b/git-send-email.perl\nindex d461ffb..7d5cc8a 100755\n--- a/git-send-email.perl\n+++ b/git-send-email.perl\n@@ -919,17 +919,23 @@ $time = time - scalar $#files;\n sub unquote_rfc2047 {\n \tlocal ($_) = @_;\n \tmy $charset;\n-\ts{$re_encoded_word}{\n-\t\t$charset = $1;\n-\t\tmy $encoding = $2;\n-\t\tmy $text = $3;\n-\t\tif ($encoding eq 'q' || $encoding eq 'Q') {\n-\t\t\t$text =~ s/_/ /g;\n-\t\t\t$text =~ s/=([0-9A-F]{2})/chr(hex($1))/egi;\n-\t\t\t$text;\n-\t\t} else {\n-\t\t\t$&; # other encodings not supported yet\n+\tmy $sep = qr/[ \\t]+/;\n+\ts{$re_encoded_word(?:$sep$re_encoded_word)*}{\n+\t\tmy @words = split $sep, $&;\n+\t\tforeach (@words) {\n+\t\t\tm/$re_encoded_word/;\n+\t\t\t$charset = $1;\n+\t\t\tmy $encoding = $2;\n+\t\t\tmy $text = $3;\n+\t\t\tif ($encoding eq 'q' || $encoding eq 'Q') {\n+\t\t\t\t$_ = $text;\n+\t\t\t\ts/_/ /g;\n+\t\t\t\ts/=([0-9A-F]{2})/chr(hex($1))/egi;\n+\t\t\t} else {\n+\t\t\t\t# other encodings not supported yet\n+\t\t\t}\n \t\t}\n+\t\tjoin '', @words;\n \t}eg;\n \treturn wantarray ? ($_, $charset) : $_;\n }\ndiff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh\nindex 19a3ced..fa965ff 100755\n--- a/t/t9001-send-email.sh\n+++ b/t/t9001-send-email.sh\n@@ -240,6 +240,13 @@ test_expect_success $PREREQ 'non-ascii self name is suppressed' \"\n \t\t'non_ascii_self_suppressed'\n \"\n \n+# This name is long enough to force format-patch to split it into multiple\n+# encoded-words, assuming it uses UTF-8 with the \"Q\" encoding.\n+test_expect_success $PREREQ 'long non-ascii self name is suppressed' \"\n+\ttest_suppress_self_quoted 'Ƒüñníęř €. Nâṁé' 'odd_?=mail@example.com' \\\n+\t\t'long_non_ascii_self_suppressed'\n+\"\n+\n test_expect_success $PREREQ 'sanitized self name is suppressed' \"\n \ttest_suppress_self_unquoted '\\\"A U. Thor\\\"' 'author@example.com' \\\n \t\t'self_name_sanitized_suppressed'\n-- \n2.1.1\n"},{"id":"253269","messageId":"20141207091859.GA21311@peff.net","threadId":"38132","inReplyTo":"1417894583-2352-2-git-send-email-dpb@corrigendum.ru","subject":"Re: [PATCH v2 2/2] send-email: handle adjacent RFC 2047-encoded words properly","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-12-07T09:18:59Z","receivedAt":"2014-12-07T09:18:59Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Dec 06, 2014 at 10:36:23PM +0300, Роман Донченко wrote:\n\n> The RFC says that they are to be concatenated after decoding (i.e. the\n> intervening whitespace is ignored).\n\nThanks. Both patches look good to me, and I'd be happy to have them\napplied as-is. I wrote a few comments below, but in all cases I think I\nconvinced myself that what you wrote is best.\n\n> +\tmy $sep = qr/[ \\t]+/;\n> +\ts{$re_encoded_word(?:$sep$re_encoded_word)*}{\n> +\t\tmy @words = split $sep, $&;\n> +\t\tforeach (@words) {\n> +\t\t\tm/$re_encoded_word/;\n> +\t\t\t$charset = $1;\n> +\t\t\tmy $encoding = $2;\n> +\t\t\tmy $text = $3;\n\nIt feels a little weird to have to split and rematch $re_encoded_word in\nthe loop, but I don't think there is a way around it. ($1, $2, $3) will\nhave our first word, and ($4, $5, $6) will have the final (if any), but\nI don't think we can get access to what is in between.\n\nSo I think what you have here is the best we can do.\n\n> +\t\t\tif ($encoding eq 'q' || $encoding eq 'Q') {\n> +\t\t\t\t$_ = $text;\n> +\t\t\t\ts/_/ /g;\n> +\t\t\t\ts/=([0-9A-F]{2})/chr(hex($1))/egi;\n\nIt took me a minute to figure out why this works. $_ is a reference to\nthe iterator for @words, so it is re-assigning that element of the array\nfirst to the encoded text, and then modifying it in place.\n\nI wonder if it would be more obvious like this:\n\n join '',\n map {\n m/$re_encoded_word/;\n\t $charset = $1;\n\t my $encoding = $2;\n\t my $text = $3;\n if ($encoding eq 'q' || $encoding eq 'Q') {\n\t $text =~ s/_/ /g;\n\t $text =~ s=([0-9A-F]{2}/chr(hex($1))/egi;\n\t } else {\n\t # other encoding not supported yet\n\t }\n } split($sep, $&);\n\n\nI dunno. I kind of like your version better now that I understand it,\nbut it did take me a minute.\n\nOne final note on this bit of code: if there are multiple encoded words,\nwe grab the $charset from the final encoded word, and never report the\nearlier charsets. Technically they do not all have to be the same\n(rfc2047 even has an example where they are not). I think we can dismiss\nthis, though, as:\n\n 1. It was like this before your patches (we might have seen multiple\n non-adjacent encoded words; you're just handling adjacent ones),\n and nobody has complained.\n\n 2. Using two separate encodings in the same header is sufficiently\n ridiculous that I can live with us not handling it properly.\n\n> +# This name is long enough to force format-patch to split it into multiple\n> +# encoded-words, assuming it uses UTF-8 with the \"Q\" encoding.\n> +test_expect_success $PREREQ 'long non-ascii self name is suppressed' \"\n> +\ttest_suppress_self_quoted 'Ƒüñníęř €. Nâṁé' 'odd_?=mail@example.com' \\\n> +\t\t'long_non_ascii_self_suppressed'\n> +\"\n\nCute. :)\n\n-Peff\n"},{"id":"253282","messageId":"op.xqh5hrafnngjn5@freezie","threadId":"38132","inReplyTo":"20141207091859.GA21311@peff.net","subject":"Re: [PATCH v2 2/2] send-email: handle adjacent RFC 2047-encoded words properly","fromName":"Роман Донченко","fromEmail":"dpb@corrigendum.ru","sentAt":"2014-12-07T14:35:41Z","receivedAt":"2014-12-07T14:35:41Z","isPatch":true,"sender":{"key":"dpb@corrigendum.ru","avatar":"https://avatars.githubusercontent.com/u/2391761?v=4"},"body":"Jeff King писал в своём письме Sun, 07 Dec 2014 12:18:59 \n+0300:\n\n> On Sat, Dec 06, 2014 at 10:36:23PM +0300, Роман Донченко wrote:\n>\n>> The RFC says that they are to be concatenated after decoding (i.e. the\n>> intervening whitespace is ignored).\n>\n> Thanks. Both patches look good to me, and I'd be happy to have them\n> applied as-is. I wrote a few comments below, but in all cases I think I\n> convinced myself that what you wrote is best.\n\nI had the same concerns myself, and eventually convinced myself of the \nsame. :-)\n\n> One final note on this bit of code: if there are multiple encoded words,\n> we grab the $charset from the final encoded word, and never report the\n> earlier charsets. Technically they do not all have to be the same\n> (rfc2047 even has an example where they are not). I think we can dismiss\n> this, though, as:\n>\n> 1. It was like this before your patches (we might have seen multiple\n> non-adjacent encoded words; you're just handling adjacent ones),\n> and nobody has complained.\n>\n> 2. Using two separate encodings in the same header is sufficiently\n> ridiculous that I can live with us not handling it properly.\n\nYeah, that bugs me as well. But I think handling multiple encodings would \nrequire substantial reworking of the code, so I chickened out (with the \nsame excuses :-)).\n\nRoman.\n"},{"id":"253284","messageId":"316EF32F3157400882911A84EA0CFC61@PhilipOakley","threadId":"38132","inReplyTo":"op.xqh5hrafnngjn5@freezie","subject":"Re: [PATCH v2 2/2] send-email: handle adjacent RFC 2047-encoded words properly","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2014-12-07T15:34:00Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Роман Донченко\" \n> Jeff King писал в своём письме Sun, 07 Dec 2014 \n> 12:18:59 +0300:\n>\n>> On Sat, Dec 06, 2014 at 10:36:23PM +0300, Роман Донченко wrote:\n>>\n>>> The RFC says that they are to be concatenated after decoding (i.e. \n>>> the\n>>> intervening whitespace is ignored).\n>>\n>> Thanks. Both patches look good to me, and I'd be happy to have them\n>> applied as-is. I wrote a few comments below, but in all cases I think \n>> I\n>> convinced myself that what you wrote is best.\n>\n> I had the same concerns myself, and eventually convinced myself of the \n> same. :-)\n>\n>> One final note on this bit of code: if there are multiple encoded \n>> words,\n>> we grab the $charset from the final encoded word, and never report \n>> the\n>> earlier charsets. Technically they do not all have to be the same\n>> (rfc2047 even has an example where they are not). I think we can \n>> dismiss\n>> this, though, as:\n>>\n>> 1. It was like this before your patches (we might have seen \n>> multiple\n>> non-adjacent encoded words; you're just handling adjacent ones),\n>> and nobody has complained.\n>>\n>> 2. Using two separate encodings in the same header is sufficiently\n>> ridiculous that I can live with us not handling it properly.\n>\n> Yeah, that bugs me as well. But I think handling multiple encodings \n> would require substantial reworking of the code, so I chickened out \n> (with the same excuses :-)).\n\nWould that be worth a terse comment in the documentation change part of \nthe patch?\n\"Multiple (RFC2047) encodings are not supported.\",\nor would that be bike shed noise.\n\nPhilip \n"},{"id":"253287","messageId":"op.xqifqmyznngjn5@freezie","threadId":"38132","inReplyTo":"316EF32F3157400882911A84EA0CFC61@PhilipOakley","subject":"Re: [PATCH v2 2/2] send-email: handle adjacent RFC 2047-encoded words properly","fromName":"Роман Донченко","fromEmail":"dpb@corrigendum.ru","sentAt":"2014-12-07T18:17:00Z","receivedAt":"2014-12-07T18:17:00Z","isPatch":true,"sender":{"key":"dpb@corrigendum.ru","avatar":"https://avatars.githubusercontent.com/u/2391761?v=4"},"body":"Philip Oakley писал в своём письме Sun, 07 Dec 2014 \n20:48:05 +0300:\n\n> From: \"Роман Донченко\" \n>> Jeff King писал в своём письме Sun, 07 Dec 2014 \n>> 12:18:59 +0300:\n>>\n>>> On Sat, Dec 06, 2014 at 10:36:23PM +0300, Роман Донченко wrote:\n>>> One final note on this bit of code: if there are multiple encoded \n>>> words,\n>>> we grab the $charset from the final encoded word, and never report the\n>>> earlier charsets. Technically they do not all have to be the same\n>>> (rfc2047 even has an example where they are not). I think we can \n>>> dismiss\n>>> this, though, as:\n>>>\n>>> 1. It was like this before your patches (we might have seen multiple\n>>> non-adjacent encoded words; you're just handling adjacent ones),\n>>> and nobody has complained.\n>>>\n>>> 2. Using two separate encodings in the same header is sufficiently\n>>> ridiculous that I can live with us not handling it properly.\n>>\n>> Yeah, that bugs me as well. But I think handling multiple encodings \n>> would require substantial reworking of the code, so I chickened out \n>> (with the same excuses :-)).\n>\n> Would that be worth a terse comment in the documentation change part of \n> the patch?\n> \"Multiple (RFC2047) encodings are not supported.\",\n> or would that be bike shed noise.\n\nI didn't change any documentation... and in either case, they weren't \nsupported in the first place, so I don't think it's anything I need to \nmention.\n"},{"id":"253538","messageId":"A6412B2B2F584CDC9ACCB22623CF789A@PhilipOakley","threadId":"38132","inReplyTo":"op.xqifqmyznngjn5@freezie","subject":"Re: [PATCH v2 2/2] send-email: handle adjacent RFC 2047-encoded words properly","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":null,"receivedAt":"2014-12-10T21:51:54Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"From: \"Роман Донченко\" \nSent: Sunday, December 07, 2014 6:17 PM\n>> Would that be worth a terse comment in the documentation change part \n>> of the patch?\n>> \"Multiple (RFC2047) encodings are not supported.\",\n>> or would that be bike shed noise.\n>\n> I didn't change any documentation... and in either case, they weren't \n> supported in the first place, so I don't think it's anything I need to \n> mention.\n\nI'd confused this with the crossing thread by Paolo Bonzini \n [PATCH 2/2] git-send-email: add --transfer-encoding \noption; 25 November 2014 14:00. $gmane/260217.\n\nSorry for the noise.\n--\nPhilip \n"}]}