Improve test coverage.

* Test equality of different length strings.
* Add tab to json_pack whitespace test.
* Test json_sprintf with empty result and invalid UTF.
* Test json_get_alloc_funcs with NULL arguments.
* Test invalid arguments.
* Add test_chaos to test allocation failure code paths.
* Remove redundant json_is_string checks from json_string_equal and
  json_string_copy.  Both functions are static and can only be called
  with a json string.

Fixes to issues found by test_chaos:
* Fix crash on OOM in pack_unpack.c:read_string().
* Unconditionally free string in string_create upon allocation failure.
  Update load.c:parse_value() to reflect this.  This resolves a leak on
  allocation failure for pack_unpack.c:pack_string() and
  value.c:json_sprintf().

Although not visible from CodeCoverage these changes significantly
increase branch coverage.  Especially in src/value.c where we previously
covered 67.4% of branches and now cover 96.3% of branches.

CMakeLists.txt

@@ -487,6 +487,7 @@ if (NOT JANSSON_WITHOUT_TESTS)
    set(api_tests
          test_array
          test_copy
+         test_chaos
          test_dump
          test_dump_callback
          test_equal

src/load.c

@@ -829,10 +829,8 @@ static json_t *parse_value(lex_t *lex, size_t flags, json_error_t *error)
             }
 
             json = jsonp_stringn_nocheck_own(value, len);
-            if(json) {
             lex->value.string.val = NULL;
             lex->value.string.len = 0;
-            }
             break;
         }
 

src/pack_unpack.c

@@ -159,7 +159,10 @@ static char *read_string(scanner_t *s, va_list *ap,
         return (char *)str;
     }
 
