From 530c1e950a1bb46ff4e4a7c8e4b7cd945ff28916 Mon Sep 17 00:00:00 2001
From: Timo Sirainen <timo.sirainen@open-xchange.com>
Date: Wed, 18 Nov 2020 18:55:34 +0200
Subject: [PATCH] lib-imap: Fix writing BODYSTRUCTURE for truncated
 message/rfc822 part

If the max nesting limit is reached, write the last part out as
application/octet-stream instead of dummy message/rfc822.

Fixes error while parsing BODYSTRUCTURE:
message_part message/rfc822 flag doesn't match BODYSTRUCTURE
---
 src/lib-imap/imap-bodystructure.c      | 54 +++++++++----------
 src/lib-imap/test-imap-bodystructure.c | 73 ++++++++++++++++++++++++--
 2 files changed, 96 insertions(+), 31 deletions(-)

diff --git a/src/lib-imap/imap-bodystructure.c b/src/lib-imap/imap-bodystructure.c
index e3da1090b4..ab422c00d2 100644
--- a/src/lib-imap/imap-bodystructure.c
+++ b/src/lib-imap/imap-bodystructure.c
@@ -142,31 +142,42 @@ static void part_write_body_multipart(const struct message_part *part,
 	part_write_bodystructure_common(data, str);
 }
 
