Improve JSON serializable object recursion protection

Author: bukka

Diff for: Zend/zend.c

@@ -554,7 +554,7 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /*
 					smart_str_appendc(buf, '\n');
 				}
 
-				if (GC_IS_RECURSIVE(Z_OBJ_P(expr))) {
+				if (GC_IS_DEBUG_RECURSIVE(Z_OBJ_P(expr))) {
 					smart_str_appends(buf, " *RECURSION*");
 					return;
 				}
@@ -564,9 +564,9 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /*
 					break;
 				}
 
-				GC_PROTECT_RECURSION(Z_OBJ_P(expr));
+				GC_PROTECT_DEBUG_RECURSION(Z_OBJ_P(expr));
 				print_hash(buf, properties, indent, 1);
-				GC_UNPROTECT_RECURSION(Z_OBJ_P(expr));
+				GC_UNPROTECT_DEBUG_RECURSION(Z_OBJ_P(expr));
 
 				zend_release_properties(properties);
 				break;

Diff for: Zend/zend_gc.c

@@ -79,13 +79,13 @@
 #endif
 
 /* GC_INFO layout */
-#define GC_ADDRESS  0x0fffffu
-#define GC_COLOR    0x300000u
+#define GC_ADDRESS  0x07ffffu
+#define GC_COLOR    0x180000u
 
 #define GC_BLACK    0x000000u /* must be zero */
-#define GC_WHITE    0x100000u
-#define GC_GREY     0x200000u
-#define GC_PURPLE   0x300000u
+#define GC_WHITE    0x080000u
+#define GC_GREY     0x100000u
+#define GC_PURPLE   0x180000u
 
 /* Debug tracing */
 #if ZEND_GC_DEBUG > 1

Diff for: Zend/zend_types.h

@@ -642,10 +642,10 @@ static zend_always_inline zend_uchar zval_get_type(const zval* pz) {
 #define GC_TRY_DELREF(p)			zend_gc_try_delref(&(p)->gc)
 
 #define GC_TYPE_MASK				0x0000000f
-#define GC_FLAGS_MASK				0x000003f0
-#define GC_INFO_MASK				0xfffffc00
+#define GC_FLAGS_MASK				0x000007f0
+#define GC_INFO_MASK				0xfffff800
 #define GC_FLAGS_SHIFT				0
-#define GC_INFO_SHIFT				10
+#define GC_INFO_SHIFT				11
 
 static zend_always_inline zend_uchar zval_gc_type(uint32_t gc_type_info) {
 	return (gc_type_info & GC_TYPE_MASK);
@@ -684,10 +684,11 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {
 
 /* zval_gc_flags(zval.value->gc.u.type_info) (common flags) */
 #define GC_NOT_COLLECTABLE			(1<<4)
-#define GC_PROTECTED                (1<<5) /* used for recursion detection */
-#define GC_IMMUTABLE                (1<<6) /* can't be changed in place */
-#define GC_PERSISTENT               (1<<7) /* allocated using malloc */
-#define GC_PERSISTENT_LOCAL         (1<<8) /* persistent, but thread-local */
+#define GC_PROTECTED                (1<<5)  /* used for recursion detection */
+#define GC_IMMUTABLE                (1<<6)  /* can't be changed in place */
+#define GC_PERSISTENT               (1<<7)  /* allocated using malloc */
+#define GC_PERSISTENT_LOCAL         (1<<8)  /* persistent, but thread-local */
+#define GC_DEBUG_PROTECTED          (1<<10) /* whether the protection is for debug functions */
 
 #define GC_NULL						(IS_NULL         | (GC_NOT_COLLECTABLE << GC_FLAGS_SHIFT))
 #define GC_STRING					(IS_STRING       | (GC_NOT_COLLECTABLE << GC_FLAGS_SHIFT))
@@ -789,6 +790,24 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {
 #define Z_PROTECT_RECURSION_P(zv)   Z_PROTECT_RECURSION(*(zv))
 #define Z_UNPROTECT_RECURSION_P(zv) Z_UNPROTECT_RECURSION(*(zv))
 
+#define GC_IS_DEBUG_RECURSIVE(p) \
+	((GC_FLAGS(p) & GC_PROTECTED) && (GC_FLAGS(p) & GC_DEBUG_PROTECTED))
+
+#define GC_PROTECT_DEBUG_RECURSION(p) do { \
+		GC_ADD_FLAGS(p, (GC_PROTECTED | GC_DEBUG_PROTECTED)); \
+	} while (0)
+
+#define GC_UNPROTECT_DEBUG_RECURSION(p) do { \
+		GC_DEL_FLAGS(p, (GC_PROTECTED | GC_DEBUG_PROTECTED)); \
+	} while (0)
+
+#define Z_IS_DEBUG_RECURSIVE(zval)        GC_IS_DEBUG_RECURSIVE(Z_COUNTED(zval))
+#define Z_PROTECT_DEBUG_RECURSION(zval)   GC_PROTECT_DEBUG_RECURSION(Z_COUNTED(zval))
+#define Z_UNPROTECT_DEBUG_RECURSION(zval) GC_UNPROTECT_DEBUG_RECURSION(Z_COUNTED(zval))
+#define Z_IS_DEBUG_RECURSIVE_P(zv)        Z_IS_DEBUG_RECURSIVE(*(zv))
+#define Z_PROTECT_DEBUG_RECURSION_P(zv)   Z_PROTECT_DEBUG_RECURSION(*(zv))
+#define Z_UNPROTECT_DEBUG_RECURSION_P(zv) Z_UNPROTECT_DEBUG_RECURSION(*(zv))
+
 /* All data types < IS_STRING have their constructor/destructors skipped */
 #define Z_CONSTANT(zval)			(Z_TYPE(zval) == IS_CONSTANT_AST)
 #define Z_CONSTANT_P(zval_p)		Z_CONSTANT(*(zval_p))

Diff for: ext/json/json_encoder.c

@@ -532,10 +532,29 @@ static zend_result php_json_escape_string(
 }
 /* }}} */
 
+#define PHP_JSON_SERIALIZABLE_OBJECT_PROTECT_RECURSION(_obj, _ht) \
+	do { \
+		if (UNEXPECTED(_ht)) { \
+			GC_PROTECT_RECURSION(_ht); \
+		} else { \
+			GC_PROTECT_RECURSION(_obj); \
+		} \
+	} while (0)
+
+#define PHP_JSON_SERIALIZABLE_OBJECT_UNPROTECT_RECURSION(_obj, _ht) \
+	do { \
+		if (UNEXPECTED(_ht)) { \
+			GC_UNPROTECT_RECURSION(_ht); \
+		} else { \
+			GC_UNPROTECT_RECURSION(_obj); \
+		} \
+	} while (0)
+
 static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val, int options, php_json_encoder *encoder) /* {{{ */
 {
 	zend_class_entry *ce = Z_OBJCE_P(val);
-	HashTable* myht = Z_OBJPROP_P(val);
+	zend_object *obj = Z_OBJ_P(val);
+	HashTable* myht = GC_IS_RECURSIVE(obj) ? Z_OBJPROP_P(val) : NULL;
 	zval retval, fname;
 	zend_result return_code;
 
@@ -547,7 +566,7 @@ static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val
 		return FAILURE;
 	}
 
-	PHP_JSON_HASH_PROTECT_RECURSION(myht);
+	PHP_JSON_SERIALIZABLE_OBJECT_PROTECT_RECURSION(obj, myht);
 
 	ZVAL_STRING(&fname, "jsonSerialize");
 
@@ -560,7 +579,7 @@ static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val
 		if (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR) {
 			smart_str_appendl(buf, "null", 4);
 		}
-		PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
+		PHP_JSON_SERIALIZABLE_OBJECT_UNPROTECT_RECURSION(obj, myht);
 		return FAILURE;
 	}
 