-    strbuffer_init(&strbuff);
+    if(strbuffer_init(&strbuff)) {
+        set_error(s, "<internal>", json_error_out_of_memory, "Out of memory");
+        s->has_error = 1;
+    }
 
     while(1) {
         str = va_arg(*ap, const char *);

src/value.c

@@ -652,7 +652,6 @@ static json_t *string_create(const char *value, size_t len, int own)
 
     string = jsonp_malloc(sizeof(json_string_t));
     if(!string) {
-        if(!own)
         jsonp_free(v);
         return NULL;
     }
@@ -768,9 +767,6 @@ static int json_string_equal(const json_t *string1, const json_t *string2)
 {
     json_string_t *s1, *s2;
 
-    if(!json_is_string(string1) || !json_is_string(string2))
-        return 0;
-
     s1 = json_to_string(string1);
     s2 = json_to_string(string2);
     return s1->length == s2->length && !memcmp(s1->value, s2->value, s1->length);
@@ -780,9 +776,6 @@ static json_t *json_string_copy(const json_t *string)
 {
     json_string_t *s;
 
-    if(!json_is_string(string))
-        return NULL;
-
     s = json_to_string(string);
     return json_stringn_nocheck(s->value, s->length);
 }

test/.gitignore

@@ -1,6 +1,7 @@
 logs
 bin/json_process
 suites/api/test_array
+suites/api/test_chaos
 suites/api/test_copy
 suites/api/test_cpp
 suites/api/test_dump

test/suites/api/Makefile.am

@@ -2,6 +2,7 @@ EXTRA_DIST = run check-exports
 
 check_PROGRAMS = \
 	test_array \
+	test_chaos \
 	test_copy \
 	test_dump \
 	test_dump_callback \
@@ -18,6 +19,7 @@ check_PROGRAMS = \
 	test_unpack
 
 test_array_SOURCES = test_array.c util.h
+test_chaos_SOURCES = test_chaos.c util.h
 test_copy_SOURCES = test_copy.c util.h
 test_dump_SOURCES = test_dump.c util.h
 test_dump_callback_SOURCES = test_dump_callback.c util.h

test/suites/api/test_array.c

@@ -419,6 +419,78 @@ static void test_array_foreach()
     json_decref(array2);
 }
 
+static void test_bad_args(void)
+{
+    json_t *arr = json_array();
+    json_t *num = json_integer(1);
+
+    if(!arr || !num)
+        fail("failed to create required objects");
+
+    if(json_array_size(NULL) != 0)
+        fail("NULL array has nonzero size");
+    if(json_array_size(num) != 0)
+        fail("non-array has nonzero array size");
+
+    if(json_array_get(NULL, 0))
+        fail("json_array_get did not return NULL for non-array");
+    if(json_array_get(num, 0))
+        fail("json_array_get did not return NULL for non-array");
+
+    if(!json_array_set_new(NULL, 0, json_incref(num)))
+        fail("json_array_set_new did not return error for non-array");
+    if(!json_array_set_new(num, 0, json_incref(num)))
+        fail("json_array_set_new did not return error for non-array");
+    if(!json_array_set_new(arr, 0, NULL))
+        fail("json_array_set_new did not return error for NULL value");
+    if(!json_array_set_new(arr, 0, json_incref(arr)))
+        fail("json_array_set_new did not return error for value == array");
+
+    if(!json_array_remove(NULL, 0))
+        fail("json_array_remove did not return error for non-array");
+    if(!json_array_remove(num, 0))
+        fail("json_array_remove did not return error for non-array");
+
+    if(!json_array_clear(NULL))
+        fail("json_array_clear did not return error for non-array");
+    if(!json_array_clear(num))
+        fail("json_array_clear did not return error for non-array");
+
+    if(!json_array_append_new(NULL, json_incref(num)))
+        fail("json_array_append_new did not return error for non-array");
+    if(!json_array_append_new(num, json_incref(num)))
+        fail("json_array_append_new did not return error for non-array");
+    if(!json_array_append_new(arr, NULL))
+        fail("json_array_append_new did not return error for NULL value");
+    if(!json_array_append_new(arr, json_incref(arr)))
+        fail("json_array_append_new did not return error for value == array");
+
+    if(!json_array_insert_new(NULL, 0, json_incref(num)))
+        fail("json_array_insert_new did not return error for non-array");
+    if(!json_array_insert_new(num, 0, json_incref(num)))
+        fail("json_array_insert_new did not return error for non-array");
+    if(!json_array_insert_new(arr, 0, NULL))
+        fail("json_array_insert_new did not return error for NULL value");
+    if(!json_array_insert_new(arr, 0, json_incref(arr)))
+        fail("json_array_insert_new did not return error for value == array");
+
+    if(!json_array_extend(NULL, arr))
+        fail("json_array_extend did not return error for first argument non-array");
+    if(!json_array_extend(num, arr))
+        fail("json_array_extend did not return error for first argument non-array");
+    if(!json_array_extend(arr, NULL))
+        fail("json_array_extend did not return error for second arguemnt non-array");
+    if(!json_array_extend(arr, num))
+        fail("json_array_extend did not return error for second arguemnt non-array");
+
+    if(num->refcount != 1)
+        fail("unexpected reference count on num");
+    if(arr->refcount != 1)
+        fail("unexpected reference count on arr");
+
+    json_decref(num);
+    json_decref(arr);
+}
 
 static void run_tests()
 {
@@ -429,4 +501,5 @@ static void run_tests()
     test_extend();
     test_circular();
     test_array_foreach();
+    test_bad_args();
 }

test/suites/api/test_chaos.c

@@ -0,0 +1,115 @@
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
+
+#include <stdio.h>
+#include <string.h>
+#include <jansson.h>
+#include "util.h"
+
+static int chaos_pos = 0;
+static int chaos_fail = 0;
+#define CHAOS_MAX_FAILURE 100
+
+void *chaos_malloc(size_t size)
+{
+    if (chaos_pos == chaos_fail)
+        return NULL;
+
+    chaos_pos++;
+
+    return malloc(size);
+}
+
+void chaos_free(void *obj)
+{
+    free(obj);
+}
+
+/* Test all potential allocation failures. */
+#define chaos_loop(condition, code, cleanup) \
+    { \
+        chaos_pos = chaos_fail = 0; \
+        while (condition) { \
+            if (chaos_fail > CHAOS_MAX_FAILURE) \
+                fail("too many chaos failures"); \
+            code \
+            chaos_pos = 0; \
+            chaos_fail++; \
+        } \
+        cleanup \
+    }
+
+#define chaos_loop_new_value(json, initcall) \
+    chaos_loop(!json, json = initcall;, json_decref(json); json = NULL;)
+
+static void test_chaos()
+{
+    json_malloc_t orig_malloc;
+    json_free_t orig_free;
+    json_t *json = NULL;
+    json_t *obj = json_object();
+    json_t *arr1 = json_array();
+    json_t *arr2 = json_array();
+    json_t *txt = json_string("test");
+    json_t *intnum = json_integer(1);
+    json_t *dblnum = json_real(0.5);
+    int keyno;
+
+    if (!obj || !arr1 || !arr2 || !txt || !intnum || !dblnum)
+        fail("failed to allocate basic objects");
+
+    json_get_alloc_funcs(&orig_malloc, &orig_free);
+    json_set_alloc_funcs(chaos_malloc, chaos_free);
+
+    chaos_loop_new_value(json, json_pack("{s:s}", "key", "value"));
+    chaos_loop_new_value(json, json_pack("{s:[]}", "key"));
+    chaos_loop_new_value(json, json_pack("[biIf]", 1, 1, (json_int_t)1, 1.0));
+    chaos_loop_new_value(json, json_pack("[s*,s*]", "v1", "v2"));
+    chaos_loop_new_value(json, json_pack("o", json_incref(txt)));
+    chaos_loop_new_value(json, json_pack("O", txt));
+    chaos_loop_new_value(json, json_pack("s++", "a",
+        "long string to force realloc",
+        "another long string to force yet another reallocation of the string because "
+        "that's what we are testing."));
+
+    chaos_loop_new_value(json, json_copy(obj));
+    chaos_loop_new_value(json, json_deep_copy(obj));
+
+    chaos_loop_new_value(json, json_copy(arr1));
+    chaos_loop_new_value(json, json_deep_copy(arr1));
+
+    chaos_loop_new_value(json, json_copy(txt));
+    chaos_loop_new_value(json, json_copy(intnum));
+    chaos_loop_new_value(json, json_copy(dblnum));
+
+    chaos_loop_new_value(json, json_sprintf("%s", "string"));
+
+    for (keyno = 0; keyno < 100; ++keyno) {
+#if !defined(_MSC_VER) || _MSC_VER >= 1900
+        /* Skip this test on old Windows compilers. */
+        char testkey[10];
+
+        snprintf(testkey, sizeof(testkey), "test%d", keyno);
+        chaos_loop(json_object_set_new_nocheck(obj, testkey, json_object()),,);
+#endif
+        chaos_loop(json_array_append_new(arr1, json_null()),,);
+        chaos_loop(json_array_insert_new(arr2, 0, json_null()),,);
+    }
+
+    chaos_loop(json_array_extend(arr1, arr2),,);
+    chaos_loop(json_string_set_nocheck(txt, "test"),,);
+
+    json_set_alloc_funcs(orig_malloc, orig_free);
+    json_decref(obj);
+    json_decref(arr1);
+    json_decref(arr2);
+    json_decref(txt);
+    json_decref(intnum);
+    json_decref(dblnum);
+}
+
+static void run_tests()
+{
+    test_chaos();
+}

test/suites/api/test_equal.c

@@ -74,6 +74,13 @@ static void test_equal_simple()
         fail("unable to create an string");
     if(json_equal(value1, value2))
         fail("json_equal fails for two inequal strings");
+    json_decref(value2);
+
+    value2 = json_string("bar2");
+    if(!value2)
+        fail("unable to create an string");
+    if(json_equal(value1, value2))
+        fail("json_equal fails for two inequal length strings");
 
     json_decref(value1);
     json_decref(value2);

test/suites/api/test_memory_funcs.c

@@ -122,9 +122,16 @@ static void test_secure_funcs(void)
     create_and_free_complex_object();
 }
 
