Skip to content

Commit 6cd3c05

Browse files
Kirill Smelkovgitster
Kirill Smelkov
authored and
gitster
committed
format-patch: RFC 2047 says multi-octet character may not be split
Even though an earlier attempt (bafc478..41dd00b) cleaned up RFC 2047 encoding, pretty.c::add_rfc2047() still decides where to split the output line by going through the input one byte at a time, and potentially splits a character in the middle. A subject line may end up showing like this: ".... fö?? bar". (instead of ".... föö bar".) if split incorrectly. RFC 2047, section 5 (3) explicitly forbids such beaviour 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 e.g. 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. To fix the problem, make the loop grab one _character_ at a time and determine its output length to see where to break the output line. Note that this version only knows about UTF-8, but the logic to grab one character is abstracted out in mbs_chrlen() function to make it possible to extend it to other encodings with the help of iconv in the future. Signed-off-by: Kirill Smelkov <kirr@mns.spb.ru> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 1599999 commit 6cd3c05

4 files changed

Lines changed: 77 additions & 25 deletions

File tree

‎pretty.c‎

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ static int needs_rfc2047_encoding(const char *line, int len,
345345
return 0;
346346
}
347347

348-
static void add_rfc2047(struct strbuf *sb, const char *line, int len,
348+
static void add_rfc2047(struct strbuf *sb, const char *line, size_t len,
349349
const char *encoding, enum rfc2047_type type)
350350
{
351351
static const int max_encoded_length = 76; /* per rfc2047 */
@@ -355,9 +355,22 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,
355355
strbuf_grow(sb, len * 3 + strlen(encoding) + 100);
356356
strbuf_addf(sb, "=?%s?q?", encoding);
357357
line_len += strlen(encoding) + 5; /* 5 for =??q? */
358-
for (i = 0; i < len; i++) {
359-
unsigned ch = line[i] & 0xFF;
360-
int is_special = is_rfc2047_special(ch, type);
358+
359+
while (len) {
360+
/*
361+
* RFC 2047, section 5 (3):
362+
*
363+
* Each 'encoded-word' MUST represent an integral number of
364+
* characters. A multi-octet character may not be split across
365+
* adjacent 'encoded- word's.
366+
*/
367+
const unsigned char *p = (const unsigned char *)line;
368+
int chrlen = mbs_chrlen(&line, &len, encoding);
369+
int is_special = (chrlen > 1) || is_rfc2047_special(*p, type);
370+
371+
/* "=%02X" * chrlen, or the byte itself */
372+
const char *encoded_fmt = is_special ? "=%02X" : "%c";
373+
int encoded_len = is_special ? 3 * chrlen : 1;
361374

362375
/*
363376
* According to RFC 2047, we could encode the special character
@@ -367,18 +380,15 @@ static void add_rfc2047(struct strbuf *sb, const char *line, int len,
367380
* causes ' ' to be encoded as '=20', avoiding this problem.
368381
*/
369382

370-
if (line_len + 2 + (is_special ? 3 : 1) > max_encoded_length) {
383+
if (line_len + encoded_len + 2 > max_encoded_length) {
384+
/* It won't fit with trailing "?=" --- break the line */
371385
strbuf_addf(sb, "?=\n =?%s?q?", encoding);
372386
line_len = strlen(encoding) + 5 + 1; /* =??q? plus SP */
373387
}
374388

375-
if (is_special) {
376-
strbuf_addf(sb, "=%02X", ch);
377-
line_len += 3;
378-
} else {
379-
strbuf_addch(sb, ch);
380-
line_len++;
381-
}
389+
for (i = 0; i < chrlen; i++)
390+
strbuf_addf(sb, encoded_fmt, p[i]);
391+
line_len += encoded_len;
382392
}
383393
strbuf_addstr(sb, "?=");
384394
}

‎t/t4014-format-patch.sh‎

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -810,25 +810,26 @@ Subject: [PATCH] =?UTF-8?q?f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
810810
=?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
811811
=?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
812812
=?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
813-
=?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
814-
=?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
815-
=?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
816-
=?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
813+
=?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
814+
=?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
817815
=?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
818816
=?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
819817
=?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
820-
=?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
821-
=?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
822-
=?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
823-
=?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
818+
=?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
819+
=?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
824820
=?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
825821
=?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
826822
=?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
827-
=?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3?=
828-
=?UTF-8?q?=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
829-
=?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3?=
830-
=?UTF-8?q?=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
831-
=?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
823+
=?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
824+
=?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
825+
=?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
826+
=?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
827+
=?UTF-8?q?bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6?=
828+
=?UTF-8?q?=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6?=
829+
=?UTF-8?q?=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f?=
830+
=?UTF-8?q?=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar?=
831+
=?UTF-8?q?=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20bar=20f=C3=B6=C3=B6=20?=
832+
=?UTF-8?q?bar?=
832833
EOF
833834
test_expect_success 'format-patch wraps extremely long subject (rfc2047)' '
834835
rm -rf patches/ &&

‎utf8.c‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -495,3 +495,42 @@ char *reencode_string(const char *in, const char *out_encoding, const char *in_e
495495
return out;
496496
}
497497
#endif
498+
499+
/*
500+
* Returns first character length in bytes for multi-byte `text` according to
501+
* `encoding`.
502+
*
503+
* - The `text` pointer is updated to point at the next character.
504+
* - When `remainder_p` is not NULL, on entry `*remainder_p` is how much bytes
505+
* we can consume from text, and on exit `*remainder_p` is reduced by returned
506+
* character length. Otherwise `text` is treated as limited by NUL.
507+
*/
508+
int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding)
509+
{
510+
int chrlen;
511+
const char *p = *text;
512+
size_t r = (remainder_p ? *remainder_p : SIZE_MAX);
513+
514+
if (r < 1)
515+
return 0;
516+
517+
if (is_encoding_utf8(encoding)) {
518+
pick_one_utf8_char(&p, &r);
519+
520+
chrlen = p ? (p - *text)
521+
: 1 /* not valid UTF-8 -> raw byte sequence */;
522+
}
523+
else {
524+
/*
525+
* TODO use iconv to decode one char and obtain its chrlen
526+
* for now, let's treat encodings != UTF-8 as one-byte
527+
*/
528+
chrlen = 1;
529+
}
530+
531+
*text += chrlen;
532+
if (remainder_p)
533+
*remainder_p -= chrlen;
534+
535+
return chrlen;
536+
}

‎utf8.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,4 +21,6 @@ char *reencode_string(const char *in, const char *out_encoding, const char *in_e
2121
#define reencode_string(a,b,c) NULL
2222
#endif
2323

24+
int mbs_chrlen(const char **text, size_t *remainder_p, const char *encoding);
25+
2426
#endif

0 commit comments

Comments
 (0)