Skip to content

Commit 53aa53f

Browse files
bukkabukka
committed
Introduce Zend guard recursion protection
This PR introduces a new way of recursion protection in JSON, var_dump and friends. It fixes issue in master for __debugInfo and also improves perf for jsonSerializable in some cases. More info can be found in phpGH-10020. Closes phpGH-11812
1 parent fd462b1 commit 53aa53f

16 files changed

+322
-37
lines changed

‎NEWS

+2
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ PHP NEWS
44

55
- Core:
66
. Fixed bug GH-11937 (Constant ASTs containing objects). (ilutov)
7+
. Introduced Zend guard recursion protection to fix __debugInfo issue.
8+
(Jakub Zelenka)
79

810
17 Aug 2023, PHP 8.3.0beta3
911

‎Zend/zend.c

+4-3
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,7 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /*
546546
HashTable *properties;
547547

548548
zend_object *zobj = Z_OBJ_P(expr);
549+
uint32_t *guard = zend_get_recursion_guard(zobj);
549550
zend_string *class_name = Z_OBJ_HANDLER_P(expr, get_class_name)(zobj);
550551
smart_str_appends(buf, ZSTR_VAL(class_name));
551552
zend_string_release_ex(class_name, 0);
@@ -561,7 +562,7 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /*
561562
smart_str_appendc(buf, '\n');
562563
}
563564

564-
if (GC_IS_RECURSIVE(Z_OBJ_P(expr))) {
565+
if (ZEND_GUARD_OR_GC_IS_RECURSIVE(guard, DEBUG, zobj)) {
565566
smart_str_appends(buf, " *RECURSION*");
566567
return;
567568
}
@@ -571,9 +572,9 @@ static void zend_print_zval_r_to_buf(smart_str *buf, zval *expr, int indent) /*
571572
break;
572573
}
573574

574-
GC_PROTECT_RECURSION(Z_OBJ_P(expr));
575+
ZEND_GUARD_OR_GC_PROTECT_RECURSION(guard, DEBUG, zobj);
575576
print_hash(buf, properties, indent, 1);
576-
GC_UNPROTECT_RECURSION(Z_OBJ_P(expr));
577+
ZEND_GUARD_OR_GC_UNPROTECT_RECURSION(guard, DEBUG, zobj);
577578

578579
zend_release_properties(properties);
579580
break;

‎Zend/zend_API.c

+1
Original file line numberDiff line numberDiff line change
@@ -2746,6 +2746,7 @@ ZEND_API void zend_add_magic_method(zend_class_entry *ce, zend_function *fptr, z
27462746
ce->__tostring = fptr;
27472747
} else if (zend_string_equals_literal(lcname, ZEND_DEBUGINFO_FUNC_NAME)) {
27482748
ce->__debugInfo = fptr;
2749+
ce->ce_flags |= ZEND_ACC_USE_GUARDS;
27492750
} else if (zend_string_equals_literal(lcname, "__serialize")) {
27502751
ce->__serialize = fptr;
27512752
} else if (zend_string_equals_literal(lcname, "__unserialize")) {

‎Zend/zend_object_handlers.c

+26-11
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@
3636
#define ZEND_WRONG_PROPERTY_OFFSET 0
3737

3838
/* guard flags */
39-
#define IN_GET (1<<0)
40-
#define IN_SET (1<<1)
41-
#define IN_UNSET (1<<2)
42-
#define IN_ISSET (1<<3)
39+
#define IN_GET ZEND_GUARD_PROPERTY_GET
40+
#define IN_SET ZEND_GUARD_PROPERTY_SET
41+
#define IN_UNSET ZEND_GUARD_PROPERTY_UNSET
42+
#define IN_ISSET ZEND_GUARD_PROPERTY_ISSET
4343

4444
/*
4545
__X accessors explanation:
@@ -542,30 +542,36 @@ static void zend_property_guard_dtor(zval *el) /* {{{ */ {
542542
}
543543
/* }}} */
544544