+static void test_bad_args(void)
+{
+    /* The result of this test is not crashing. */
+    json_get_alloc_funcs(NULL, NULL);
+}
+
 static void run_tests()
 {
     test_simple();
     test_secure_funcs();
     test_oom();
+    test_bad_args();
 }

test/suites/api/test_number.c

@@ -37,6 +37,41 @@ static void test_inifity()
 }
 #endif // INFINITY
 
+static void test_bad_args(void)
+{
+    json_t *txt = json_string("test");
+
+    if(json_integer_value(NULL) != 0)
+        fail("json_integer_value did not return 0 for non-integer");
+    if(json_integer_value(txt) != 0)
+        fail("json_integer_value did not return 0 for non-integer");
+
+    if(!json_integer_set(NULL, 0))
+        fail("json_integer_set did not return error for non-integer");
+    if(!json_integer_set(txt, 0))
+        fail("json_integer_set did not return error for non-integer");
+
+    if(json_real_value(NULL) != 0.0)
+        fail("json_real_value did not return 0.0 for non-real");
+    if(json_real_value(txt) != 0.0)
+        fail("json_real_value did not return 0.0 for non-real");
+
+    if(!json_real_set(NULL, 0.0))
+        fail("json_real_set did not return error for non-real");
+    if(!json_real_set(txt, 0.0))
+        fail("json_real_set did not return error for non-real");
+
+    if(json_number_value(NULL) != 0.0)
+        fail("json_number_value did not return 0.0 for non-numeric");
+    if(json_number_value(txt) != 0.0)
+        fail("json_number_value did not return 0.0 for non-numeric");
+
+    if (txt->refcount != 1)
+        fail("unexpected reference count for txt");
+
+    json_decref(txt);
+}
+
 static void run_tests()
 {
     json_t *integer, *real;
@@ -87,4 +122,5 @@ static void run_tests()
 #ifdef INFINITY
     test_inifity();
 #endif
+    test_bad_args();
 }

