[json-glib/deep-copy] Fix copy behavior for complex types

[Emmanuele Bassi <ebassi src gnome org>]

commit 73b4f5d3e25d0ecfafd390a880365ffa858f0e73
Author: Ole André Vadla Ravnås <oleavr gmail com>
Date:   Mon Aug 24 17:00:50 2020 +0100

    Fix copy behavior for complex types
    
    The parent of the reffed children were previously left pointing to the
    original node. In the best-case scenario this would lead to inconsistent
    state when walking down a tree and then walking back up again. In the
    worst-case scenario this would happen when the original node had a
    shorter life-time than the copy, resulting in use-after-free.
    
    A typical scenario where this went wrong was with json_from_string(),
    which would copy the root node, let go of the last reference to the root
    node, and then return the copy. The copy would then have dangling
    `parent` pointers. This probably went unnoticed for most use-cases, but
    would go terribly wrong if someone used a JsonReader and navigated back
    up the tree by e.g. calling end_member().
    
    Fixes: #20
    Fixes: #32

 json-glib/json-array.c         | 24 ++++++++++++++++++++++++
 json-glib/json-node.c          |  9 ++++-----
 json-glib/json-object.c        | 29 +++++++++++++++++++++++++++++
 json-glib/json-types-private.h |  8 ++++++++
 json-glib/tests/node.c         |  2 +-
 5 files changed, 66 insertions(+), 6 deletions(-)
---
diff --git a/json-glib/json-array.c b/json-glib/json-array.c
index 4834cdc..ecb9e72 100644
--- a/json-glib/json-array.c
+++ b/json-glib/json-array.c
@@ -85,6 +85,30 @@ json_array_sized_new (guint n_elements)
   return array;
 }
 
+JsonArray *
+json_array_copy (JsonArray *array,
+                 JsonNode  *new_parent)
+{
+  JsonArray *copy;
+  guint i;
+
+  copy = json_array_sized_new (array->elements->len);
+  for (i = 0; i < array->elements->len; i++)
+    {
+      JsonNode *child_copy;
+
+      child_copy = json_node_copy (g_ptr_array_index (array->elements, i));
+      child_copy->parent = new_parent;
+      g_ptr_array_index (copy->elements, i) = child_copy;
+    }
+  copy->elements->len = array->elements->len;
+
+  copy->immutable_hash = array->immutable_hash;
+  copy->immutable = array->immutable;
+
+  return copy;
+}
+
 /**
  * json_array_ref:
  * @array: a #JsonArray
diff --git a/json-glib/json-node.c b/json-glib/json-node.c
index a6898d9..fc12d1f 100644
--- a/json-glib/json-node.c
+++ b/json-glib/json-node.c
@@ -398,9 +398,8 @@ json_node_new (JsonNodeType type)
  * json_node_copy:
  * @node: a #JsonNode
  *
- * Copies @node. If the node contains complex data types, their reference
- * counts are increased, regardless of whether the node is mutable or
- * immutable.
+ * Copies @node. If the node contains complex data types, those will also
+ * be copied.
  *
  * The copy will be immutable if, and only if, @node is immutable. However,
  * there should be no need to copy an immutable node.
@@ -430,11 +429,11 @@ json_node_copy (JsonNode *node)
   switch (copy->type)
     {
     case JSON_NODE_OBJECT:
-      copy->data.object = json_node_dup_object (node);
+      copy->data.object = json_object_copy (node->data.object, copy);
       break;
 
     case JSON_NODE_ARRAY:
-      copy->data.array = json_node_dup_array (node);
+      copy->data.array = json_array_copy (node->data.array, copy);
       break;
 
     case JSON_NODE_VALUE:
diff --git a/json-glib/json-object.c b/json-glib/json-object.c
index 8e6b293..4335256 100644
--- a/json-glib/json-object.c
+++ b/json-glib/json-object.c
@@ -72,6 +72,35 @@ json_object_new (void)
   return object;
 }
 
+JsonObject *
+json_object_copy (JsonObject *object,
+                  JsonNode   *new_parent)
+{
+  JsonObject *copy;
+  GList *cur;
+
+  copy = json_object_new ();
+
+  for (cur = object->members_ordered.head; cur != NULL; cur = cur->next)
+    {
+      gchar *name;
+      JsonNode *child_copy;
+
+      name = g_strdup (cur->data);
+
+      child_copy = json_node_copy (g_hash_table_lookup (object->members, name));
+      child_copy->parent = new_parent;
+
+      g_hash_table_insert (copy->members, name, child_copy);
+      g_queue_push_tail (&copy->members_ordered, name);
+    }
+
+  copy->immutable_hash = object->immutable_hash;
+  copy->immutable = object->immutable;
+
+  return copy;
+}
+
 /**
  * json_object_ref:
  * @object: a #JsonObject
diff --git a/json-glib/json-types-private.h b/json-glib/json-types-private.h
index bc29e80..fca579c 100644
--- a/json-glib/json-types-private.h
+++ b/json-glib/json-types-private.h
@@ -169,6 +169,14 @@ void            json_value_seal                 (JsonValue       *value);
 G_GNUC_INTERNAL
 guint           json_value_hash                 (gconstpointer    key);
 
+G_GNUC_INTERNAL
+JsonArray *     json_array_copy                 (JsonArray       *array,
+                                                 JsonNode        *new_parent);
+
+G_GNUC_INTERNAL
+JsonObject *    json_object_copy                (JsonObject      *object,
+                                                 JsonNode        *new_parent);
+
 G_END_DECLS
 
 #endif /* __JSON_TYPES_PRIVATE_H__ */
diff --git a/json-glib/tests/node.c b/json-glib/tests/node.c
index 80beb78..abab8f0 100644
--- a/json-glib/tests/node.c
+++ b/json-glib/tests/node.c
@@ -91,7 +91,7 @@ test_copy_object (void)
   copy = json_node_copy (node);
 
   g_assert_cmpint (json_node_get_node_type (node), ==, json_node_get_node_type (copy));
-  g_assert (json_node_get_object (node) == json_node_get_object (copy));
+  g_assert (json_node_get_object (node) != json_node_get_object (copy));
 
   json_node_free (copy);
   json_node_free (node);