545+
static zend_always_inline zval *zend_get_guard_value(zend_object *zobj)
546+
{
547+
return zobj->properties_table + zobj->ce->default_properties_count;
548+
}
549+
545550
ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *member) /* {{{ */
546551
{
547552
HashTable *guards;
548553
zval *zv;
549554
uint32_t *ptr;
550555

556+
551557
ZEND_ASSERT(zobj->ce->ce_flags & ZEND_ACC_USE_GUARDS);
552-
zv = zobj->properties_table + zobj->ce->default_properties_count;
558+
zv = zend_get_guard_value(zobj);
553559
if (EXPECTED(Z_TYPE_P(zv) == IS_STRING)) {
554560
zend_string *str = Z_STR_P(zv);
555561
if (EXPECTED(str == member) ||
556562
/* str and member don't necessarily have a pre-calculated hash value here */
557563
EXPECTED(zend_string_equal_content(str, member))) {
558-
return &Z_PROPERTY_GUARD_P(zv);
559-
} else if (EXPECTED(Z_PROPERTY_GUARD_P(zv) == 0)) {
564+
return &Z_GUARD_P(zv);
565+
} else if (EXPECTED(Z_GUARD_P(zv) == 0)) {
560566
zval_ptr_dtor_str(zv);
561567
ZVAL_STR_COPY(zv, member);
562-
return &Z_PROPERTY_GUARD_P(zv);
568+
return &Z_GUARD_P(zv);
563569
} else {
564570
ALLOC_HASHTABLE(guards);
565571
zend_hash_init(guards, 8, NULL, zend_property_guard_dtor, 0);
566572
/* mark pointer as "special" using low bit */
567573
zend_hash_add_new_ptr(guards, str,
568-
(void*)(((uintptr_t)&Z_PROPERTY_GUARD_P(zv)) | 1));
574+
(void*)(((uintptr_t)&Z_GUARD_P(zv)) | 1));
569575
zval_ptr_dtor_str(zv);
570576
ZVAL_ARR(zv, guards);
571577
}
@@ -579,8 +585,8 @@ ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *membe
579585
} else {
580586
ZEND_ASSERT(Z_TYPE_P(zv) == IS_UNDEF);
581587
ZVAL_STR_COPY(zv, member);
582-
Z_PROPERTY_GUARD_P(zv) = 0;
583-
return &Z_PROPERTY_GUARD_P(zv);
588+
Z_GUARD_P(zv) &= ~ZEND_GUARD_PROPERTY_MASK;
589+
return &Z_GUARD_P(zv);
584590
}
585591
/* we have to allocate uint32_t separately because ht->arData may be reallocated */
586592
ptr = (uint32_t*)emalloc(sizeof(uint32_t));
@@ -589,6 +595,15 @@ ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *membe
589595
}
590596
/* }}} */
591597