@@ -572,19 +591,19 @@ static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val
 		if (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR) {
 			smart_str_appendl(buf, "null", 4);
 		}
-		PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
+		PHP_JSON_SERIALIZABLE_OBJECT_UNPROTECT_RECURSION(obj, myht);
 		return FAILURE;
 	}
 
 	if ((Z_TYPE(retval) == IS_OBJECT) &&
 		(Z_OBJ(retval) == Z_OBJ_P(val))) {
 		/* Handle the case where jsonSerialize does: return $this; by going straight to encode array */
-		PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
+		PHP_JSON_SERIALIZABLE_OBJECT_UNPROTECT_RECURSION(obj, myht);
 		return_code = php_json_encode_array(buf, &retval, options, encoder);
 	} else {
 		/* All other types, encode as normal */
 		return_code = php_json_encode_zval(buf, &retval, options, encoder);
-		PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
+		PHP_JSON_SERIALIZABLE_OBJECT_UNPROTECT_RECURSION(obj, myht);
 	}
 
 	zval_ptr_dtor(&retval);

Diff for: ext/json/tests/bug61978.phpt

@@ -38,4 +38,4 @@ var_dump(json_encode($obj2, JSON_PARTIAL_OUTPUT_ON_ERROR));
 --EXPECT--
 string(24) "{"test":"123","me":null}"
 ==
-string(24) "{"test":"123","me":null}"
+string(44) "{"test":"123","me":{"test":"123","me":null}}"

Diff for: ext/json/tests/json_encode_recursion_01.phpt

@@ -0,0 +1,27 @@
+--TEST--
+json_encode() Recursion test with just JsonSerializable
+--FILE--
+<?php
+
+class SerializingTest implements JsonSerializable
+{
+    public $a = 1;
+
+    private $b = 'hide';
+
+    protected $c = 'protect';
+
+    public function jsonSerialize(): mixed
+    {
+        $result = json_encode($this);
+        var_dump($result);
+        return $this;
+    }
+}
+
+var_dump(json_encode(new SerializingTest()));
+?>
+--EXPECT--
+bool(false)
+string(7) "{"a":1}"
+string(7) "{"a":1}"

Diff for: ext/json/tests/json_encode_recursion_02.phpt

