[PATCH] json: Avoid calling strlen(NULL)

[PATCH] json: Avoid calling strlen(NULL)

[David Edmondson]

commit b65817262b3a275ecd0ef1898d92ec5508a9f810
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).

	Modified json.c
diff --git a/json.c b/json.c
index f90b0fa..b73f22a 100644
--- a/json.c
+++ b/json.c
@@ -105,5 +105,8 @@ json_quote_chararray(const void *ctx, const char *str, const size_t len)
 char *
 json_quote_str(const void *ctx, const char *str)
 {
+    if (str == NULL)
+	return (char *)"\"\"";
+
     return (json_quote_chararray (ctx, str, strlen (str)));
 }


dme.
-- 
David Edmondson, http://dme.org

Re: [PATCH] json: Avoid calling strlen(NULL)

[Anthony Towns]

On Tue, Apr 6, 2010 at 17:25, David Edmondson <dme@dme.org> wrote:
>    json: Avoid calling strlen(NULL)
>    MIME parts may have no filename, which previously resulted in calling
>    strlen(NULL).
>  char *
>  json_quote_str(const void *ctx, const char *str)
>  {
> +    if (str == NULL)
> +       return (char *)"\"\"";
> +
>     return (json_quote_chararray (ctx, str, strlen (str)));
>  }

There's already a check in json_quote_chararray for len==0, so it
might be sensible to say:

    return (json_quote_chararray (ctx, str, str != NULL ? strlen (str) : 0));

OTOH, the code in json_quote_array to deal with that does the same
thing (returns a literal string containing two quote marks), which
seems wrong -- the normal code path is to talloc to get a newly
allocated, editable string, that might be talloc_free'd later,
wouldn't it make more sense just to let the str==NULL / len==0
behaviour fall through into the normal case code?

FWIW:

commit 5b93a488221b50c02db18d86a550cb3c038c00da
Author: Anthony <aj@erisian.com.au>
Date:   Tue Apr 6 18:10:39 2010 +1000

    json: Avoid calling strlen(NULL), and always return a newly talloced array.

    MIME parts may have a no filename, which causes json_quote_str()
to be invoked
    with NULL instead of a string.

diff --git a/json.c b/json.c
index f90b0fa..5e379ef 100644
--- a/json.c
+++ b/json.c
@@ -57,9 +57,6 @@ json_quote_chararray(const void *ctx, const char
*str, const size_t len)
     size_t loop;
     size_t required;

-    if (len == 0)
-	return (char *)"\"\"";
-
     for (loop = 0, required = 0, ptr = str;
 	 loop < len;
 	 loop++, required++, ptr++) {
@@ -105,5 +102,8 @@ json_quote_chararray(const void *ctx, const char
*str, const size_t len)
 char *
 json_quote_str(const void *ctx, const char *str)
 {
+    if (str == NULL)
+        str = "";
+
     return (json_quote_chararray (ctx, str, strlen (str)));
 }



-- 
Anthony Towns <aj@erisian.com.au>

Re: [PATCH] json: Avoid calling strlen(NULL)

[David Edmondson]

On Tue, 6 Apr 2010 18:17:44 +1000, Anthony Towns <aj@erisian.com.au> wrote:
> OTOH, the code in json_quote_array to deal with that does the same
> thing (returns a literal string containing two quote marks), which
> seems wrong -- the normal code path is to talloc to get a newly
> allocated, editable string, that might be talloc_free'd later,
> wouldn't it make more sense just to let the str==NULL / len==0
> behaviour fall through into the normal case code?

This is a much nicer solution.

dme.
-- 
David Edmondson, http://dme.org

Re: [PATCH] json: Avoid calling strlen(NULL)

[Carl Worth]

[-- Attachment #1: Type: text/plain, Size: 928 bytes --]

> There's already a check in json_quote_chararray for len==0, so it
> might be sensible to say:
> 
>     return (json_quote_chararray (ctx, str, str != NULL ? strlen (str) : 0));
> 
> OTOH, the code in json_quote_array to deal with that does the same
> thing (returns a literal string containing two quote marks), which
> seems wrong -- the normal code path is to talloc to get a newly
> allocated, editable string, that might be talloc_free'd later,
> wouldn't it make more sense just to let the str==NULL / len==0
> behaviour fall through into the normal case code?

Yes, that's the correct analysis. Thanks so much.

> commit 5b93a488221b50c02db18d86a550cb3c038c00da
> Author: Anthony <aj@erisian.com.au>
> Date:   Tue Apr 6 18:10:39 2010 +1000
> 
>     json: Avoid calling strlen(NULL), and always return a newly
>     talloced array.


I've pushed this out now, (separated into two pieces).

-Carl

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]