598+
ZEND_API uint32_t *zend_get_recursion_guard(zend_object *zobj)
599+
{
600+
if (!(zobj->ce->ce_flags & ZEND_ACC_USE_GUARDS)) {
601+
return NULL;
602+
}
603+
zval *zv = zend_get_guard_value(zobj);
604+
return &Z_GUARD_P(zv);
605+
}
606+
592607
ZEND_API zval *zend_std_read_property(zend_object *zobj, zend_string *name, int type, void **cache_slot, zval *rv) /* {{{ */
593608
{
594609
zval *retval;

‎Zend/zend_object_handlers.h

+4
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,10 @@ ZEND_API zend_function *zend_get_call_trampoline_func(const zend_class_entry *ce
241241

242242
ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *member);
243243

244+
ZEND_API uint32_t *zend_get_property_guard(zend_object *zobj, zend_string *member);
245+
246+
ZEND_API uint32_t *zend_get_recursion_guard(zend_object *zobj);
247+
244248
/* Default behavior for get_properties_for. For use as a fallback in custom
245249
* get_properties_for implementations. */
246250
ZEND_API HashTable *zend_std_get_properties_for(zend_object *obj, zend_prop_purpose purpose);

‎Zend/zend_objects.c

+3-1
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ static zend_always_inline void _zend_object_std_init(zend_object *object, zend_c
3535
object->properties = NULL;
3636
zend_objects_store_put(object);
3737
if (UNEXPECTED(ce->ce_flags & ZEND_ACC_USE_GUARDS)) {
38-
ZVAL_UNDEF(object->properties_table + object->ce->default_properties_count);
38+
zval *guard_value = object->properties_table + object->ce->default_properties_count;
39+
ZVAL_UNDEF(guard_value);
40+
Z_GUARD_P(guard_value) = 0;
3941
}
4042
}
4143

‎Zend/zend_types.h

+38-3
Original file line numberDiff line numberDiff line change
@@ -345,7 +345,7 @@ struct _zval_struct {
345345
uint32_t num_args; /* arguments number for EX(This) */
346346
uint32_t fe_pos; /* foreach position */
347347
uint32_t fe_iter_idx; /* foreach iterator index */
348-
uint32_t property_guard; /* single property guard */
348+
uint32_t guard; /* recursion and single property guard */
349349
uint32_t constant_flags; /* constant flags */
350350
uint32_t extra; /* not further specified */
351351
} u2;
@@ -619,6 +619,22 @@ struct _zend_ast_ref {
619619
#define _IS_BOOL 18
620620
#define _IS_NUMBER 19
621621

622+
/* guard flags */
623+
#define ZEND_GUARD_PROPERTY_GET (1<<0)
624+
#define ZEND_GUARD_PROPERTY_SET (1<<1)
625+
#define ZEND_GUARD_PROPERTY_UNSET (1<<2)
626+
#define ZEND_GUARD_PROPERTY_ISSET (1<<3)
627+
#define ZEND_GUARD_PROPERTY_MASK 15
628+
#define ZEND_GUARD_RECURSION_DEBUG (1<<4)
629+
#define ZEND_GUARD_RECURSION_EXPORT (1<<5)
630+
#define ZEND_GUARD_RECURSION_JSON (1<<6)
631+
632+
#define ZEND_GUARD_RECURSION_TYPE(t) ZEND_GUARD_RECURSION_ ## t
633+
634+
#define ZEND_GUARD_IS_RECURSIVE(pg, t) ((*pg & ZEND_GUARD_RECURSION_TYPE(t)) != 0)
635+
#define ZEND_GUARD_PROTECT_RECURSION(pg, t) *pg |= ZEND_GUARD_RECURSION_TYPE(t)
636+
#define ZEND_GUARD_UNPROTECT_RECURSION(pg, t) *pg &= ~ZEND_GUARD_RECURSION_TYPE(t)
637+
622638
static zend_always_inline uint8_t zval_get_type(const zval* pz) {
623639
return pz->u1.v.type;
624640
}
@@ -659,8 +675,8 @@ static zend_always_inline uint8_t zval_get_type(const zval* pz) {
659675
#define Z_FE_ITER(zval) (zval).u2.fe_iter_idx
660676
#define Z_FE_ITER_P(zval_p) Z_FE_ITER(*(zval_p))
661677

662-
#define Z_PROPERTY_GUARD(zval) (zval).u2.property_guard
663-
#define Z_PROPERTY_GUARD_P(zval_p) Z_PROPERTY_GUARD(*(zval_p))
678+
#define Z_GUARD(zval) (zval).u2.guard
679+
#define Z_GUARD_P(zval_p) Z_GUARD(*(zval_p))
664680

665681
#define Z_CONSTANT_FLAGS(zval) (zval).u2.constant_flags
666682
#define Z_CONSTANT_FLAGS_P(zval_p) Z_CONSTANT_FLAGS(*(zval_p))
@@ -859,6 +875,25 @@ static zend_always_inline uint32_t zval_gc_info(uint32_t gc_type_info) {
859875
#define Z_PROTECT_RECURSION_P(zv) Z_PROTECT_RECURSION(*(zv))
860876
#define Z_UNPROTECT_RECURSION_P(zv) Z_UNPROTECT_RECURSION(*(zv))
861877

878+
#define ZEND_GUARD_OR_GC_IS_RECURSIVE(pg, t, zobj) \
879+
(pg ? ZEND_GUARD_IS_RECURSIVE(pg, t) : GC_IS_RECURSIVE(zobj))
880+
881+
#define ZEND_GUARD_OR_GC_PROTECT_RECURSION(pg, t, zobj) do { \
882+
if (pg) { \
883+
ZEND_GUARD_PROTECT_RECURSION(pg, t); \
884+
} else { \
885+
GC_PROTECT_RECURSION(zobj); \
886+
} \
887+
} while(0)
888+
889+
#define ZEND_GUARD_OR_GC_UNPROTECT_RECURSION(pg, t, zobj) do { \
890+
if (pg) { \
891+
ZEND_GUARD_UNPROTECT_RECURSION(pg, t); \
892+
} else { \
893+
GC_UNPROTECT_RECURSION(zobj); \
894+
} \
895+
} while(0)
896+
862897
/* All data types < IS_STRING have their constructor/destructors skipped */
863898
#define Z_CONSTANT(zval) (Z_TYPE(zval) == IS_CONSTANT_AST)
864899
#define Z_CONSTANT_P(zval_p) Z_CONSTANT(*(zval_p))

‎ext/json/json.c

+7
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,17 @@ PHP_JSON_API zend_class_entry *php_json_exception_ce;
3737

3838
PHP_JSON_API ZEND_DECLARE_MODULE_GLOBALS(json)
3939

40+
static int php_json_implement_json_serializable(zend_class_entry *interface, zend_class_entry *class_type)
41+
{
42+
class_type->ce_flags |= ZEND_ACC_USE_GUARDS;
43+
return SUCCESS;
44+
}
45+
4046
/* {{{ MINIT */
4147
static PHP_MINIT_FUNCTION(json)
4248
{
4349
php_json_serializable_ce = register_class_JsonSerializable();
50+
php_json_serializable_ce->interface_gets_implemented = php_json_implement_json_serializable;
4451

4552
php_json_exception_ce = register_class_JsonException(zend_ce_exception);
4653

‎ext/json/json_encoder.c

+10-7
Original file line numberDiff line numberDiff line change
@@ -531,19 +531,22 @@ zend_result php_json_escape_string(
531531
static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val, int options, php_json_encoder *encoder) /* {{{ */
532532
{
533533
zend_class_entry *ce = Z_OBJCE_P(val);
534-
HashTable* myht = Z_OBJPROP_P(val);
534+
zend_object *obj = Z_OBJ_P(val);
535+
uint32_t *guard = zend_get_recursion_guard(obj);
535536
zval retval, fname;
536537
zend_result return_code;
537538

538-
if (myht && GC_IS_RECURSIVE(myht)) {
539+
ZEND_ASSERT(guard != NULL);
540+
541+
if (ZEND_GUARD_IS_RECURSIVE(guard, JSON)) {
539542
encoder->error_code = PHP_JSON_ERROR_RECURSION;
540543
if (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR) {
541544
smart_str_appendl(buf, "null", 4);
542545
}
543546
return FAILURE;
544547
}
545548

546-
PHP_JSON_HASH_PROTECT_RECURSION(myht);
549+
ZEND_GUARD_PROTECT_RECURSION(guard, JSON);
547550

548551
ZVAL_STRING(&fname, "jsonSerialize");
549552

@@ -556,7 +559,7 @@ static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val
556559
if (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR) {
557560
smart_str_appendl(buf, "null", 4);
558561
}
559-
PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
562+
ZEND_GUARD_UNPROTECT_RECURSION(guard, JSON);
560563
return FAILURE;
561564
}
562565

@@ -568,19 +571,19 @@ static zend_result php_json_encode_serializable_object(smart_str *buf, zval *val
568571
if (options & PHP_JSON_PARTIAL_OUTPUT_ON_ERROR) {
569572
smart_str_appendl(buf, "null", 4);
570573
}
571-
PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
574+
ZEND_GUARD_UNPROTECT_RECURSION(guard, JSON);
572575
return FAILURE;
573576
}
574577

575578
if ((Z_TYPE(retval) == IS_OBJECT) &&
576579
(Z_OBJ(retval) == Z_OBJ_P(val))) {
577580
/* Handle the case where jsonSerialize does: return $this; by going straight to encode array */
578-
PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
581+
ZEND_GUARD_UNPROTECT_RECURSION(guard, JSON);
579582
return_code = php_json_encode_array(buf, &retval, options, encoder);
580583
} else {
581584
/* All other types, encode as normal */
582585
return_code = php_json_encode_zval(buf, &retval, options, encoder);
583-
PHP_JSON_HASH_UNPROTECT_RECURSION(myht);
586+
ZEND_GUARD_UNPROTECT_RECURSION(guard, JSON);
584587
}
585588

586589
zval_ptr_dtor(&retval);

‎ext/json/tests/json_encode_recursion_01.phpt

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
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}"

‎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}"

‎ext/json/tests/json_encode_recursion_03.phpt

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
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+
object(SerializingTest)#1 (1) {
29+
["result"]=>
30+
bool(false)
31+
}
32+
string(7) "{"a":1}"
33+
---------
34+
*RECURSION*
35+
object(SerializingTest)#1 (1) {
36+
["result"]=>
37+
string(7) "{"a":1}"
38+
}

0 commit comments

Comments
 (0)