From: Jameson Graef Rollins <jrollins@finestructure.net>
To: Mark Walters <markwalters1009@gmail.com>,
	Ben Gamari <bgamari.foss@gmail.com>, notmuch@notmuchmail.org
Subject: Re: Segmentation fault in notmuch search --format=json
In-Reply-To: <878vdrp4d9.fsf@qmul.ac.uk>
References: <871ujjuu2z.fsf@gmail.com> <878vdrp4d9.fsf@qmul.ac.uk>
User-Agent: Notmuch/0.13.2+136~g6c0183b (http://notmuchmail.org) Emacs/23.4.1
	(x86_64-pc-linux-gnu)
Date: Tue, 07 Aug 2012 01:08:40 -0700
Message-ID: <87pq73jf92.fsf@servo.finestructure.net>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="=-=-=";
	micalg=pgp-sha256; protocol="application/pgp-signature"
Precedence: list

--=-=-=
Content-Transfer-Encoding: quoted-printable

On Tue, Aug 07 2012, Mark Walters <markwalters1009@gmail.com> wrote:
> 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.

Thanks Mark!  I was experiencing the same problem and this fixed the
issue before I even got a chance to respond.  This seems like a fine
solution.

> Incidentally this is the second time this bug has appeared:=20
>
>         commit cacefbf3d6dd5bce0b60b3cdfce29bfa371dfaea
>         Author: David Edmondson <dme@dme.org>
>         Date:   Tue Apr 6 08:24:00 2010 +0100
>
>             json: Avoid calling strlen(NULL)
>=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20=20
>                     MIME parts may have no filename, which previously res=
ulted in calling
>                         strlen(NULL).
>
> so it really might be worth having a test for it!

Indeed!  I think the problematic email in this case was one with no
subject.

> Finally, I think nothing in json.c is used anymore so perhaps it
>  could be removed.

Agreed.

jamie.

--=-=-=
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.12 (GNU/Linux)

iQIcBAEBCAAGBQJQIM0JAAoJEO00zqvie6q8C84P/0+M3A6/vj9qdDrQRaOOvFRR
p6vNJ+ebIZegrtS2DzFan7Idy9v+CNnTRt9hEJUO+xJcVZcD2LnwFo43DBeYyAhn
BTDR2IjvHDPKGg3J7uYe7DAjhZdfIJd5BYbXkVd3in3j6EQbQsXiiEua4wqyn3E8
gQB6zZXDEGY9F7Bj/v+PhpNVXNJOfS64QVIc2fWyMUg8Lrj12dYYpEnNS/21pj7S
GbCYFgtcdSk4ODqA3gLoVeZqJ/io//w33hS/P5decwQJkceSNBVppQ7KXImKQqlg
6iXyz9IjDtyBMqVaBuDutWwwJcYxa1+AdKpobyi0RgfcGfX2YKDjqHA1DPo142pz
fVNUviRv8Q2UxIxxKC4BvQGcftUmwlPG+6OlFV9lUL6NAgkf6tIg1vMo8OFm88Y1
ibbxBPe5hCOqq1K+ELUEV8E2VVFBlEBlVyfe9/piWO507bkCAAIubzCV12lX3KHe
otJ2VleJd3Y6/49E/6zV9AGLavEJXjaINtQyNVi7m4yf/MVWdgpwE4hgZxUqF5sJ
aXggn6ppJtGsTtKTC3/Nfu62TImzcEM9OHej5NEO/xU+rTBI6ZUKNNYXMBtv6aYY
IlST5W78DqcW9SJvGJWP+ZbcX8U027yShRy+s8v4uklxUNjcFFjeKvxl35HAoQ00
GYMob3scnwRD67mnAWUd
=DuIz
-----END PGP SIGNATURE-----
--=-=-=--