Skip to content

Commit bb5e561

Browse files
bukkabukka
committed
Improve JSON serializable object recursion protection
1 parent ebde5d3 commit bb5e561

14 files changed

+283
-28
lines changed

Diff for: ‎Zend/zend.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /*
501501
smart_str_appendc(buf, '\n');
502502
}
503503

504-
if (GC_IS_RECURSIVE(Z_OBJ_P(expr))) {
504+
if (GC_IS_DEBUG_RECURSIVE(Z_OBJ_P(expr))) {
505505
smart_str_appends(buf, " *RECURSION*");
506506
return;
507507
}
@@ -511,9 +511,9 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /*
511511
break;
512512
}
513513

514-
GC_PROTECT_RECURSION(Z_OBJ_P(expr));
514+
GC_PROTECT_DEBUG_RECURSION(Z_OBJ_P(expr));
515515
print_hash(buf, properties, indent, 1);
516-
GC_UNPROTECT_RECURSION(Z_OBJ_P(expr));
516+
GC_UNPROTECT_DEBUG_RECURSION(Z_OBJ_P(expr));
517517

518518
zend_release_properties(properties);
519519
break;

Diff for: ‎Zend/zend_types.h

+19
Original file line numberDiff line numberDiff line change
@@ -681,6 +681,7 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {
681681
#define GC_IMMUTABLE (1<<6) /* can't be changed in place */
682682
#define GC_PERSISTENT (1<<7) /* allocated using malloc */
683683
#define GC_PERSISTENT_LOCAL (1<<8) /* persistent, but thread-local */
684+
#define GC_DEBUG_PROTECTED (1<<9) /* whether the protection is for debug functions */
684685

685686
#define GC_NULL (IS_NULL | (GC_NOT_COLLECTABLE << GC_FLAGS_SHIFT))
686687
#define GC_STRING (IS_STRING | (GC_NOT_COLLECTABLE << GC_FLAGS_SHIFT))
@@ -782,6 +783,24 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {
782783
#define Z_PROTECT_RECURSION_P(zv) Z_PROTECT_RECURSION(*(zv))
783784
#define Z_UNPROTECT_RECURSION_P(zv) Z_UNPROTECT_RECURSION(*(zv))
784785

786+
#define GC_IS_DEBUG_RECURSIVE(p) \
787+
((GC_FLAGS(p) & GC_PROTECTED) && (GC_FLAGS(p) & GC_DEBUG_PROTECTED))
788+
789+
#define GC_PROTECT_DEBUG_RECURSION(p) do { \
790+
GC_ADD_FLAGS(p, (GC_PROTECTED | GC_DEBUG_PROTECTED)); \
791+
} while (0)
792+
793+
#define GC_UNPROTECT_DEBUG_RECURSION(p) do { \
794+
GC_DEL_FLAGS(p, (GC_PROTECTED | GC_DEBUG_PROTECTED)); \
795+
} while (0)
796+
797+
#define Z_IS_DEBUG_RECURSIVE(zval) GC_IS_DEBUG_RECURSIVE(Z_COUNTED(zval))
798+
#define Z_PROTECT_DEBUG_RECURSION(zval) GC_PROTECT_DEBUG_RECURSION(Z_COUNTED(zval))
799+
#define Z_UNPROTECT_DEBUG_RECURSION(zval) GC_UNPROTECT_DEBUG_RECURSION(Z_COUNTED(zval))
800+
#define Z_IS_DEBUG_RECURSIVE_P(zv) Z_IS_DEBUG_RECURSIVE(*(zv))
801+
#define Z_PROTECT_DEBUG_RECURSION_P(zv) Z_PROTECT_DEBUG_RECURSION(*(zv))
802+
#define Z_UNPROTECT_DEBUG_RECURSION_P(zv) Z_UNPROTECT_DEBUG_RECURSION(*(zv))
803+
785804
/* All data types < IS_STRING have their constructor/destructors skipped */
786805
#define Z_CONSTANT(zval) (Z_TYPE(zval) == IS_CONSTANT_AST)
787806
#define Z_CONSTANT_P(zval_p) Z_CONSTANT(*(zval_p))

Diff for: ‎ext/json/json_encoder.c

+25-6
Original file line numberDiff line numberDiff line change
@@ -532,10 +532,29 @@ static zend_result php_json_escape_string(
532532
}
533533
/* }}} */
534534