test/suites/api/test_object.c

@@ -539,6 +539,127 @@ static void test_object_foreach_safe()
     json_decref(object);
 }
 
+static void test_bad_args(void)
+{
+    json_t *obj = json_object();
+    json_t *num = json_integer(1);
+    void *iter;
+
+    if (!obj || !num)
+        fail("failed to allocate test objects");
+
+    if (json_object_set(obj, "testkey", json_null()))
+        fail("failed to set testkey on object");
+
+    iter = json_object_iter(obj);
+    if (!iter)
+        fail("failed to retrieve test iterator");
+
+    if(json_object_size(NULL) != 0)
+        fail("json_object_size with non-object argument returned non-zero");
+    if(json_object_size(num) != 0)
+        fail("json_object_size with non-object argument returned non-zero");
+
+    if(json_object_get(NULL, "test") != NULL)
+        fail("json_object_get with non-object argument returned non-NULL");
+    if(json_object_get(num, "test") != NULL)
+        fail("json_object_get with non-object argument returned non-NULL");
+    if(json_object_get(obj, NULL) != NULL)
+        fail("json_object_get with NULL key returned non-NULL");
+
+    if(!json_object_set_new_nocheck(NULL, "test", json_null()))
+        fail("json_object_set_new_nocheck with non-object argument did not return error");
+    if(!json_object_set_new_nocheck(num, "test", json_null()))
+        fail("json_object_set_new_nocheck with non-object argument did not return error");
+    if(!json_object_set_new_nocheck(obj, "test", json_incref(obj)))
+        fail("json_object_set_new_nocheck with object == value did not return error");
+    if(!json_object_set_new_nocheck(obj, NULL, json_object()))
+        fail("json_object_set_new_nocheck with NULL key did not return error");
+
+    if(!json_object_del(NULL, "test"))
+        fail("json_object_del with non-object argument did not return error");
+    if(!json_object_del(num, "test"))
+        fail("json_object_del with non-object argument did not return error");
+    if(!json_object_del(obj, NULL))
+        fail("json_object_del with NULL key did not return error");
+
+    if(!json_object_clear(NULL))
+        fail("json_object_clear with non-object argument did not return error");
+    if(!json_object_clear(num))
+        fail("json_object_clear with non-object argument did not return error");
+
+    if(!json_object_update(NULL, obj))
+        fail("json_object_update with non-object first argument did not return error");
+    if(!json_object_update(num, obj))
+        fail("json_object_update with non-object first argument did not return error");
+    if(!json_object_update(obj, NULL))
+        fail("json_object_update with non-object second argument did not return error");
+    if(!json_object_update(obj, num))
+        fail("json_object_update with non-object second argument did not return error");
+
+    if(!json_object_update_existing(NULL, obj))
+        fail("json_object_update_existing with non-object first argument did not return error");
+    if(!json_object_update_existing(num, obj))
+        fail("json_object_update_existing with non-object first argument did not return error");
+    if(!json_object_update_existing(obj, NULL))
+        fail("json_object_update_existing with non-object second argument did not return error");
+    if(!json_object_update_existing(obj, num))
+        fail("json_object_update_existing with non-object second argument did not return error");
+
+    if(!json_object_update_missing(NULL, obj))
+        fail("json_object_update_missing with non-object first argument did not return error");
+    if(!json_object_update_missing(num, obj))
+        fail("json_object_update_missing with non-object first argument did not return error");
+    if(!json_object_update_missing(obj, NULL))
+        fail("json_object_update_missing with non-object second argument did not return error");
+    if(!json_object_update_missing(obj, num))
+        fail("json_object_update_missing with non-object second argument did not return error");
+
+    if(json_object_iter(NULL) != NULL)
+        fail("json_object_iter with non-object argument returned non-NULL");
+    if(json_object_iter(num) != NULL)
+        fail("json_object_iter with non-object argument returned non-NULL");
+
+    if(json_object_iter_at(NULL, "test") != NULL)
+        fail("json_object_iter_at with non-object argument returned non-NULL");
+    if(json_object_iter_at(num, "test") != NULL)
+        fail("json_object_iter_at with non-object argument returned non-NULL");
+    if(json_object_iter_at(obj, NULL) != NULL)
+        fail("json_object_iter_at with NULL iter returned non-NULL");
+
+    if(json_object_iter_next(obj, NULL) != NULL)
+        fail("json_object_iter_next with NULL iter returned non-NULL");
+    if(json_object_iter_next(num, iter) != NULL)
+        fail("json_object_iter_next with non-object argument returned non-NULL");
+
+    if(json_object_iter_key(NULL) != NULL)
+        fail("json_object_iter_key with NULL iter returned non-NULL");
+
+    if(json_object_key_to_iter(NULL) != NULL)
+        fail("json_object_key_to_iter with NULL iter returned non-NULL");
+
+    if(json_object_iter_value(NULL) != NULL)
+        fail("json_object_iter_value with NULL iter returned non-NULL");
+
+    if(!json_object_iter_set_new(NULL, iter, json_incref(num)))
+        fail("json_object_iter_set_new with non-object argument did not return error");
+    if(!json_object_iter_set_new(num, iter, json_incref(num)))
+        fail("json_object_iter_set_new with non-object argument did not return error");
+    if(!json_object_iter_set_new(obj, NULL, json_incref(num)))
+        fail("json_object_iter_set_new with NULL iter did not return error");
+    if(!json_object_iter_set_new(obj, iter, NULL))
+        fail("json_object_iter_set_new with NULL value did not return error");
+
+    if (obj->refcount != 1)
+        fail("unexpected reference count for obj");
+
+    if (num->refcount != 1)
+        fail("unexpected reference count for num");
+
+    json_decref(obj);
+    json_decref(num);
+}
+
 static void run_tests()
 {
     test_misc();
@@ -552,4 +673,5 @@ static void run_tests()
     test_preserve_order();
     test_object_foreach();
     test_object_foreach_safe();
+    test_bad_args();
 }

