Intro
-----

In 'Subject:' characters are encoded in Q encoding, as per RFC 2047, e.g.

    föö

becomes

    =?UTF-8?q?f=C3=B6=C3=B6?=

. Long encoded lines must be wrapped to be no longer than 76 bytes.

Also RFC 2047, section 5 (3) says:

    Each 'encoded-word' MUST represent an integral number of
    characters.  A multi-octet character may not be split across
    adjacent 'encoded- word's.

that means that for

    Subject: .... föö bar

encoding

    Subject: =?UTF-8?q?....=20f=C3=B6=C3=B6?=
     =?UTF-8?q?=20bar?=

is correct, and

    Subject: =?UTF-8?q?....=20f=C3=B6=C3?=      <-- NOTE ö is broken here
     =?UTF-8?q?=B6=20bar?=

is not, because "ö" character UTF-8 encoding C3 B6 is split here across
adjacent encoded words.

~~~~

As it is now, format-patch does not respect "multi-octet charactes may
not be split" rule, and so sending patches with non-english subject has
issues:

    The problematic case shows in mail readers as ".... fö?? bar".

Solution
--------

I introduce mbs_chrlen() function to compute character length in bytes
for multi-byte text according to encoding, and use it appropriately in
add_rfc2047() in pretty.

So far it works correctly only for UTF-8 encoding, because we have
infrastructure for it in place already, but other encoding could be
supported too in the future with the help of iconv. For now they all, except
UTF-8, are treated as being one-byte encodings, which was format-patch
current behaviour, with appropriate TODO put in mbs_chrlen().

Not sure whether mbs_chrlen() is a good name, but otherwise my
understanding is that the patch is ok to go in.

Thanks.

Cc: Jan H. Schönherr <schn...@cs.tu-berlin.de>
Signed-off-by: Kirill Smelkov <k...@mns.spb.ru>
---
 pretty.c                | 27 +++++++++++++++++++--------
 t/t4014-format-patch.sh | 27 ++++++++++++++-------------
 utf8.c                  | 39 +++++++++++++++++++++++++++++++++++++++
 utf8.h                  |  2 ++
 4 files changed, 74 insertions(+), 21 deletions(-)

diff --git a/pretty.c b/pretty.c
index b57adef..c9c7ff5 100644
--- a/pretty.c
+++ b/pretty.c
@@ -345,7 +345,7 @@ static int needs_rfc2047_encoding(const char *line, int len,
        return 0;
 }
 
-static void add_rfc2047(struct strbuf *sb, const char *line, int len,
+static void add_rfc2047(struct strbuf *sb, const char *line, size_t len,
                       const char *encoding, enum rfc2047_type type)
 {
        static const int max_encoded_length = 76; /* per rfc2047 */
@@ -355,9 +355,18 @@ static void add_rfc2047(struct strbuf *sb, const char 
*line, int len,
        strbuf_grow(sb, len * 3 + strlen(encoding) + 100);
        strbuf_addf(sb, "=?%s?q?", encoding);
        line_len += strlen(encoding) + 5; /* 5 for =??q? */
-       for (i = 0; i < len; i++) {
-               unsigned ch = line[i] & 0xFF;
-               int is_special = is_rfc2047_special(ch, type);
+
+       while (len) {
+               /*
+                * RFC 2047, section 5 (3):
+                *
+                * Each 'encoded-word' MUST represent an integral number of
+                * characters.  A multi-octet character may not be split across
+                * adjacent 'encoded- word's.
+                */
+               const unsigned char *p = (const unsigned char *)line;
+               int chrlen = mbs_chrlen(&line, &len, encoding);
+               int is_special = (chrlen > 1) || is_rfc2047_special(*p, type);
 
                /*
                 * According to RFC 2047, we could encode the special character
@@ -367,16 +376,18 @@ static void add_rfc2047(struct strbuf *sb, const char 
*line, int len,
                 * causes ' ' to be encoded as '=20', avoiding this problem.
                 */
 
-               if (line_len + 2 + (is_special ? 3 : 1) > max_encoded_length) {
+               if (line_len + 2 + (is_special ? 3*chrlen : 1) > 
max_encoded_length) {
                        strbuf_addf(sb, "?=\n =?%s?q?", encoding);
                        line_len = strlen(encoding) + 5 + 1; /* =??q? plus SP */
                }
 
                if (is_special) {
-                       strbuf_addf(sb, "=%02X", ch);
-                       line_len += 3;
+                       for (i = 0; i < chrlen; i++) {
+                               strbuf_addf(sb, "=%02X", p[i]);
+                               line_len += 3;
+                       }
                } else {
-                       strbuf_addch(sb, ch);
+                       strbuf_addch(sb, *p);
                        line_len++;
                }
        }
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 78633cb..b993dae 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -837,25 +837,26 @@ Subject: [PATCH] 
=?UTF-8?q?f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
  =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
  =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
  =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
- =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
- =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
- =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
  =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
  =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
  =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
- =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
- =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
- =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
  =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
  =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
  =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
- =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
- =?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
- =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
- =?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
- =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
+ =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
+ =?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
+ =?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
+ =?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
+ =?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
+ =?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
+ =?UTF-8?q?bar?=
 EOF
 test_expect_success 'format-patch wraps extremely long subject (rfc2047)' '
        rm -rf patches/ &&
diff --git a/utf8.c b/utf8.c
index 8f6e84b..7911b58 100644
--- a/utf8.c
+++ b/utf8.c
@@ -531,3 +531,42 @@ char *reencode_string(const char *in, const char 
*out_encoding, const char *in_e
        return out;
 }
 #endif
+
+/*
+ * Returns first character length in bytes for multi-byte `text` according to
+ * `encoding`.
+ *
+ * - The `text` pointer is updated to point at the next character.
+ * - When `remainder_p` is not NULL, on entry `*remainder_p` is how much bytes
+ *   we can consume from text, and on exit `*remainder_p` is reduced by 
returned
+ *   character length. Otherwise `text` is treated as limited by NUL.
+ */
+int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding)
+{
+       int chrlen;
+       const char *p = *text;
+       size_t r = (remainder_p ? *remainder_p : INT_MAX);
+
+       if (r < 1)
+               return 0;
+
+       if (is_encoding_utf8(encoding)) {
+               pick_one_utf8_char(&p, &r);
+
+               chrlen = p ? (p - *text)
+                          : 1 /* not valid UTF-8 -> raw byte sequence */;
+       }
+       else {
+               /* TODO use iconv to decode one char and obtain its chrlen
+                *
+                * for now, let's treat encodings != UTF-8 as one-byte
+                */
+               chrlen = 1;
+       }
+
+       *text += chrlen;
+       if (remainder_p)
+               *remainder_p -= chrlen;
+
+       return chrlen;
+}
diff --git a/utf8.h b/utf8.h
index 501b2bd..1f8ecad 100644
--- a/utf8.h
+++ b/utf8.h
@@ -22,4 +22,6 @@ char *reencode_string(const char *in, const char 
*out_encoding, const char *in_e
 #define reencode_string(a,b,c) NULL
 #endif
 
+int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
+
 #endif
-- 
1.8.2.rc2.353.gd2380b4

--
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

Reply via email to