Date: Tue, 7 Aug 2012 08:49:15 -0400
From: Austin Clements <amdragon@MIT.EDU>
To: Mark Walters <markwalters1009@gmail.com>
Subject: Re: Segmentation fault in notmuch search --format=json
Message-ID: <20120807124857.GN22601@mit.edu>
References: <871ujjuu2z.fsf@gmail.com>
 <878vdrp4d9.fsf@qmul.ac.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <878vdrp4d9.fsf@qmul.ac.uk>
User-Agent: Mutt/1.5.21 (2010-09-15)
Cc: notmuch@notmuchmail.org
Precedence: list

Quoth Mark Walters on Aug 07 at  8:07 am:
> On Tue, 07 Aug 2012, Ben Gamari <bgamari.foss@gmail.com> wrote:
> > It seems some messages trigger a segmentation fault in
> > `do_search_threads()`. It appears the problem occurs (at least) when
> > `authors` is NULL.
> 
> Hi thanks for the bug report and detailed debugging. I think I can see
> the problem and there is a test patch to fix it below, and this does
> appear to be a regression.
> 
> In json.c the function json_quote_str explicitly checks/allows for a
> NULL pointer passed as a string and pretends it is just an empty
> string. That behaviour was lost in the move to structured formatters.
> 
> A simple fix is to put this check for a null pointer in json_string in
> sprinter-json.c which is what this patch does.
> 
> Incidentally this is the second time this bug has appeared: 
> 
>         commit cacefbf3d6dd5bce0b60b3cdfce29bfa371dfaea
>         Author: David Edmondson <dme@dme.org>
>         Date:   Tue Apr 6 08:24:00 2010 +0100
> 
>             json: Avoid calling strlen(NULL)
>                 
>                     MIME parts may have no filename, which previously resulted in calling
>                         strlen(NULL).
> 
> so it really might be worth having a test for it!
> 
> Finally, I think nothing in json.c is used anymore so perhaps it
>  could be removed.

LGTM.  We'll want to do something similar for text_string and, of
course, update the sprinter doc comments.

> diff --git a/sprinter-json.c b/sprinter-json.c
> index c9b6835..0a07790 100644
> --- a/sprinter-json.c
> +++ b/sprinter-json.c
> @@ -118,6 +118,8 @@ json_string_len (struct sprinter *sp, const char *val, size_t len)
>  static void
>  json_string (struct sprinter *sp, const char *val)
>  {
> +    if (val == NULL)
> +	val = "";
>      json_string_len (sp, val, strlen (val));
>  }
>