test/suites/api/test_pack.c

@@ -132,6 +132,9 @@ static void run_tests()
     json_decref(value);
 
     /* string concatenation */
+    if (json_pack("s+", "test", NULL))
+        fail("json_pack string concatenation succeeded with NULL string");
+
     value = json_pack("s++", "te", "st", "ing");
     if(!json_is_string(value) || strcmp("testing", json_string_value(value)))
         fail("json_pack string concatenation failed");
@@ -278,7 +281,7 @@ static void run_tests()
     json_decref(value);
 
     /* Whitespace; regular string */
-    value = json_pack(" s ", "test");
+    value = json_pack(" s\t ", "test");
     if(!json_is_string(value) || strcmp("test", json_string_value(value)))
         fail("json_pack string (with whitespace) failed");
     json_decref(value);
@@ -385,4 +388,9 @@ static void run_tests()
     if(json_pack_ex(&error, 0, "{s:s}", "foo", "\xff\xff"))
         fail("json_pack failed to catch invalid UTF-8 in a string");
     check_error(json_error_invalid_utf8, "Invalid UTF-8 string", "<args>", 1, 4, 4);
+
+    /* Invalid UTF-8 in a concatenated key */
+    if(json_pack_ex(&error, 0, "{s+:i}", "\xff\xff", "concat", 42))
+        fail("json_pack failed to catch invalid UTF-8 in an object key");
+    check_error(json_error_invalid_utf8, "Invalid UTF-8 object key", "<args>", 1, 3, 3);
 }

