Skip to content

Commit f0f666b

Browse files
nielsdosnielsdos
committed
Fix GH-16601: Memory leak in Reflection constructors
Additionally fixes wrong behaviour in ReflectionParameter when you first have a construction that uses an object and the subsequent doesn't. Closes GH-16672.
1 parent 5253647 commit f0f666b

6 files changed

+130
-10
lines changed

‎NEWS

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,9 @@ PHP NEWS
9191
- PHPDBG:
9292
. Fixed bug GH-16174 (Empty string is an invalid expression for ev). (cmb)
9393

94+
- Reflection:
95+
. Fixed bug GH-16601 (Memory leak in Reflection constructors). (nielsdos)
96+
9497
- Session:
9598
. Fixed bug GH-16385 (Unexpected null returned by session_set_cookie_params).
9699
(nielsdos)

‎ext/reflection/php_reflection.c

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -217,18 +217,26 @@ static void _free_function(zend_function *fptr) /* {{{ */
217217
}
218218
/* }}} */
219219

220+
static void reflection_free_property_reference(property_reference *reference)
221+
{
222+
zend_string_release_ex(reference->unmangled_name, 0);
223+
efree(reference);
224+
}
225+
226+
static void reflection_free_parameter_reference(parameter_reference *reference)
227+
{
228+
_free_function(reference->fptr);
229+
efree(reference);
230+
}
231+
220232
static void reflection_free_objects_storage(zend_object *object) /* {{{ */
221233
{
222234
reflection_object *intern = reflection_object_from_obj(object);
223-
parameter_reference *reference;
224-
property_reference *prop_reference;
225235

226236
if (intern->ptr) {
227237
switch (intern->ref_type) {
228238
case REF_TYPE_PARAMETER:
229-
reference = (parameter_reference*)intern->ptr;
230-
_free_function(reference->fptr);
231-
efree(intern->ptr);
239+
reflection_free_parameter_reference(intern->ptr);
232240
break;
233241
case REF_TYPE_TYPE:
234242
{
@@ -243,9 +251,7 @@ static void reflection_free_objects_storage(zend_object *object) /* {{{ */
243251
_free_function(intern->ptr);
244252
break;
245253
case REF_TYPE_PROPERTY:
246-
prop_reference = (property_reference*)intern->ptr;
247-
zend_string_release_ex(prop_reference->unmangled_name, 0);
248-
efree(intern->ptr);
254+
reflection_free_property_reference(intern->ptr);
249255
break;
250256
case REF_TYPE_ATTRIBUTE: {
251257
attribute_reference *attr_ref = intern->ptr;
@@ -2546,6 +2552,10 @@ ZEND_METHOD(ReflectionParameter, __construct)
25462552
}
25472553
}
25482554

2555+
if (intern->ptr) {
2556+
reflection_free_parameter_reference(intern->ptr);
2557+
}
2558+
25492559
ref = (parameter_reference*) emalloc(sizeof(parameter_reference));
25502560
ref->arg_info = &arg_info[position];
25512561
ref->offset = (uint32_t)position;
@@ -2555,11 +2565,15 @@ ZEND_METHOD(ReflectionParameter, __construct)
25552565
intern->ptr = ref;
25562566
intern->ref_type = REF_TYPE_PARAMETER;
25572567
intern->ce = ce;
2568+
zval_ptr_dtor(&intern->obj);
25582569
if (reference && is_closure) {
25592570
ZVAL_COPY_VALUE(&intern->obj, reference);
2571+
} else {
2572+
ZVAL_UNDEF(&intern->obj);
25602573
}
25612574

25622575
prop_name = reflection_prop_name(object);
2576+
zval_ptr_dtor(prop_name);
25632577
if (has_internal_arg_info(fptr)) {
25642578
ZVAL_STRING(prop_name, ((zend_internal_arg_info*)arg_info)[position].name);
25652579
} else {
@@ -4015,10 +4029,12 @@ static void reflection_class_object_ctor(INTERNAL_FUNCTION_PARAMETERS, int is_ob
40154029
object = ZEND_THIS;
40164030
intern = Z_REFLECTION_P(object);
40174031

4032+
/* Note: class entry name is interned, no need to destroy them */
40184033
if (arg_obj) {
40194034
ZVAL_STR_COPY(reflection_prop_name(object), arg_obj->ce->name);
40204035
intern->ptr = arg_obj->ce;
40214036
if (is_object) {
4037+
zval_ptr_dtor(&intern->obj);
40224038
ZVAL_OBJ_COPY(&intern->obj, arg_obj);
40234039
}
40244040
} else {
@@ -5510,13 +5526,20 @@ ZEND_METHOD(ReflectionProperty, __construct)
55105526
}
55115527
}
55125528

5513-
ZVAL_STR_COPY(reflection_prop_name(object), name);
5529+
zval *prop_name = reflection_prop_name(object);
5530+
zval_ptr_dtor(prop_name);
5531+
ZVAL_STR_COPY(prop_name, name);
5532+
/* Note: class name are always interned, no need to destroy them */
55145533
if (dynam_prop == 0) {
55155534
ZVAL_STR_COPY(reflection_prop_class(object), property_info->ce->name);
55165535
} else {
55175536
ZVAL_STR_COPY(reflection_prop_class(object), ce->name);
55185537
}
55195538

5539+
if (intern->ptr) {
5540+
reflection_free_property_reference(intern->ptr);
5541+
}
5542+
55205543
reference = (property_reference*) emalloc(sizeof(property_reference));
55215544
reference->prop = dynam_prop ? NULL : property_info;
55225545
reference->unmangled_name = zend_string_copy(name);
@@ -5949,7 +5972,9 @@ ZEND_METHOD(ReflectionExtension, __construct)
59495972
RETURN_THROWS();
59505973
}
59515974
free_alloca(lcname, use_heap);
5952-
ZVAL_STRING(reflection_prop_name(object), module->name);
5975+
zval *prop_name = reflection_prop_name(object);
5976+
zval_ptr_dtor(prop_name);
5977+
ZVAL_STRING(prop_name, module->name);
59535978
intern->ptr = module;
59545979
intern->ref_type = REF_TYPE_OTHER;
59555980
intern->ce = NULL;

‎ext/reflection/tests/ReflectionExtension_double_construct.phpt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
--TEST--
2+
ReflectionExtension double construct call
3+
--FILE--
4+
<?php
5+
6+
$r = new ReflectionExtension('standard');
7+
var_dump($r);
8+
$r->__construct('standard');
9+
var_dump($r);
10+
11+
?>
12+
--EXPECT--
13+
object(ReflectionExtension)#1 (1) {
14+
["name"]=>
15+
string(8) "standard"
16+
}
17+
object(ReflectionExtension)#1 (1) {
18+
["name"]=>
19+
string(8) "standard"
20+
}

‎ext/reflection/tests/ReflectionObject_double_construct.phpt

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
--TEST--
2+
ReflectionObject double construct call
3+
--FILE--
4+
<?php
5+
6+
$obj = new stdClass;
7+
$r = new ReflectionObject($obj);
8+
var_dump($r);
9+
$r->__construct($obj);
10+
var_dump($r);
11+
12+
?>
13+
--EXPECT--
14+
object(ReflectionObject)#2 (1) {
15+
["name"]=>
16+
string(8) "stdClass"
17+
}
18+
object(ReflectionObject)#2 (1) {
19+
["name"]=>
20+
string(8) "stdClass"
21+
}