+static bool part_is_truncated(const struct message_part *part)
+{
+	const struct message_part_data *data = part->data;
+
+	i_assert((part->flags & MESSAGE_PART_FLAG_MESSAGE_RFC822) == 0);
+
+	if (data->content_type != NULL) {
+		if (strcasecmp(data->content_type, "message") == 0 &&
+		    strcasecmp(data->content_subtype, "rfc822") == 0) {
+			/* It's message/rfc822, but without
+			   MESSAGE_PART_FLAG_MESSAGE_RFC822. */
+			return TRUE;
+		}
+	}
+	return FALSE;
+}
+
 static void part_write_body(const struct message_part *part,
 			    string_t *str, bool extended)
 {
 	const struct message_part_data *data = part->data;
-	bool text, message_rfc822;
+	bool text;
 
 	i_assert(part->data != NULL);
 
-	if ((part->flags & MESSAGE_PART_FLAG_MESSAGE_RFC822) != 0)
-		message_rfc822 = TRUE;
-	else if (data->content_type != NULL &&
-		 strcasecmp(data->content_type, "message") == 0 &&
-		 strcasecmp(data->content_subtype, "rfc822") == 0) {
-		/* It's message/rfc822, but without
-		   MESSAGE_PART_FLAG_MESSAGE_RFC822. That likely means maximum
-		   MIME part count was reached while parsing the mail. Write
-		   the missing child mail's ENVELOPE and BODY as empty dummy
-		   values. */
-		message_rfc822 = TRUE;
-	} else
-		message_rfc822 = FALSE;
-
-	if (message_rfc822) {
+	if ((part->flags & MESSAGE_PART_FLAG_MESSAGE_RFC822) != 0) {
 		str_append(str, "\"message\" \"rfc822\"");
 		text = FALSE;
+	} else if (part_is_truncated(part)) {
+		/* Maximum MIME part count was reached while parsing the mail.
+		   Write this part out as application/octet-stream instead.
+		   We're not using text/plain, because it would require
+		   message-parser to use MESSAGE_PART_FLAG_TEXT for this part
+		   to avoid losing line count in message_part serialization. */
+		str_append(str, "\"application\" \"octet-stream\"");
+		text = FALSE;
 	} else {
 		/* "content type" "subtype" */
 		if (data->content_type == NULL) {
@@ -214,17 +225,6 @@ static void part_write_body(const struct message_part *part,
 
 		part_write_bodystructure_siblings(part->children, str, extended);
 		str_printfa(str, " %u", part->body_size.lines);
-	} else if (message_rfc822) {
-		/* truncated MIME part - write out dummy values */
-		i_assert(part->children == NULL);
-
-		str_append(str, " (NIL NIL NIL NIL NIL NIL NIL NIL NIL NIL) ");
-
-		if (!extended)
-			str_append(str, EMPTY_BODY);
-		else
-			str_append(str, EMPTY_BODYSTRUCTURE);
-		str_printfa(str, " %u", part->body_size.lines);
 	}
 
 	if (!extended)
diff --git a/src/lib-imap/test-imap-bodystructure.c b/src/lib-imap/test-imap-bodystructure.c
index dfc9957488..6cb699e126 100644
--- a/src/lib-imap/test-imap-bodystructure.c
+++ b/src/lib-imap/test-imap-bodystructure.c
@@ -4,6 +4,7 @@
 #include "istream.h"
 #include "str.h"
 #include "message-part-data.h"
+#include "message-part-serialize.h"
 #include "message-parser.h"
 #include "imap-bodystructure.h"
 #include "test-common.h"
@@ -379,12 +380,14 @@ struct normalize_test normalize_tests[] = {
 static const unsigned int normalize_tests_count = N_ELEMENTS(normalize_tests);
 
 static struct message_part *
-msg_parse(pool_t pool, const char *message, bool parse_bodystructure)
+msg_parse(pool_t pool, const char *message, unsigned int max_nested_mime_parts,
+	  bool parse_bodystructure)
 {
 	const struct message_parser_settings parser_set = {
 		.hdr_flags = MESSAGE_HEADER_PARSER_FLAG_SKIP_INITIAL_LWSP |
 			MESSAGE_HEADER_PARSER_FLAG_DROP_CR,
 		.flags = MESSAGE_PARSER_FLAG_SKIP_BODY_BLOCK,
+		.max_nested_mime_parts = max_nested_mime_parts,
 	};
 	struct message_parser_ctx *parser;
 	struct istream *input;
@@ -418,7 +421,7 @@ static void test_imap_bodystructure_write(void)
 		pool_t pool = pool_alloconly_create("imap bodystructure write", 1024);
 
 		test_begin(t_strdup_printf("imap bodystructure write [%u]", i));
-		parts = msg_parse(pool, test->message, TRUE);
+		parts = msg_parse(pool, test->message, 0, TRUE);
 
 		imap_bodystructure_write(parts, str, TRUE);
 		test_assert(strcmp(str_c(str), test->bodystructure) == 0);
@@ -445,7 +448,7 @@ static void test_imap_bodystructure_parse(void)
 		pool_t pool = pool_alloconly_create("imap bodystructure parse", 1024);
 
 		test_begin(t_strdup_printf("imap bodystructure parser [%u]", i));
-		parts = msg_parse(pool, test->message, FALSE);
+		parts = msg_parse(pool, test->message, 0, FALSE);
 
 		test_assert(imap_body_parse_from_bodystructure(test->bodystructure,
 								     str, &error) == 0);
@@ -512,7 +515,7 @@ static void test_imap_bodystructure_normalize(void)
 		pool_t pool = pool_alloconly_create("imap bodystructure parse", 1024);
 
 		test_begin(t_strdup_printf("imap bodystructure normalize [%u]", i));
-		parts = msg_parse(pool, test->message, FALSE);
+		parts = msg_parse(pool, test->message, 0, FALSE);
 
 		ret = imap_bodystructure_parse(test->input,
 							   pool, parts, &error);
@@ -531,6 +534,67 @@ static void test_imap_bodystructure_normalize(void)
 	} T_END;
 }
 
+static const struct {
+	const char *input;
+	const char *bodystructure;
+	unsigned int max_depth;
+} truncation_tests[] = {
+	{
+		.input = "Content-Type: message/rfc822\n"
+			"\n"
+			"Content-Type: message/rfc822\n"
+			"Header2: value2\n"
+			"\n"
+			"Subject: hello world\n"
+			"Header2: value2\n"
+			"Header3: value3\n"
+			"\n"
+			"body line 1\n"
+			"body line 2\n"
+			"body line 4\n"
+			"body line 3\n",
+		.bodystructure = "\"message\" \"rfc822\" NIL NIL NIL \"7bit\" 159 (NIL NIL NIL NIL NIL NIL NIL NIL NIL NIL) (\"application\" \"octet-stream\" NIL NIL NIL \"7bit\" 110 NIL NIL NIL NIL) 11 NIL NIL NIL NIL",
+		.max_depth = 2,
+	},
+};
+
+static void test_imap_bodystructure_truncation(void)
+{
+	struct message_part *parts;
+	const char *error;
+	string_t *str_body = t_str_new(128);
+	string_t *str_parts = t_str_new(128);
+	pool_t pool = pool_alloconly_create("imap bodystructure parse", 1024);
+
+	test_begin("imap bodystructure truncation");
+
+	for (unsigned int i = 0; i < N_ELEMENTS(truncation_tests); i++) {
+		p_clear(pool);
+		str_truncate(str_body, 0);
+		str_truncate(str_parts, 0);
+
+		parts = msg_parse(pool, truncation_tests[i].input,
+				  truncation_tests[i].max_depth,
+				  TRUE);
+
+		/* write out BODYSTRUCTURE and serialize message_parts */
+		imap_bodystructure_write(parts, str_body, TRUE);
+		message_part_serialize(parts, str_parts);
+
+		/* now deserialize message_parts and make sure they can be used
+		   to parse BODYSTRUCTURE */
+		parts = message_part_deserialize(pool, str_data(str_parts),
+						 str_len(str_parts), &error);
+		test_assert(parts != NULL);
+		test_assert(imap_bodystructure_parse(str_c(str_body), pool,
+						     parts, &error) == 0);
+		test_assert_strcmp(str_c(str_body),
+				   truncation_tests[i].bodystructure);
+	}
+	pool_unref(&pool);
+	test_end();
+}
+
 int main(void)
 {
 	static void (*const test_functions[])(void) = {
@@ -538,6 +602,7 @@ int main(void)
 		test_imap_bodystructure_parse,
 		test_imap_bodystructure_normalize,
 		test_imap_bodystructure_parse_full,
+		test_imap_bodystructure_truncation,
 		NULL
 	};
 	return test_run(test_functions);