@@ -0,0 +1,25 @@
+--TEST--
+json_encode() Recursion test with JsonSerializable and var_dump simple
+--FILE--
+<?php
+
+class SerializingTest implements JsonSerializable
+{
+    public $a = 1;
+
+    public function jsonSerialize(): mixed
+    {
+        var_dump($this);
+        return $this;
+    }
+}
+
+var_dump(json_encode(new SerializingTest()));
+
+?>
+--EXPECT--
+object(SerializingTest)#1 (1) {
+  ["a"]=>
+  int(1)
+}
+string(7) "{"a":1}"

Diff for: ext/json/tests/json_encode_recursion_03.phpt

@@ -0,0 +1,39 @@
+--TEST--
+json_encode() Recursion test with JsonSerializable and __debugInfo
+--FILE--
+<?php
+
+class SerializingTest implements JsonSerializable
+{
+    public $a = 1;
+
+    public function __debugInfo()
+    {
+        return [ 'result' => json_encode($this) ];
+    }
+
+    public function jsonSerialize(): mixed
+    {
+        var_dump($this);
+        return $this;
+    }
+}
+
+var_dump(json_encode(new SerializingTest()));
+echo "---------\n";
+var_dump(new SerializingTest());
+
+?>
+--EXPECT--
+*RECURSION*
+object(SerializingTest)#1 (1) {
+  ["result"]=>
+  string(7) "{"a":1}"
+}
+string(7) "{"a":1}"
+---------
+*RECURSION*
+object(SerializingTest)#1 (1) {
+  ["result"]=>
+  string(7) "{"a":1}"
+}

Diff for: ext/json/tests/json_encode_recursion_04.phpt

@@ -0,0 +1,41 @@
+--TEST--
+json_encode() Recursion test with JsonSerializable, __debugInfo and var_export
+--FILE--
+<?php
+
+class SerializingTest implements JsonSerializable
+{
+    public $a = 1;
+
+    public function __debugInfo()
+    {
+        return [ 'result' => var_export($this, true) ];
+    }
+
+    public function jsonSerialize(): mixed
+    {
+        var_dump($this);
+        return $this;
+    }
+}
+
+var_dump(json_encode(new SerializingTest()));
+echo "---------\n";
+var_dump(new SerializingTest());
+
+?>
+--EXPECT--
+object(SerializingTest)#1 (1) {
+  ["result"]=>
+  string(52) "\SerializingTest::__set_state(array(
+   'a' => 1,
+))"
+}
+string(7) "{"a":1}"
+---------
+object(SerializingTest)#1 (1) {
+  ["result"]=>
+  string(52) "\SerializingTest::__set_state(array(
+   'a' => 1,
+))"
+}

Diff for: ext/json/tests/json_encode_recursion_05.phpt

@@ -0,0 +1,37 @@
+--TEST--
+json_encode() Recursion test with JsonSerializable, __debugInfo and print_r
+--FILE--
+<?php
+
+class SerializingTest implements JsonSerializable
+{
+    public $a = 1;
+
+    public function __debugInfo()
+    {
+        return [ 'result' => $this->a ];
+    }
+
+    public function jsonSerialize(): mixed
+    {
+        print_r($this);
+        return $this;
+    }
+}
+
+var_dump(json_encode(new SerializingTest()));
+echo "---------\n";
+var_dump(new SerializingTest());
+
+?>
+--EXPECT--
+SerializingTest Object
+(
+    [result] => 1
+)
+string(7) "{"a":1}"
+---------
+object(SerializingTest)#1 (1) {
+  ["result"]=>
+  int(1)
+}

Diff for: ext/json/tests/json_encode_recursion_06.phpt

@@ -0,0 +1,41 @@
+--TEST--
+json_encode() Recursion test with JsonSerializable and serialize
+--FILE--
+<?php
+
+class JsonEncodeFirstTest implements JsonSerializable
+{
+    public $a = 1;
+
+    public function __serialize()
+    {
+        return [ 'result' => $this->a ];
+    }
+
+    public function jsonSerialize(): mixed
+    {
+        return [ 'serialize' => serialize($this) ];
+    }
+}
+
+class SerializeFirstTest implements JsonSerializable
+{
+    public $a = 1;
+
+    public function __serialize()
+    {
+        return [ 'result' => json_encode($this) ];
+    }
+
+    public function jsonSerialize(): mixed
+    {
+        return [ 'json' => serialize($this) ];
+    }
+}
+
+var_dump(json_encode(new JsonEncodeFirstTest()));
+var_dump(serialize(new SerializeFirstTest()));
+?>
+--EXPECT--
+string(68) "{"serialize":"O:19:\"JsonEncodeFirstTest\":1:{s:6:\"result\";i:1;}"}"
+string(194) "O:18:"SerializeFirstTest":1:{s:6:"result";s:142:"{"json":"O:18:\"SerializeFirstTest\":1:{s:6:\"result\";s:62:\"{\"json\":\"O:18:\\\"SerializeFirstTest\\\":1:{s:6:\\\"result\\\";b:0;}\"}\";}"}";}"