From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NdN30-0006U5-My for qemu-devel@nongnu.org; Fri, 05 Feb 2010 07:12:30 -0500 Received: from [199.232.76.173] (port=53342 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NdN30-0006Tx-9A for qemu-devel@nongnu.org; Fri, 05 Feb 2010 07:12:30 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NdN2y-0002kH-TQ for qemu-devel@nongnu.org; Fri, 05 Feb 2010 07:12:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:51613) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NdN2y-0002kB-FC for qemu-devel@nongnu.org; Fri, 05 Feb 2010 07:12:28 -0500 Date: Fri, 5 Feb 2010 10:12:17 -0200 From: Luiz Capitulino Subject: Re: [Qemu-devel] [PATCH 1/4] qjson: Improve debugging Message-ID: <20100205101217.7ec97899@doriath> In-Reply-To: <4B6B4AD2.3060704@codemonkey.ws> References: <1265314396-6583-1-git-send-email-lcapitulino@redhat.com> <1265314396-6583-2-git-send-email-lcapitulino@redhat.com> <4B6B4AD2.3060704@codemonkey.ws> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: qemu-devel@nongnu.org On Thu, 04 Feb 2010 16:31:46 -0600 Anthony Liguori wrote: > On 02/04/2010 02:13 PM, Luiz Capitulino wrote: > > Add an assert() to qobject_from_jsonf() to assure that the returned > > QObject is not NULL. Currently this is duplicated in the callers. > > > > Signed-off-by: Luiz Capitulino > > --- > > qjson.c | 1 + > > 1 files changed, 1 insertions(+), 0 deletions(-) > > > > diff --git a/qjson.c b/qjson.c > > index 9ad8a91..0922c06 100644 > > --- a/qjson.c > > +++ b/qjson.c > > @@ -62,6 +62,7 @@ QObject *qobject_from_jsonf(const char *string, ...) > > obj = qobject_from_jsonv(string,&ap); > > va_end(ap); > > > > + assert(obj != NULL); > > > > This is wrong. We may get JSON from an untrusted source. Callers need > to deal with failure appropriately. What kind of untrusted source? This function is only used by handlers and assuming that the only possible error here is bad syntax, not having this check in the source will only duplicate it in the users. > It just so happens that we only parse JSON from an untrusted source via > qobject_from_json(), but the trust relationship is not obvious given the > two functions in their current form. Not exactly, qobject_from_json() is not even being currently used. We parse JSON data from clients by using the low-level parser API, that's by calling json_message_parser_feed() to read the input and then calling json_parser_parse() when we have collected enough data. qobject_from_jsonf() is only used internally, by handlers. Both, qobject_from_jsonf() and qobject_from_json() are _wrappers_ to qobject_from_jsonv(), which uses the low-level API directly. So, having the assert() in qobject_from_jsonf() should only affect handlers, which seems fine to me.