‎ext/reflection/tests/ReflectionParameter_double_construct.phpt

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
--TEST--
2+
ReflectionParameter double construct call
3+
--FILE--
4+
<?php
5+
6+
$closure = function (int $x): void {};
7+
$r = new ReflectionParameter($closure, 'x');
8+
var_dump($r);
9+
$r->__construct($closure, 'x');
10+
var_dump($r);
11+
$r->__construct('ord', 'character');
12+
var_dump($r);
13+
14+
?>
15+
--EXPECT--
16+
object(ReflectionParameter)#2 (1) {
17+
["name"]=>
18+
string(1) "x"
19+
}
20+
object(ReflectionParameter)#2 (1) {
21+
["name"]=>
22+
string(1) "x"
23+
}
24+
object(ReflectionParameter)#2 (1) {
25+
["name"]=>
26+
string(9) "character"
27+
}

‎ext/reflection/tests/ReflectionProperty_double_construct.phpt

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
--TEST--
2+
ReflectionProperty double construct call
3+
--FILE--
4+
<?php
5+
6+
$r = new ReflectionProperty(Exception::class, 'message');
7+
var_dump($r);
8+
$r->__construct(Exception::class, 'message');
9+
var_dump($r);
10+
11+
?>
12+
--EXPECT--
13+
object(ReflectionProperty)#1 (2) {
14+
["name"]=>
15+
string(7) "message"
16+
["class"]=>
17+
string(9) "Exception"
18+
}
19+
object(ReflectionProperty)#1 (2) {
20+
["name"]=>
21+
string(7) "message"
22+
["class"]=>
23+
string(9) "Exception"
24+
}

0 commit comments

Comments
 (0)