535+
#define PHP_JSON_SERIALIZABLE_OBJECT_PROTECT_RECURSION(_obj, _ht) \
536+
do { \
537+
if (UNEXPECTED(_ht)) { \
538+
GC_PROTECT_RECURSION(_ht); \
539+
} else { \
540+
GC_PROTECT_RECURSION(_obj); \
541+
} \
542+
} while (0)
543+
544+
#define PHP_JSON_SERIALIZABLE_OBJECT_UNPROTECT_RECURSION(_obj, _ht) \
545+
do { \
546+
if (UNEXPECTED(_ht)) { \
547+
GC_UNPROTECT_RECURSION(_ht); \
548+
} else { \
549+
GC_UNPROTECT_RECURSION(_obj); \
550+
} \
551+
} while (0)
552+
535553
static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val, int options, php_json_encoder *encoder) /* {{{ */
536554
{
537555
zend_class_entry *ce = Z_OBJCE_P(val);
538-
HashTable* myht = Z_OBJPROP_P(val);
556+
zend_object *obj = Z_OBJ_P(val);
557+
HashTable* myht = GC_IS_RECURSIVE(obj) ? Z_OBJPROP_P(val) : NULL;
539558
zval retval, fname;
540559
zend_result return_code;
541560

@@ -547,7 +566,7 @@ static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val
547566
return FAILURE;
548567
}
549568

550-
PHP_JSON_HASH_PROTECT_RECURSION(myht);
569+
PHP_JSON_SERIALIZABLE_OBJECT_PROTECT_RECURSION(obj, myht);
551570

552571
ZVAL_STRING(&fname, "jsonSerialize");
553572

@@ -560,7 +579,7 @@ static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val
560579
if (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR) {
561580
smart_str_appendl(buf, "null", 4);
562581
}
563-
PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
582+
PHP_JSON_SERIALIZABLE_OBJECT_UNPROTECT_RECURSION(obj, myht);
564583
return FAILURE;
565584
}
566585

@@ -572,19 +591,19 @@ static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val
572591
if (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR) {
573592
smart_str_appendl(buf, "null", 4);
574593
}
575-
PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
594+
PHP_JSON_SERIALIZABLE_OBJECT_UNPROTECT_RECURSION(obj, myht);
576595
return FAILURE;
577596
}
578597

579598
if ((Z_TYPE(retval) == IS_OBJECT) &&
580599
(Z_OBJ(retval) == Z_OBJ_P(val))) {
581600
/* Handle the case where jsonSerialize does: return $this; by going straight to encode array */
582-
PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
601+
PHP_JSON_SERIALIZABLE_OBJECT_UNPROTECT_RECURSION(obj, myht);
583602
return_code = php_json_encode_array(buf, &retval, options, encoder);
584603
} else {
585604
/* All other types, encode as normal */
586605
return_code = php_json_encode_zval(buf, &retval, options, encoder);
587-
PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
606+
PHP_JSON_SERIALIZABLE_OBJECT_UNPROTECT_RECURSION(obj, myht);
588607
}
589608

590609
zval_ptr_dtor(&retval);

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -38,4 +38,4 @@ var_dump(json_encode($obj2, JSON_PARTIAL_OUTPUT_ON_ERROR));
3838
--EXPECT--
3939
string(24) "{"test":"123","me":null}"
4040
==
41-
string(24) "{"test":"123","me":null}"
41+
string(44) "{"test":"123","me":{"test":"123","me":null}}"

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

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
json_encode() Recursion test with just JsonSerializable
3+
--FILE--
4+
<?php
5+
6+
class SerializingTest implements JsonSerializable
7+
{
8+
public $a = 1;
9+
10+
private $b = 'hide';
11+
12+
protected $c = 'protect';
13+
14+
public function jsonSerialize(): mixed
15+
{
16+
$result = json_encode($this);
17+
var_dump($result);
18+
return $this;
19+
}
20+
}
21+
22+
var_dump(json_encode(new SerializingTest()));
23+
?>
24+
--EXPECT--
25+
bool(false)
26+
string(7) "{"a":1}"
27+
string(7) "{"a":1}"

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

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
--TEST--
2+
json_encode() Recursion test with JsonSerializable and var_dump simple
3+
--FILE--
4+
<?php
5+
6+
class SerializingTest implements JsonSerializable
7+
{
8+
public $a = 1;
9+
10+
public function jsonSerialize(): mixed
11+
{
12+
var_dump($this);
13+
return $this;
14+
}
15+
}
16+
17+
var_dump(json_encode(new SerializingTest()));
18+
19+
?>
20+
--EXPECT--
21+
object(SerializingTest)#1 (1) {
22+
["a"]=>
23+
int(1)
24+
}
25+
string(7) "{"a":1}"

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

