Re: Segfault searching for tags

[Carl Worth <cworth@cworth.org>]

On Thu, 19 Nov 2009 16:45:43 +0100, Adrian Perez de Castro <aperez@igalia.com> wrote:
> The thing is that in notmuch_message_get_in_reply_to(), line 288, a NULL
> instance of Xapian::TermIterator is dereferenced. In my particular case,
> the culpript is a cache file of Claws-Mail, as seen in the following GDB
> session:

Not quite NULL, (nor is it quite dereferencing---this is nasty C++
overloading), but yeah, the idea is the same. We need to protect all of
our "calls" to this overloaded operator to not call it when the iterator
is equal to the value returned by termlist_end ().

On Thu, 19 Nov 2009 20:23:15 -0600, Jeffrey Ollie <jeff@ocjtech.us> wrote:
> I straced some of the crashes, and the last file that was read before
> the crash was a malformed message.  I've attached one of the messages.
>  I've been using offlineimap to sync my gmail mailbox to my laptop so
> that I can use notmuch.  offlineimap isn't the most stable program,
> but I'm not sure yet if offlineimap is causing the problem or if
> that's the way the message is in gmail.

Thanks for the file. I never like to push code that I haven't tested, so
this was very helpful.

Below is the patch that I just pushed which seems to do the trick.

-Carl

commit 31b54bc78735c628035a046e526ac4c596d830cf
Author: Carl Worth <cworth@cworth.org>
Date:   Fri Nov 20 12:06:11 2009 +0100

    Avoid access of a Xapian iterator's object when there's nothing
    there.
    
    This eliminates a crash when a message (either corrupted or a
    non-mail
    file that wasn't properly detected as not being mail) has no
    In-Reply-To
    header, (and so few terms that trying to skip to the prefix of the
    In-Reply-To terms actually brings us to the end of the termlist).

diff --git a/lib/message.cc b/lib/message.cc
index 9488fb6..41dddd0 100644
--- a/lib/message.cc
+++ b/lib/message.cc
@@ -285,7 +285,8 @@ _notmuch_message_get_in_reply_to (notmuch_message_t
*message
     i = message->doc.termlist_begin ();
     i.skip_to (prefix);
 
-    in_reply_to = *i;
+    if (i != message->doc.termlist_end ())
+       in_reply_to = *i;
 
     /* It's perfectly valid for a message to have no In-Reply-To
      * header. For these cases, we return an empty string. */
@@ -332,10 +333,10 @@ notmuch_message_get_thread_id (notmuch_message_t
     *message)
        return message->thread_id;
 
     i = message->doc.termlist_begin ();
-
     i.skip_to (prefix);
 
-    id = *i;
+    if (i != message->doc.termlist_end ())
+       id = *i;
 
     if (i == message->doc.termlist_end () || id[0] != *prefix)
        INTERNAL_ERROR ("Message with document ID of %d has no thread
        ID.\n",
@@ -466,7 +467,7 @@ notmuch_message_get_tags (notmuch_message_t
        *message)
 
     i.skip_to (prefix);
 
-    while (1) {
+    while (i != end) {
        tag = *i;
 
        if (tag.empty () || tag[0] != *prefix)