Merge pull request #490 from AllenX2018/master

add loop check in json deep copy #457

doc/apiref.rst

@@ -1091,7 +1091,9 @@ These functions output UTF-8:
 
    Returns the JSON representation of *json* as a string, or *NULL* on
    error. *flags* is described above. The return value must be freed
-   by the caller using :func:`free()`.
+   by the caller using :func:`free()`. Note that if you have called
+   :func:`json_set_alloc_funcs()` to override :func:`free()`, you should
+   call your custom free function instead to free the return value.
 
 .. function:: size_t json_dumpb(const json_t *json, char *buffer, size_t size, size_t flags)
 

src/dump.c

@@ -196,15 +196,6 @@ static int compare_keys(const void *key1, const void *key2)
     return strcmp(*(const char **)key1, *(const char **)key2);
 }
 
-static int loop_check(hashtable_t *parents, const json_t *json, char *key, size_t key_size)
-{
-    snprintf(key, key_size, "%p", json);
-    if (hashtable_get(parents, key))
-        return -1;
-
-    return hashtable_set(parents, key, json_null());
-}
-
 static int do_dump(const json_t *json, size_t flags, int depth,
                    hashtable_t *parents, json_dump_callback_t dump, void *data)
 {
@@ -264,7 +255,7 @@ static int do_dump(const json_t *json, size_t flags, int depth,
             char key[2 + (sizeof(json) * 2) + 1];
 
             /* detect circular references */
-            if (loop_check(parents, json, key, sizeof(key)))
+            if (jsonp_loop_check(parents, json, key, sizeof(key)))
                 return -1;
 
             n = json_array_size(json);
@@ -305,8 +296,7 @@ static int do_dump(const json_t *json, size_t flags, int depth,
             void *iter;
             const char *separator;
             int separator_length;
-            /* Space for "0x", double the sizeof a pointer for the hex and a terminator. */
+            char loop_key[LOOP_KEY_LEN];
-            char loop_key[2 + (sizeof(json) * 2) + 1];
 
             if(flags & JSON_COMPACT) {
                 separator = ":";
@@ -318,7 +308,7 @@ static int do_dump(const json_t *json, size_t flags, int depth,
             }
 
             /* detect circular references */
-            if (loop_check(parents, json, loop_key, sizeof(loop_key)))
+            if (jsonp_loop_check(parents, json, loop_key, sizeof(loop_key)))
                 return -1;
 
             iter = json_object_iter((json_t *)json);

src/jansson_private.h

@@ -90,6 +90,11 @@ char *jsonp_strndup(const char *str, size_t length) JANSSON_ATTRS(warn_unused_re
 char *jsonp_strdup(const char *str) JANSSON_ATTRS(warn_unused_result);
 char *jsonp_strndup(const char *str, size_t len) JANSSON_ATTRS(warn_unused_result);
 
+/* Circular reference check*/
+/* Space for "0x", double the sizeof a pointer for the hex and a terminator. */
+#define LOOP_KEY_LEN (2 + (sizeof(json_t *) * 2) + 1)
+int jsonp_loop_check(hashtable_t *parents, const json_t *json, char *key, size_t key_size);
+
 
 /* Windows compatibility */
 #if defined(_WIN32) || defined(WIN32)

src/value.c

@@ -37,12 +37,22 @@ static JSON_INLINE int isnan(double x) { return x != x; }
 static JSON_INLINE int isinf(double x) { return !isnan(x) && isnan(x - x); }
 #endif
 
+json_t *do_deep_copy(const json_t *, hashtable_t *);
+
 static JSON_INLINE void json_init(json_t *json, json_type type)
 {
     json->type = type;
     json->refcount = 1;
 }
 
+int jsonp_loop_check(hashtable_t *parents, const json_t *json, char *key, size_t key_size)
+{
+    snprintf(key, key_size, "%p", json);
+    if (hashtable_get(parents, key))
+        return -1;
+
+    return hashtable_set(parents, key, json_null());
+}
 
 /*** object ***/
 
@@ -308,10 +318,14 @@ static json_t *json_object_copy(json_t *object)
     return result;
 }
 
-static json_t *json_object_deep_copy(const json_t *object)
+static json_t *json_object_deep_copy(const json_t *object, hashtable_t *parents)
 {
     json_t *result;
     void *iter;
+    char loop_key[LOOP_KEY_LEN];
+
+    if (jsonp_loop_check(parents, object, loop_key, sizeof(loop_key)))
+        return NULL;
 
     result = json_object();
     if(!result)
@@ -326,9 +340,15 @@ static json_t *json_object_deep_copy(const json_t *object)
         key = json_object_iter_key(iter);
         value = json_object_iter_value(iter);
 
-        json_object_set_new_nocheck(result, key, json_deep_copy(value));
+        if (json_object_set_new_nocheck(result, key, do_deep_copy(value, parents)))
+        {
+            json_decref(result);
+            result = NULL;
+            break;
+        }
         iter = json_object_iter_next((json_t *)object, iter);
     }
+    hashtable_del(parents, loop_key);
 
     return result;
 }
@@ -617,17 +637,29 @@ static json_t *json_array_copy(json_t *array)
     return result;
 }
 
-static json_t *json_array_deep_copy(const json_t *array)
+static json_t *json_array_deep_copy(const json_t *array, hashtable_t *parents)
 {
     json_t *result;
     size_t i;
+    char loop_key[LOOP_KEY_LEN];
+
+    if (jsonp_loop_check(parents, array, loop_key, sizeof(loop_key)))
+        return NULL;
 
     result = json_array();
     if(!result)
         return NULL;
 
     for(i = 0; i < json_array_size(array); i++)
-        json_array_append_new(result, json_deep_copy(json_array_get(array, i)));
+    {
+        if (json_array_append_new(result, do_deep_copy(json_array_get(array, i), parents)))
+        {
+            json_decref(result);
+            result = NULL;
+            break;
+        }
+    }
+    hashtable_del(parents, loop_key);
 
     return result;
 }
@@ -1047,15 +1079,28 @@ json_t *json_copy(json_t *json)
 }
 
 json_t *json_deep_copy(const json_t *json)
+{
+    json_t *res;
+    hashtable_t parents_set;
+
+    if (hashtable_init(&parents_set))
+        return NULL;
+    res = do_deep_copy(json, &parents_set);
+    hashtable_close(&parents_set);
+
+    return res;
+}
+
+json_t *do_deep_copy(const json_t *json, hashtable_t *parents)
 {
     if(!json)
         return NULL;
 
     switch(json_typeof(json)) {
         case JSON_OBJECT:
-            return json_object_deep_copy(json);
+            return json_object_deep_copy(json, parents);
         case JSON_ARRAY:
-            return json_array_deep_copy(json);
+            return json_array_deep_copy(json, parents);
             /* for the rest of the types, deep copying doesn't differ from
                shallow copying */
         case JSON_STRING:

test/suites/api/test_copy.c

@@ -323,6 +323,58 @@ static void test_deep_copy_object(void)
     json_decref(copy);
 }
 
+static void test_deep_copy_circular_references(void)
+{
+     /* Construct a JSON object/array with a circular reference:
+
+       object: {"a": {"b": {"c": <circular reference to $.a>}}}
+       array: [[[<circular reference to the $[0] array>]]]
+
+       Deep copy it, remove the circular reference and deep copy again.
+    */
+
+    json_t *json;
+    json_t *copy;
+
+    json = json_object();
+    json_object_set_new(json, "a", json_object());
+    json_object_set_new(json_object_get(json, "a"), "b", json_object());
+    json_object_set(json_object_get(json_object_get(json, "a"), "b"), "c",
+                    json_object_get(json, "a"));
+
+    copy = json_deep_copy(json);
+    if(copy)
+        fail("json_deep_copy copied a circular reference!");
+
+    json_object_del(json_object_get(json_object_get(json, "a"), "b"), "c");
+
+    copy = json_deep_copy(json);
+    if(!copy)
+        fail("json_deep_copy failed!");
+
+    json_decref(copy);
+    json_decref(json);
+
+    json = json_array();
+    json_array_append_new(json, json_array());
+    json_array_append_new(json_array_get(json, 0), json_array());
+    json_array_append(json_array_get(json_array_get(json, 0), 0),
+                      json_array_get(json, 0));
+
+    copy = json_deep_copy(json);
+    if(copy)
+        fail("json_deep_copy copied a circular reference!");
+
+    json_array_remove(json_array_get(json_array_get(json, 0), 0), 0);
+
+    copy = json_deep_copy(json);
+    if(!copy)
+        fail("json_deep_copy failed!");
+
+    json_decref(copy);
+    json_decref(json);
+}
+
 static void run_tests()
 {
     test_copy_simple();
@@ -331,4 +383,5 @@ static void run_tests()
     test_deep_copy_array();
     test_copy_object();
     test_deep_copy_object();
+    test_deep_copy_circular_references();
 }