test/suites/api/test_simple.c

@@ -9,6 +9,56 @@
 #include <jansson.h>
 #include "util.h"
 
+static void test_bad_args(void)
+{
+    json_t *num = json_integer(1);
+    json_t *txt = json_string("test");
+
+    if (!num || !txt)
+        fail("failed to allocate test objects");
+
+    if(json_string_nocheck(NULL) != NULL)
+        fail("json_string_nocheck with NULL argument did not return NULL");
+    if(json_stringn_nocheck(NULL, 0) != NULL)
+        fail("json_stringn_nocheck with NULL argument did not return NULL");
+    if(json_string(NULL) != NULL)
+        fail("json_string with NULL argument did not return NULL");
+    if(json_stringn(NULL, 0) != NULL)
+        fail("json_stringn with NULL argument did not return NULL");
+
+    if(json_string_length(NULL) != 0)
+        fail("json_string_length with non-string argument did not return 0");
+    if(json_string_length(num) != 0)
+        fail("json_string_length with non-string argument did not return 0");
+
+    if(json_string_value(NULL) != NULL)
+        fail("json_string_value with non-string argument did not return NULL");
+    if(json_string_value(num) != NULL)
+        fail("json_string_value with non-string argument did not return NULL");
+
+    if(!json_string_setn_nocheck(NULL, "", 0))
+        fail("json_string_setn with non-string argument did not return error");
+    if(!json_string_setn_nocheck(num, "", 0))
+        fail("json_string_setn with non-string argument did not return error");
+    if(!json_string_setn_nocheck(txt, NULL, 0))
+        fail("json_string_setn_nocheck with NULL value did not return error");
+
+    if(!json_string_set_nocheck(txt, NULL))
+        fail("json_string_set_nocheck with NULL value did not return error");
+    if(!json_string_set(txt, NULL))
+        fail("json_string_set with NULL value did not return error");
+    if(!json_string_setn(txt, NULL, 0))
+        fail("json_string_setn with NULL value did not return error");
+
+    if(num->refcount != 1)
+        fail("unexpected reference count for num");
+    if(txt->refcount != 1)
+        fail("unexpected reference count for txt");
+
+    json_decref(num);
+    json_decref(txt);
+}
+
 /* Call the simple functions not covered by other tests of the public API */
 static void run_tests()
 {
@@ -237,4 +287,6 @@ static void run_tests()
 	fail("automatic decrement failed");
     json_decref(value);
 #endif
+
+    test_bad_args();
 }

test/suites/api/test_sprintf.c

@@ -13,6 +13,18 @@ static void test_sprintf() {
         fail("json_sprintf generated an unexpected string");
 
     json_decref(s);
+
+    s = json_sprintf("%s", "");
+    if (!s)
+        fail("json_sprintf returned NULL");
+    if (!json_is_string(s))
+        fail("json_sprintf didn't return a JSON string");
+    if (json_string_length(s) != 0)
+        fail("string is not empty");
+    json_decref(s);
+
+    if (json_sprintf("%s", "\xff\xff"))
+        fail("json_sprintf unexpected success with invalid UTF");
 }