+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
json_encode() Recursion test with JsonSerializable and __debugInfo
3+
--FILE--
4+
<?php
5+
6+
class SerializingTest implements JsonSerializable
7+
{
8+
public $a = 1;
9+
10+
public function __debugInfo()
11+
{
12+
return [ 'result' => json_encode($this) ];
13+
}
14+
15+
public function jsonSerialize(): mixed
16+
{
17+
var_dump($this);
18+
return $this;
19+
}
20+
}
21+
22+
var_dump(json_encode(new SerializingTest()));
23+
echo "---------\n";
24+
var_dump(new SerializingTest());
25+
26+
?>
27+
--EXPECT--
28+
*RECURSION*
29+
object(SerializingTest)#1 (1) {
30+
["result"]=>
31+
string(7) "{"a":1}"
32+
}
33+
string(7) "{"a":1}"
34+
---------
35+
*RECURSION*
36+
object(SerializingTest)#1 (1) {
37+
["result"]=>
38+
string(7) "{"a":1}"
39+
}

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

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
json_encode() Recursion test with JsonSerializable, __debugInfo and var_export
3+
--FILE--
4+
<?php
5+
6+
class SerializingTest implements JsonSerializable
7+
{
8+
public $a = 1;
9+
10+
public function __debugInfo()
11+
{
12+
return [ 'result' => var_export($this, true) ];
13+
}
14+
15+
public function jsonSerialize(): mixed
16+
{
17+
var_dump($this);
18+
return $this;
19+
}
20+
}
21+
22+
var_dump(json_encode(new SerializingTest()));
23+
echo "---------\n";
24+
var_dump(new SerializingTest());
25+
26+
?>
27+
--EXPECT--
28+
object(SerializingTest)#1 (1) {
29+
["result"]=>
30+
string(52) "\SerializingTest::__set_state(array(
31+
'a' => 1,
32+
))"
33+
}
34+
string(7) "{"a":1}"
35+
---------
36+
object(SerializingTest)#1 (1) {
37+
["result"]=>
38+
string(52) "\SerializingTest::__set_state(array(
39+
'a' => 1,
40+
))"
41+
}

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

+37
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
--TEST--
2+
json_encode() Recursion test with JsonSerializable, __debugInfo and print_r
3+
--FILE--
4+
<?php
5+
6+
class SerializingTest implements JsonSerializable
7+
{
8+
public $a = 1;
9+
10+
public function __debugInfo()
11+
{
12+
return [ 'result' => $this->a ];
13+
}
14+
15+
public function jsonSerialize(): mixed
16+
{
17+
print_r($this);
18+
return $this;
19+
}
20+
}
21+
22+
var_dump(json_encode(new SerializingTest()));
23+
echo "---------\n";
24+
var_dump(new SerializingTest());
25+
26+
?>
27+
--EXPECT--
28+
SerializingTest Object
29+
(
30+
[result] => 1
31+
)
32+
string(7) "{"a":1}"
33+
---------
34+
object(SerializingTest)#1 (1) {
35+
["result"]=>
36+
int(1)
37+
}

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

+41
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
--TEST--
2+
json_encode() Recursion test with JsonSerializable and serialize
3+
--FILE--
4+
<?php
5+
6+
class JsonEncodeFirstTest implements JsonSerializable
7+
{
8+
public $a = 1;
9+
10+
public function __serialize()
11+
{
12+
return [ 'result' => $this->a ];
13+
}
14+
15+
public function jsonSerialize(): mixed
16+
{
17+
return [ 'serialize' => serialize($this) ];
18+
}
19+
}
20+
21+
class SerializeFirstTest implements JsonSerializable
22+
{
23+
public $a = 1;
24+
25+
public function __serialize()
26+
{
27+
return [ 'result' => json_encode($this) ];
28+
}
29+
30+
public function jsonSerialize(): mixed
31+
{
32+
return [ 'json' => serialize($this) ];
33+
}
34+
}
35+
36+
var_dump(json_encode(new JsonEncodeFirstTest()));
37+
var_dump(serialize(new SerializeFirstTest()));
38+
?>
39+
--EXPECT--
40+
string(68) "{"serialize":"O:19:\"JsonEncodeFirstTest\":1:{s:6:\"result\";i:1;}"}"
41+
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;}\"}\";}"}";}"

Diff for: ‎ext/spl/tests/fixedarray_022.phpt

+3-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
--TEST--
22
SPL: FixedArray: Bug GH-8044 (var_export/debug_zval_dump HT_ASSERT_RC1 debug failure for SplFixedArray)
3+
--XFAIL--
4+
var_export now expect consistent properties
35
--FILE--
46
<?php
57
call_user_func(function () {
68
$x = new SplFixedArray(1);
79
$x[0] = $x;
8-
var_export($x); echo "\n";
10+
json_encode($x); echo "\n";
911
debug_zval_dump($x); echo "\n";
1012
});
1113
?>

Diff for: ‎ext/spl/tests/fixedarray_023.phpt

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
--TEST--
22
SPL: FixedArray: Infinite loop in var_export bugfix
3+
--XFAIL--
4+
var_export now expect consistent properties
35
--FILE--
46
<?php
57
call_user_func(function () {

Diff for: ‎ext/spl/tests/gh8044.phpt

+2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
--TEST--
22
Bug GH-8044 (var_export/debug_zval_dump HT_ASSERT_RC1 debug failure for SplFixedArray)
3+
--XFAIL--
4+
var_export now expect consistent properties
35
--FILE--
46
<?php
57
call_user_func(function () {

0 commit comments

Comments
 (0)