From c521b1ec014fd23169e727941aa1097f43bdcf81 Mon Sep 17 00:00:00 2001 From: Michal Sojka Date: Mon, 26 May 2014 16:05:57 +0200 Subject: [PATCH] [PATCH 2/2] Make parsing of References and In-Reply-To header less error prone --- 16/36ef3e028388a499907e13eae0fa853dae0aa0 | 128 ++++++++++++++++++++++ 1 file changed, 128 insertions(+) create mode 100644 16/36ef3e028388a499907e13eae0fa853dae0aa0 diff --git a/16/36ef3e028388a499907e13eae0fa853dae0aa0 b/16/36ef3e028388a499907e13eae0fa853dae0aa0 new file mode 100644 index 000000000..95fc6c2bc --- /dev/null +++ b/16/36ef3e028388a499907e13eae0fa853dae0aa0 @@ -0,0 +1,128 @@ +Return-Path: +X-Original-To: notmuch@notmuchmail.org +Delivered-To: notmuch@notmuchmail.org +Received: from localhost (localhost [127.0.0.1]) + by olra.theworths.org (Postfix) with ESMTP id EB92E431FBD + for ; Mon, 26 May 2014 07:06:18 -0700 (PDT) +X-Virus-Scanned: Debian amavisd-new at olra.theworths.org +X-Spam-Flag: NO +X-Spam-Score: -2.3 +X-Spam-Level: +X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5 + tests=[RCVD_IN_DNSWL_MED=-2.3] autolearn=disabled +Received: from olra.theworths.org ([127.0.0.1]) + by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) + with ESMTP id qn2HFwlTKUBV for ; + Mon, 26 May 2014 07:06:10 -0700 (PDT) +Received: from max.feld.cvut.cz (max.feld.cvut.cz [147.32.192.36]) + by olra.theworths.org (Postfix) with ESMTP id 8B123431FAF + for ; Mon, 26 May 2014 07:06:10 -0700 (PDT) +Received: from localhost (unknown [192.168.200.7]) + by max.feld.cvut.cz (Postfix) with ESMTP id 191E319F3634 + for ; Mon, 26 May 2014 16:06:06 +0200 (CEST) +X-Virus-Scanned: IMAP STYX AMAVIS +Received: from max.feld.cvut.cz ([192.168.200.1]) + by localhost (styx.feld.cvut.cz [192.168.200.7]) (amavisd-new, + port 10044) with ESMTP id MGCC8KNkeYLG for ; + Mon, 26 May 2014 16:06:02 +0200 (CEST) +Received: from imap.feld.cvut.cz (imap.feld.cvut.cz [147.32.192.34]) + by max.feld.cvut.cz (Postfix) with ESMTP id F07CE19F362F + for ; Mon, 26 May 2014 16:06:01 +0200 (CEST) +Received: from wsh by steelpick.2x.cz with local (Exim 4.82) + (envelope-from ) + id 1WovXM-0008SE-1z; Mon, 26 May 2014 16:06:00 +0200 +From: Michal Sojka +To: notmuch@notmuchmail.org +Subject: [PATCH 2/2] Make parsing of References and In-Reply-To header less + error prone +Date: Mon, 26 May 2014 16:05:57 +0200 +Message-Id: <1401113157-32454-3-git-send-email-sojkam1@fel.cvut.cz> +X-Mailer: git-send-email 2.0.0.rc2 +In-Reply-To: <1401113157-32454-1-git-send-email-sojkam1@fel.cvut.cz> +References: <1401113157-32454-1-git-send-email-sojkam1@fel.cvut.cz> +X-BeenThere: notmuch@notmuchmail.org +X-Mailman-Version: 2.1.13 +Precedence: list +List-Id: "Use and development of the notmuch mail system." + +List-Unsubscribe: , + +List-Archive: +List-Post: +List-Help: +List-Subscribe: , + +X-List-Received-Date: Mon, 26 May 2014 14:06:19 -0000 + +According to RFC2822 References and In-Reply-To headers are supposed +to contain one or more Message-IDs, however older RFC822 allowed +almost any content. When both References and In-Reply-To headers ends +with something else that a Message-ID (see e.g. [1]), the thread +structure presented by notmuch is incorrect. The reason is that +notmuch treats this case as if the email contained no "replyto" +information (see _notmuch_database_link_message_to_parents). + +This patch changes the parse_references() function to return the last +valid Message-ID encountered rather than NULL resulting from the last +hunk of text not being the Message-ID. + +[1] https://lkml.org/lkml/headers/2014/5/19/864 +--- + lib/database.cc | 15 ++++++--------- + test/T510-thread-replies.sh | 1 - + 2 files changed, 6 insertions(+), 10 deletions(-) + +diff --git a/lib/database.cc b/lib/database.cc +index 1efb14d..26f15fd 100644 +--- a/lib/database.cc ++++ b/lib/database.cc +@@ -521,7 +521,7 @@ parse_references (void *ctx, + GHashTable *hash, + const char *refs) + { +- char *ref; ++ char *ref, *last_ref = NULL; + + if (refs == NULL || *refs == '\0') + return NULL; +@@ -529,20 +529,17 @@ parse_references (void *ctx, + while (*refs) { + ref = _parse_message_id (ctx, refs, &refs); + +- if (ref && strcmp (ref, message_id)) ++ if (ref && strcmp (ref, message_id)) { + g_hash_table_insert (hash, ref, NULL); ++ last_ref = ref; ++ } + } + + /* The return value of this function is used to add a parent + * reference to the database. We should avoid making a message +- * its own parent, thus the following check. ++ * its own parent, thus the above check. + */ +- +- if (ref && strcmp(ref, message_id)) { +- return ref; +- } else { +- return NULL; +- } ++ return last_ref; + } + + notmuch_status_t +diff --git a/test/T510-thread-replies.sh b/test/T510-thread-replies.sh +index d818b89..1392fbe 100755 +--- a/test/T510-thread-replies.sh ++++ b/test/T510-thread-replies.sh +@@ -138,7 +138,6 @@ expected=`echo "$expected" | notmuch_json_show_sanitize` + test_expect_equal_json "$output" "$expected" + + test_begin_subtest "Ignore garbage at the end of References" +-test_subtest_known_broken + add_message '[id]="foo@five.com"' \ + '[subject]="five"' + add_message '[id]="bar@five.com"' \ +-- +2.0.0.rc2 + -- 2.26.2