* Segfault with weird Message-ID @ 2009-11-20 13:26 Mike Hommey 2009-11-20 17:04 ` Carl Worth 2009-11-20 20:53 ` Carl Worth 0 siblings, 2 replies; 6+ messages in thread From: Mike Hommey @ 2009-11-20 13:26 UTC (permalink / raw) To: notmuch Hi, I got a segfault when importing my maildir. It happened because of an old weird email, where the message-id is the following: Message-ID: <000022b17a1f$00004fbe$00000550@myrop (ew6.southwind.net [216.53.98.70]) by onyx.southwind.net from homepage.com (114.230.197.216) by newmail.spectraweb.ch from default (m202.2-25.warwick.net [ 218.242.202.80]) by host.warwick.net (8.10.0.Beta10/8.10.0.Beta10) with SMTP id e9GKEKk19201> I have absolutely no idea how it got this value, but the mail being an archived 8 years old spam, I'm not exactly sure if anyone would still expect such message id to occur. Anyways, the stack dump is the following: #0 0x00007ffff6d1e598 in Xapian::Document::add_term(std::string const&, unsigned int) () from /usr/lib/libxapian.so.15 #1 0x000000000040f5ff in _notmuch_message_add_term (message=0x0, prefix_name=0x41ad7f "tag", value=0x4191b0 "inbox") at lib/message.cc:587 #2 0x000000000040f827 in notmuch_message_add_tag (message=0x0, tag=0x4191b0 "inbox") at lib/message.cc:668 #3 0x0000000000407bc8 in tag_inbox_and_unread (message=0x0) at notmuch-new.c:44 #4 0x0000000000407f63 in add_files_recursive (notmuch=0x62cc20, path=0x832e90 "/home/mh/Maildir/saved-messages/cur", st=0x7fffffffe000, state=0x7fffffffe240) at notmuch-new.c:185 #5 0x0000000000408036 in add_files_recursive (notmuch=0x62cc20, path=0x832de0 "/home/mh/Maildir/saved-messages", st=0x7fffffffe000, state=0x7fffffffe240) at notmuch-new.c:223 #6 0x0000000000408036 in add_files_recursive (notmuch=0x62cc20, path=0x62c920 "/home/mh/Maildir", st=0x7fffffffe000, state=0x7fffffffe240) at notmuch-new.c:223 #7 0x0000000000408245 in add_files (notmuch=0x62cc20, path=0x62c920 "/home/mh/Maildir", state=0x7fffffffe240) at notmuch-new.c:287 #8 0x0000000000408704 in notmuch_new_command (ctx=0x61f140, argc=0, argv=0x7fffffffe3e8) at notmuch-new.c:431 #9 0x0000000000406ea8 in main (argc=2, argv=0x7fffffffe3d8) at notmuch.c:400 And the most likely problem is that message is NULL. Now, looking at the code, there seems to me there actually 3 problems: - _notmuch_message_create_for_message_id can return NULL, and while there is a test for it in notmuch_database_add_message, the function still returns a success code - things are still going on even when message is NULL in add_files_recursive - for some reason, xapian doesn't want to add the document corresponding to this old spam message: notmuch->xapian_db->add_document throws an exception. I can provide the spam if necessary, or can continue debugging the issue with some guidance. Cheers, Mike ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Segfault with weird Message-ID 2009-11-20 13:26 Segfault with weird Message-ID Mike Hommey @ 2009-11-20 17:04 ` Carl Worth 2009-11-20 20:53 ` Carl Worth 1 sibling, 0 replies; 6+ messages in thread From: Carl Worth @ 2009-11-20 17:04 UTC (permalink / raw) To: Mike Hommey, notmuch On Fri, 20 Nov 2009 14:26:25 +0100, Mike Hommey <mh+notmuch@glandium.org> wrote: > I can provide the spam if necessary, or can continue debugging the issue > with some guidance. It sounds strange to say it, but yes, please send me that spam! -Carl ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Segfault with weird Message-ID 2009-11-20 13:26 Segfault with weird Message-ID Mike Hommey 2009-11-20 17:04 ` Carl Worth @ 2009-11-20 20:53 ` Carl Worth 2009-11-20 21:05 ` Mike Hommey 1 sibling, 1 reply; 6+ messages in thread From: Carl Worth @ 2009-11-20 20:53 UTC (permalink / raw) To: Mike Hommey, notmuch [-- Attachment #1: Type: text/plain, Size: 3887 bytes --] On Fri, 20 Nov 2009 14:26:25 +0100, Mike Hommey <mh+notmuch@glandium.org> wrote: > I got a segfault when importing my maildir. It happened because of an > old weird email, where the message-id is the following: > Message-ID: <000022b17a1f$00004fbe$00000550@myrop (ew6.southwind.net [216.53.98.70]) by onyx.southwind.net from homepage.com (114.230.197.216) by newmail.spectraweb.ch from default (m202.2-25.warwick.net [ > 218.242.202.80]) by host.warwick.net (8.10.0.Beta10/8.10.0.Beta10) with SMTP id e9GKEKk19201> Thanks for sharing this Mike, (and for sending me the original file). > Anyways, the stack dump is the following: > #0 0x00007ffff6d1e598 in Xapian::Document::add_term(std::string const&, unsigned int) () from /usr/lib/libxapian.so.15 > #1 0x000000000040f5ff in _notmuch_message_add_term (message=0x0, prefix_name=0x41ad7f "tag", value=0x4191b0 "inbox") at lib/message.cc:587 > #2 0x000000000040f827 in notmuch_message_add_tag (message=0x0, tag=0x4191b0 "inbox") at lib/message.cc:668 > #3 0x0000000000407bc8 in tag_inbox_and_unread (message=0x0) at notmuch-new.c:44 > #4 0x0000000000407f63 in add_files_recursive (notmuch=0x62cc20, path=0x832e90 "/home/mh/Maildir/saved-messages/cur", st=0x7fffffffe000, state=0x7fffffffe240) at notmuch-new.c:185 > #5 0x0000000000408036 in add_files_recursive (notmuch=0x62cc20, path=0x832de0 "/home/mh/Maildir/saved-messages", st=0x7fffffffe000, state=0x7fffffffe240) at notmuch-new.c:223 > #6 0x0000000000408036 in add_files_recursive (notmuch=0x62cc20, path=0x62c920 "/home/mh/Maildir", st=0x7fffffffe000, state=0x7fffffffe240) at notmuch-new.c:223 > #7 0x0000000000408245 in add_files (notmuch=0x62cc20, path=0x62c920 "/home/mh/Maildir", state=0x7fffffffe240) at notmuch-new.c:287 > #8 0x0000000000408704 in notmuch_new_command (ctx=0x61f140, argc=0, argv=0x7fffffffe3e8) at notmuch-new.c:431 > #9 0x0000000000406ea8 in main (argc=2, argv=0x7fffffffe3d8) at notmuch.c:400 I didn't get the same crash when importing the file. But I did get a short document out of it (just a handful of terms indexed) and most significantly, an empty message-ID term. Xapian has a limit on the maximum length of a term, so one thing we'll need to do here is to notice if the message ID exceeds that length and then treat it as a we treat a missing Message-ID header, (that is, generate our own message ID by computing a sha-1 hash over the message). So, there was an obvious bug in the message-ID handling, (the code was still looking for NULL for a missing header, but we now return "" for a missing header instead). I've fixed this. > Now, looking at the code, there seems to me there actually 3 problems: > - _notmuch_message_create_for_message_id can return NULL, and while > there is a test for it in notmuch_database_add_message, the function > still returns a success code Thanks. This is fixed now. > - things are still going on even when message is NULL in > add_files_recursive I didn't replicate this case, but it *should* be fixed now that notmuch_database_add_message is returning a non-success value. > - for some reason, xapian doesn't want to add the document corresponding > to this old spam message: notmuch->xapian_db->add_document throws an > exception. I think things had just gone wrong long before then. > I can provide the spam if necessary, or can continue debugging the issue > with some guidance. Thanks for providing it. It turns out that the giant Message-Id value wasn't causing the problem. Instead the message was corrupt by having a stray new line at the third line. (So GMime is seeing only the first two lines of headers). We *used* to have working code to detect this kind of file as "not an email" but again, this broke when we changed notmuch_message_get_header to return "" instead of NULL for missing headers. See patches below (just pushed now as well) for the fixes. -Carl [-- Attachment #2: handle-corrupt-mail-and-non-mail.patchset --] [-- Type: text/plain, Size: 4233 bytes --] From 52292c548512214fd3dd205edb4ca9cf7955f2b3 Mon Sep 17 00:00:00 2001 From: Carl Worth <cworth@cworth.org> Date: Fri, 20 Nov 2009 19:31:00 +0100 Subject: [PATCH 1/3] add_message: Properly handle missing Message-ID once again. There's been a fair amount of fallout from when we changed message_file_get_header from returning NULL to returning "" for missing headers. This is yet more fallout from that, (where we were accepting an empty message-ID rather than generating one like we want to). --- lib/database.cc | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 726c5a9..294247e 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -908,7 +908,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch, * is to find a message ID (or else create one ourselves). */ header = notmuch_message_file_get_header (message_file, "message-id"); - if (header) { + if (header && *header != '\0') { message_id = _parse_message_id (message_file, header, NULL); /* So the header value isn't RFC-compliant, but it's * better than no message-id at all. */ -- 1.6.5.2 From 656e4c413d84984dcc5fbd8016907ed03c343cb8 Mon Sep 17 00:00:00 2001 From: Carl Worth <cworth@cworth.org> Date: Fri, 20 Nov 2009 21:02:11 +0100 Subject: [PATCH 2/3] notmuch_database_add_message: Add missing error-value propagation. Thanks to Mike Hommey for doing the analysis that led to noticing that this was missing. --- lib/database.cc | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 294247e..58a350d 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -940,8 +940,11 @@ notmuch_database_add_message (notmuch_database_t *notmuch, talloc_free (message_id); - if (message == NULL) + if (message == NULL) { + ret = COERCE_STATUS (private_status, + "Unexpected status value from _notmuch_message_create_for_message_id"); goto DONE; + } /* Is this a newly created message object? */ if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) { -- 1.6.5.2 From 3ae12b1e286d1c0041a2e3957cb01daa2981dad9 Mon Sep 17 00:00:00 2001 From: Carl Worth <cworth@cworth.org> Date: Fri, 20 Nov 2009 21:46:37 +0100 Subject: [PATCH 3/3] add_message: Re-fix handling of non-mail files. More fallout from _get_header now returning "" for missing headers. The bug here is that we would no longer detect that a file is not an email message and give up on it like we should. And this time, I actually audited all callers to notmuch_message_get_header, so hopefully we're done fixing this bug over and over. --- lib/database.cc | 10 +++++----- lib/message.cc | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/database.cc b/lib/database.cc index 58a350d..207246c 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -323,7 +323,7 @@ _parse_message_id (void *ctx, const char *message_id, const char **next) const char *s, *end; char *result; - if (message_id == NULL) + if (message_id == NULL || *message_id == '\0') return NULL; s = message_id; @@ -391,7 +391,7 @@ parse_references (void *ctx, { char *ref; - if (refs == NULL) + if (refs == NULL || *refs == '\0') return; while (*refs) { @@ -896,9 +896,9 @@ notmuch_database_add_message (notmuch_database_t *notmuch, subject = notmuch_message_file_get_header (message_file, "subject"); to = notmuch_message_file_get_header (message_file, "to"); - if (from == NULL && - subject == NULL && - to == NULL) + if ((from == NULL || *from == '\0') && + (subject == NULL || *subject == '\0') && + (to == NULL || *to == '\0')) { ret = NOTMUCH_STATUS_FILE_NOT_EMAIL; goto DONE; diff --git a/lib/message.cc b/lib/message.cc index 41dddd0..e0b8a8e 100644 --- a/lib/message.cc +++ b/lib/message.cc @@ -491,7 +491,7 @@ _notmuch_message_set_date (notmuch_message_t *message, /* GMime really doesn't want to see a NULL date, so protect its * sensibilities. */ - if (date == NULL) + if (date == NULL || *date == '\0') time_value = 0; else time_value = g_mime_utils_header_decode_date (date, NULL); -- 1.6.5.2 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: Segfault with weird Message-ID 2009-11-20 20:53 ` Carl Worth @ 2009-11-20 21:05 ` Mike Hommey 2009-11-21 22:26 ` Mike Hommey 0 siblings, 1 reply; 6+ messages in thread From: Mike Hommey @ 2009-11-20 21:05 UTC (permalink / raw) To: Carl Worth; +Cc: notmuch On Fri, Nov 20, 2009 at 09:53:37PM +0100, Carl Worth wrote: > On Fri, 20 Nov 2009 14:26:25 +0100, Mike Hommey <mh+notmuch@glandium.org> wrote: > > - for some reason, xapian doesn't want to add the document corresponding > > to this old spam message: notmuch->xapian_db->add_document throws an > > exception. > > I think things had just gone wrong long before then. I *did* see it throwing an exception from there. The sad thing is that I can't reproduce the problem anymore :-/ > > I can provide the spam if necessary, or can continue debugging the issue > > with some guidance. > > Thanks for providing it. It turns out that the giant Message-Id value > wasn't causing the problem. Instead the message was corrupt by having a > stray new line at the third line. (So GMime is seeing only the first two > lines of headers). We *used* to have working code to detect this kind of > file as "not an email" but again, this broke when we changed > notmuch_message_get_header to return "" instead of NULL for missing > headers. Interestingly, when I first traced on what message the crash was happening, I did see notmuch having the message-id in the message_id variable. FWIW, that was using c05c3f1. I'll see if I can reproduce my segfault again when starting from scratch again, and will also give a try to your patches. Cheers, Mike ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Segfault with weird Message-ID 2009-11-20 21:05 ` Mike Hommey @ 2009-11-21 22:26 ` Mike Hommey 2009-11-22 3:12 ` Carl Worth 0 siblings, 1 reply; 6+ messages in thread From: Mike Hommey @ 2009-11-21 22:26 UTC (permalink / raw) To: Carl Worth; +Cc: notmuch On Fri, Nov 20, 2009 at 10:05:56PM +0100, Mike Hommey wrote: > On Fri, Nov 20, 2009 at 09:53:37PM +0100, Carl Worth wrote: > > On Fri, 20 Nov 2009 14:26:25 +0100, Mike Hommey <mh+notmuch@glandium.org> wrote: > > > - for some reason, xapian doesn't want to add the document corresponding > > > to this old spam message: notmuch->xapian_db->add_document throws an > > > exception. > > > > I think things had just gone wrong long before then. > > I *did* see it throwing an exception from there. The sad thing is that I > can't reproduce the problem anymore :-/ > > > > I can provide the spam if necessary, or can continue debugging the issue > > > with some guidance. > > > > Thanks for providing it. It turns out that the giant Message-Id value > > wasn't causing the problem. Instead the message was corrupt by having a > > stray new line at the third line. (So GMime is seeing only the first two > > lines of headers). We *used* to have working code to detect this kind of > > file as "not an email" but again, this broke when we changed > > notmuch_message_get_header to return "" instead of NULL for missing > > headers. > > Interestingly, when I first traced on what message the crash was > happening, I did see notmuch having the message-id in the message_id > variable. I just was able to reproduce after starting over. header isn't "", and message_id is correctly filled. I can also confirm the exception is thrown from notmuch->xapian_db->add_document. > FWIW, that was using c05c3f1. With 3ae12b1, I get the following output: Error: A Xapian exception occurred. Halting processing. But I confirm there is no crash, now. Cheers, Mike ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Segfault with weird Message-ID 2009-11-21 22:26 ` Mike Hommey @ 2009-11-22 3:12 ` Carl Worth 0 siblings, 0 replies; 6+ messages in thread From: Carl Worth @ 2009-11-22 3:12 UTC (permalink / raw) To: Mike Hommey; +Cc: notmuch On Sat, 21 Nov 2009 23:26:15 +0100, Mike Hommey <mh+notmuch@glandium.org> wrote: > I just was able to reproduce after starting over. Thanks Mike. I was able to reproduce this as well by eliminating the spurious blank line I had on the 2nd or 3rd line. (So maybe that managed to sneak in when you sent me the message.) > header isn't "", and message_id is correctly filled. I can also confirm > the exception is thrown from notmuch->xapian_db->add_document. Yes. We were trying to add a term that is too long for Xapian. I've fixed this by simply falling back to our existing sha-1 code when a message ID is long. Thanks so much for the bug report! -Carl commit 5d56e931b99d575dbb0b936d24aae5e9903861ad Author: Carl Worth <cworth@cworth.org> Date: Sun Nov 22 04:03:49 2009 +0100 add_message: Use sha-1 in place of overly long message ID. Since Xapian has a limit on the maximum length of a term, we have to check for that before trying to add the message ID as a term. This fixes the bug reported by Mike Hommey here: <20091120132625.GA19246@glandium.org> I've also constructed 20 files with a range of message ID lengths centered around the Xapian term-length limit which I'll use to seed a new test suite soon. diff --git a/lib/database.cc b/lib/database.cc index 169dc5e..f4a445a 100644 --- a/lib/database.cc +++ b/lib/database.cc @@ -892,7 +892,7 @@ notmuch_database_add_message (notmuch_database_t *notmuch, const char *date, *header; const char *from, *to, *subject; - char *message_id; + char *message_id = NULL; if (message_ret) *message_ret = NULL; @@ -937,11 +937,20 @@ notmuch_database_add_message (notmuch_database_t *notmuch, header = notmuch_message_file_get_header (message_file, "message-id"); if (header && *header != '\0') { message_id = _parse_message_id (message_file, header, NULL); + /* So the header value isn't RFC-compliant, but it's * better than no message-id at all. */ if (message_id == NULL) message_id = talloc_strdup (message_file, header); - } else { + + /* Reject a Message ID that's too long. */ + if (message_id && strlen (message_id) + 1 > NOTMUCH_TERM_MAX) { + talloc_free (message_id); + message_id = NULL; + } + } + + if (message_id == NULL ) { /* No message-id at all, let's generate one by taking a * hash over the file's contents. */ char *sha1 = notmuch_sha1_of_file (filename); ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-11-22 3:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-11-20 13:26 Segfault with weird Message-ID Mike Hommey 2009-11-20 17:04 ` Carl Worth 2009-11-20 20:53 ` Carl Worth 2009-11-20 21:05 ` Mike Hommey 2009-11-21 22:26 ` Mike Hommey 2009-11-22 3:12 ` Carl Worth
Code repositories for project(s) associated with this public inbox https://yhetil.org/notmuch.git/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).