[PATCH RFC] mailinfo: correctly handle multiline 'Subject:' header

Subsystems: the rest

DORMANTno replies

4 messages, 4 authors, 2016-08-14 · open the first message on its own page

[PATCH RFC] mailinfo: correctly handle multiline 'Subject:' header

From: Kirill Smelkov <hidden>
Date: 2016-08-14 00:39:35

When native language (RU) is in use, subject header usually contains several
parts, e.g.

Subject: [Navy-patches] [PATCH]
	=?utf-8?b?0JjQt9C80LXQvdGR0L0g0YHQv9C40YHQvtC6INC/0LA=?=
	=?utf-8?b?0LrQtdGC0L7QsiDQvdC10L7QsdGF0L7QtNC40LzRi9GFINC00LvRjyA=?=
	=?utf-8?b?0YHQsdC+0YDQutC4?=

This exposes several bugs in builtin-mailinfo.c that I try to fix:


1. decode_b_segment: do not append explicit NUL -- explicit NUL was preventing
   correct header construction on parts concatenation via strbuf_addbuf in
   decode_header_bq. Fixes:

-Subject: �������������� ������������ �������������� ���������������������� ������ ������������
+Subject: �������������� ������������ ����


Then

2. (hackish) do not emit '\n' after processing of every header segment. It
   seems we should emit previous part as-is only if it does not end with
   '=?='. Fixes:

-Subject: �������������� ������������ �������������� ���������������������� ������ ������������
+Subject: �������������� ������������ ���� ���������� ���������������������� ������ ������������


Sorry for low-quality patch and description. I did what I could and don't have
energy and time dig more into MIME.

Please help.

Signed-off-by: Kirill Smelkov <redacted>

---
 builtin-mailinfo.c  |   18 ++++++++++++++++-
 t/t5100-mailinfo.sh |    2 +-
 t/t5100/info0012    |    5 ++++
 t/t5100/msg0012     |    7 ++++++
 t/t5100/patch0012   |   30 +++++++++++++++++++++++++++++
 t/t5100/sample.mbox |   52 +++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 112 insertions(+), 2 deletions(-)
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index e890f7a..d138bc3 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -436,6 +436,14 @@ static struct strbuf *decode_b_segment(const struct strbuf *b_seg)
 			 * for now we just trust the data.
 			 */
 			c = 0;
+
+			/* XXX: the following is needed not to output NUL in
+			 * the resulting string
+			 *
+			 * This seems to be ok, but I'm not 100% sure -- that's
+			 * why this is an RFC.
+			 */
+			continue;
 		}
 		else
 			continue; /* garbage */
@@ -513,7 +521,15 @@ static int decode_header_bq(struct strbuf *it)
 		strbuf_reset(&piecebuf);
 		rfc2047 = 1;
 
-		if (in != ep) {
+		/* XXX: the follwoing is needed not to output '\n' on every
+		 * multi-line segment in Subject.
+		 *
+		 * I suspect this is not 100% correct, but I'm not a MIME guy
+		 * -- that's why this is an RFC.
+		 */
+
+		/* if in does not end with '=?=', we emit it as is */
+		if (in <= (ep-2) && !(ep[-1]=='\n' && ep[-2]=='=')) {
 			strbuf_add(&outbuf, in, ep - in);
 			in = ep;
 		}
diff --git a/t/t5100-mailinfo.sh b/t/t5100-mailinfo.sh
index fe14589..6825f99 100755
--- a/t/t5100-mailinfo.sh
+++ b/t/t5100-mailinfo.sh
@@ -11,7 +11,7 @@ test_expect_success 'split sample box' \
 	'git mailsplit -o. "$TEST_DIRECTORY"/t5100/sample.mbox >last &&
 	last=`cat last` &&
 	echo total is $last &&
-	test `cat last` = 11'
+	test `cat last` = 12'
 
 for mail in `echo 00*`
 do
diff --git a/t/t5100/info0012 b/t/t5100/info0012
new file mode 100644
index 0000000..ac1216f
--- /dev/null
+++ b/t/t5100/info0012
@@ -0,0 +1,5 @@
+Author: Dmitriy Blinov
+Email: bda@mnsspb.ru
+Subject: �������������� ������������ �������������� ���������������������� ������ ������������
+Date: Wed, 12 Nov 2008 17:54:41 +0300
+
diff --git a/t/t5100/msg0012 b/t/t5100/msg0012
new file mode 100644
index 0000000..1dc2bf7
--- /dev/null
+++ b/t/t5100/msg0012
@@ -0,0 +1,7 @@
+textlive-* �������������������� ���� texlive-*
+docutils �������������� ���� python-docutils
+
+��������������������������, ������������������, ������ rest2web �������������������� ���� ����������
+python-docutils. �� ���� ���������� ������ ������ rest2web ���� ����������.
+
+Signed-off-by: Dmitriy Blinov <bda@mnsspb.ru>
diff --git a/t/t5100/patch0012 b/t/t5100/patch0012
new file mode 100644
index 0000000..36a0b68
--- /dev/null
+++ b/t/t5100/patch0012
@@ -0,0 +1,30 @@
+---
+ howto/build_navy.txt |    6 +++---
+ 1 files changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/howto/build_navy.txt b/howto/build_navy.txt
+index 3fd3afb..0ee807e 100644
+--- a/howto/build_navy.txt
++++ b/howto/build_navy.txt
+@@ -119,8 +119,8 @@
+    - libxv-dev
+    - libusplash-dev
+    - latex-make
+-   - textlive-lang-cyrillic
+-   - textlive-latex-extra
++   - texlive-lang-cyrillic
++   - texlive-latex-extra
+    - dia
+    - python-pyrex
+    - libtool
+@@ -128,7 +128,7 @@
+    - sox
+    - cython
+    - imagemagick
+-   - docutils
++   - python-docutils
+ 
+ #. ���� ������������ dinar: ���������������� �������� ���������������� ssh-�������� �� authorized_keys2 ������������������������ ddev
+ #. ���� ���������� ������������: ������������������������������ /etc/sudoers (�������������� ``visudo``) ���������������� ������������������ ��������������::
+-- 
+1.5.6.5
diff --git a/t/t5100/sample.mbox b/t/t5100/sample.mbox
index 4bf7947..94da4da 100644
--- a/t/t5100/sample.mbox
+++ b/t/t5100/sample.mbox
@@ -501,3 +501,55 @@ index 3e5fe51..aabfe5c 100644
 
 --=-=-=--
 
+From bda@mnsspb.ru Wed Nov 12 17:54:41 2008
+From: Dmitriy Blinov <bda@mnsspb.ru>
+To: navy-patches@dinar.mns.mnsspb.ru
+Date: Wed, 12 Nov 2008 17:54:41 +0300
+Message-Id: <1226501681-24923-1-git-send-email-bda@mnsspb.ru>
+X-Mailer: git-send-email 1.5.6.5
+MIME-Version: 1.0
+Content-Type: text/plain;
+  charset=utf-8
+Content-Transfer-Encoding: 8bit
+Subject: [Navy-patches] [PATCH]
+	=?utf-8?b?0JjQt9C80LXQvdGR0L0g0YHQv9C40YHQvtC6INC/0LA=?=
+	=?utf-8?b?0LrQtdGC0L7QsiDQvdC10L7QsdGF0L7QtNC40LzRi9GFINC00LvRjyA=?=
+	=?utf-8?b?0YHQsdC+0YDQutC4?=
+
+textlive-* �������������������� ���� texlive-*
+docutils �������������� ���� python-docutils
+
+��������������������������, ������������������, ������ rest2web �������������������� ���� ����������
+python-docutils. �� ���� ���������� ������ ������ rest2web ���� ����������.
+
+Signed-off-by: Dmitriy Blinov <bda@mnsspb.ru>
+---
+ howto/build_navy.txt |    6 +++---
+ 1 files changed, 3 insertions(+), 3 deletions(-)
+
+diff --git a/howto/build_navy.txt b/howto/build_navy.txt
+index 3fd3afb..0ee807e 100644
+--- a/howto/build_navy.txt
++++ b/howto/build_navy.txt
+@@ -119,8 +119,8 @@
+    - libxv-dev
+    - libusplash-dev
+    - latex-make
+-   - textlive-lang-cyrillic
+-   - textlive-latex-extra
++   - texlive-lang-cyrillic
++   - texlive-latex-extra
+    - dia
+    - python-pyrex
+    - libtool
+@@ -128,7 +128,7 @@
+    - sox
+    - cython
+    - imagemagick
+-   - docutils
++   - python-docutils
+ 
+ #. ���� ������������ dinar: ���������������� �������� ���������������� ssh-�������� �� authorized_keys2 ������������������������ ddev
+ #. ���� ���������� ������������: ������������������������������ /etc/sudoers (�������������� ``visudo``) ���������������� ������������������ ��������������::
+-- 
+1.5.6.5
-- 
tg: (2292ebd..) t/mailinfo-multiline-subject (depends on: tmp)

Re: [BUG PATCH RFC] mailinfo: correctly handle multiline 'Subject:' header

From: Kirill Smelkov <hidden>
Date: 2016-06-15 22:45:53

On Fri, Dec 26, 2008 at 09:38:41PM +0300, Kirill Smelkov wrote:
When native language (RU) is in use, subject header usually contains several
parts, e.g.

Subject: [Navy-patches] [PATCH]
	=?utf-8?b?0JjQt9C80LXQvdGR0L0g0YHQv9C40YHQvtC6INC/0LA=?=
	=?utf-8?b?0LrQtdGC0L7QsiDQvdC10L7QsdGF0L7QtNC40LzRi9GFINC00LvRjyA=?=
	=?utf-8?b?0YHQsdC+0YDQutC4?=
Which btw should be extracted by git-mailinfo to:

    'Subject: Изменён список пакетов необходимых для сборки'
This exposes several bugs in builtin-mailinfo.c that I try to fix:


1. decode_b_segment: do not append explicit NUL -- explicit NUL was preventing
   correct header construction on parts concatenation via strbuf_addbuf in
   decode_header_bq. Fixes:

-Subject: Изменён список пакетов необходимых для сборки
+Subject: Изменён список па


Then

2. (hackish) do not emit '\n' after processing of every header segment. It
   seems we should emit previous part as-is only if it does not end with
   '=?='. Fixes:

-Subject: Изменён список пакетов необходимых для сборки
+Subject: Изменён список па кетов необходимых для сборки


Sorry for low-quality patch and description. I did what I could and don't have
energy and time dig more into MIME.

Please help.

Signed-off-by: Kirill Smelkov <redacted>

---
 builtin-mailinfo.c  |   18 ++++++++++++++++-
 t/t5100-mailinfo.sh |    2 +-
 t/t5100/info0012    |    5 ++++
 t/t5100/msg0012     |    7 ++++++
 t/t5100/patch0012   |   30 +++++++++++++++++++++++++++++
 t/t5100/sample.mbox |   52 +++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 112 insertions(+), 2 deletions(-)
Junio, All,

What about this patch?

It at least exposes bug in git-mailinfo wrt handling of multiline
subjects, and in very details documents it and adds a test for it.


Yes, my fixes are of 'low quality', but may I try to attract git
community attention one more time?


Thanks beforehand,
Kirill


P.S. original post with patch:

http://marc.info/?l=git&m=123031899307286&w=2

Re: [BUG PATCH RFC] mailinfo: correctly handle multiline 'Subject:' header

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:53

Kirill Smelkov [off-list ref] writes:
On Fri, Dec 26, 2008 at 09:38:41PM +0300, Kirill Smelkov wrote:
quoted
When native language (RU) is in use, subject header usually contains several
parts, e.g.
...
Junio, All,

What about this patch?
What's most interesting is that I do not recall seeing this patch before.
Neither gmane (which is my back-up interface to the mailing list) nor my
mailbox seems to have a copy, and from the look of quoted parts (namely,
some Russian strings in the message), it is not implausible that my spam
filter (either on my receiving end or at the ISP) may have eaten it.
It at least exposes bug in git-mailinfo wrt handling of multiline
subjects, and in very details documents it and adds a test for it.

..., but may I try to attract git
community attention one more time?
It is very appreciated.
P.S. original post with patch:

http://marc.info/?l=git&m=123031899307286&w=2
I have not had chance to look at your patch at marc yet, but from the look
of your problem description, I presume you could trigger this with any
utf-8 b-encoded loooooong subject line?

Re: [PATCH RFC] mailinfo: correctly handle multiline 'Subject:' header

From: Alexander Potashev <hidden>
Date: 2016-06-15 22:45:53

On 21:38 Fri 26 Dec     , Kirill Smelkov wrote:
When native language (RU) is in use, subject header usually contains several
parts, e.g.

Subject: [Navy-patches] [PATCH]
	=?utf-8?b?0JjQt9C80LXQvdGR0L0g0YHQv9C40YHQvtC6INC/0LA=?=
	=?utf-8?b?0LrQtdGC0L7QsiDQvdC10L7QsdGF0L7QtNC40LzRi9GFINC00LvRjyA=?=
	=?utf-8?b?0YHQsdC+0YDQutC4?=
 t/t5100/info0012    |    5 ++++
 t/t5100/msg0012     |    7 ++++++
 t/t5100/patch0012   |   30 +++++++++++++++++++++++++++++
 t/t5100/sample.mbox |   52 +++++++++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 112 insertions(+), 2 deletions(-)
The testcases are too long, a minimal mbox with encoded "Subject:" would
be enough to test the mailinfo parser, it's all the you need to test
here.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help