Skip to content

Commit 16cda10

Browse files
nielsdosnielsdos
committed
Merge branch 'PHP-8.2' into PHP-8.3
* PHP-8.2: Fix GH-16628: FPM logs are getting corrupted with this log statement Fix GH-16601: Memory leak in Reflection constructors
2 parents 65d4234 + e643129 commit 16cda10

11 files changed

+224
-13
lines changed

‎NEWS

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,10 @@ PHP NEWS
5656
- Filter:
5757
. Fixed bug GH-16523 (FILTER_FLAG_HOSTNAME accepts ending hyphen). (cmb)
5858

59+
- FPM:
60+
. Fixed bug GH-16628 (FPM logs are getting corrupted with this log
61+
statement). (nielsdos)
62+
5963
- GD:
6064
. Fixed bug GH-16334 (imageaffine overflow on matrix elements).
6165
(David Carlier)
@@ -97,6 +101,9 @@ PHP NEWS
97101
- PHPDBG:
98102
. Fixed bug GH-16174 (Empty string is an invalid expression for ev). (cmb)
99103

104+
- Reflection:
105+
. Fixed bug GH-16601 (Memory leak in Reflection constructors). (nielsdos)
106+
100107
- Session:
101108
. Fixed bug GH-16385 (Unexpected null returned by session_set_cookie_params).
102109
(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;
@@ -2521,6 +2527,10 @@ ZEND_METHOD(ReflectionParameter, __construct)
25212527
}
25222528
}
25232529

2530+
if (intern->ptr) {
2531+
reflection_free_parameter_reference(intern->ptr);
2532+
}
2533+
25242534
ref = (parameter_reference*) emalloc(sizeof(parameter_reference));
25252535
ref->arg_info = &arg_info[position];
25262536
ref->offset = (uint32_t)position;
@@ -2530,11 +2540,15 @@ ZEND_METHOD(ReflectionParameter, __construct)
25302540
intern->ptr = ref;
25312541
intern->ref_type = REF_TYPE_PARAMETER;
25322542
intern->ce = ce;
2543+
zval_ptr_dtor(&intern->obj);
25332544
if (reference && is_closure) {
25342545
ZVAL_COPY_VALUE(&intern->obj, reference);
2546+
} else {
2547+
ZVAL_UNDEF(&intern->obj);
25352548
}
25362549

25372550
prop_name = reflection_prop_name(object);
2551+
zval_ptr_dtor(prop_name);
25382552
if (has_internal_arg_info(fptr)) {
25392553
ZVAL_STRING(prop_name, ((zend_internal_arg_info*)arg_info)[position].name);
25402554
} else {
@@ -4032,10 +4046,12 @@ static void reflection_class_object_ctor(INTERNAL_FUNCTION_PARAMETERS, int is_ob
40324046
object = ZEND_THIS;
40334047
intern = Z_REFLECTION_P(object);
40344048

4049+
/* Note: class entry name is interned, no need to destroy them */
40354050
if (arg_obj) {
40364051
ZVAL_STR_COPY(reflection_prop_name(object), arg_obj->ce->name);
40374052
intern->ptr = arg_obj->ce;
40384053
if (is_object) {
4054+
zval_ptr_dtor(&intern->obj);
40394055
ZVAL_OBJ_COPY(&intern->obj, arg_obj);
40404056
}
40414057
} else {
@@ -5527,13 +5543,20 @@ ZEND_METHOD(ReflectionProperty, __construct)
55275543
}
55285544
}
55295545

5530-
ZVAL_STR_COPY(reflection_prop_name(object), name);
5546+
zval *prop_name = reflection_prop_name(object);
5547+
zval_ptr_dtor(prop_name);
5548+
ZVAL_STR_COPY(prop_name, name);
5549+
/* Note: class name are always interned, no need to destroy them */
55315550
if (dynam_prop == 0) {
55325551
ZVAL_STR_COPY(reflection_prop_class(object), property_info->ce->name);
55335552
} else {
55345553
ZVAL_STR_COPY(reflection_prop_class(object), ce->name);
55355554
}
55365555

5556+
if (intern->ptr) {
5557+
reflection_free_property_reference(intern->ptr);
5558+
}
5559+
55375560
reference = (property_reference*) emalloc(sizeof(property_reference));
55385561
reference->prop = dynam_prop ? NULL : property_info;
55395562
reference->unmangled_name = zend_string_copy(name);
@@ -5982,7 +6005,9 @@ ZEND_METHOD(ReflectionExtension, __construct)
59826005
RETURN_THROWS();
59836006
}
59846007
free_alloca(lcname, use_heap);
5985-
ZVAL_STRING(reflection_prop_name(object), module->name);
6008+
zval *prop_name = reflection_prop_name(object);
6009+
zval_ptr_dtor(prop_name);
6010+
ZVAL_STRING(prop_name, module->name);
59866011
intern->ptr = module;
59876012
intern->ref_type = REF_TYPE_OTHER;
59886013
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+
}

‎ext/zend_test/test.c

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -793,6 +793,17 @@ static ZEND_FUNCTION(zend_test_is_zend_ptr)
793793
RETURN_BOOL(is_zend_ptr((void*)addr));
794794
}
795795

796+
static ZEND_FUNCTION(zend_test_log_err_debug)
797+
{
798+
zend_string *str;
799+
800+
ZEND_PARSE_PARAMETERS_START(1, 1)
801+
Z_PARAM_STR(str);
802+
ZEND_PARSE_PARAMETERS_END();
803+
804+
php_log_err_with_severity(ZSTR_VAL(str), LOG_DEBUG);
805+
}
806+
796807
static zend_object *zend_test_class_new(zend_class_entry *class_type)
797808
{
798809
zend_object *obj = zend_objects_new(class_type);

‎ext/zend_test/test.stub.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,8 @@ function zend_test_set_fmode(bool $binary): void {}
260260
function zend_test_cast_fread($stream): void {}
261261

262262
function zend_test_is_zend_ptr(int $addr): bool {}
263+
264+
function zend_test_log_err_debug(string $str): void {}
263265
}
264266

265267
namespace ZendTestNS {

‎ext/zend_test/test_arginfo.h

Lines changed: 7 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎sapi/fpm/fpm/zlog.c

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ static inline void zlog_external(
153153
}
154154
/* }}} */
155155

156+
/* Returns the length if the print were complete, this can be larger than buf_size. */
156157
static size_t zlog_buf_prefix(
157158
const char *function, int line, int flags,
158159
char *buf, size_t buf_size, int use_syslog) /* {{{ */
@@ -189,6 +190,7 @@ static size_t zlog_buf_prefix(
189190
}
190191
}
191192

193+
/* Important: snprintf returns the number of bytes if the print were complete. */
192194
return len;
193195
}
194196
/* }}} */
@@ -411,6 +413,7 @@ static inline ssize_t zlog_stream_unbuffered_write(
411413
static inline ssize_t zlog_stream_buf_copy_cstr(
412414
struct zlog_stream *stream, const char *str, size_t str_len) /* {{{ */
413415
{
416+
ZEND_ASSERT(stream->len <= stream->buf.size);
414417
if (stream->buf.size - stream->len <= str_len &&
415418
!zlog_stream_buf_alloc_ex(stream, str_len + stream->len)) {
416419
return -1;
@@ -425,6 +428,7 @@ static inline ssize_t zlog_stream_buf_copy_cstr(
425428

426429
static inline ssize_t zlog_stream_buf_copy_char(struct zlog_stream *stream, char c) /* {{{ */
427430
{
431+
ZEND_ASSERT(stream->len <= stream->buf.size);
428432
if (stream->buf.size - stream->len < 1 && !zlog_stream_buf_alloc_ex(stream, 1)) {
429433
return -1;
430434
}
@@ -681,6 +685,17 @@ ssize_t zlog_stream_prefix_ex(struct zlog_stream *stream, const char *function,
681685
len = zlog_buf_prefix(
682686
function, line, stream->flags,
683687
stream->buf.data, stream->buf.size, stream->use_syslog);
688+
if (!EXPECTED(len + 1 <= stream->buf.size)) {
689+
/* If the buffer was not large enough, try with a larger buffer.
690+
* Note that this may still truncate if the zlog_limit is reached. */
691+
len = MIN(len + 1, zlog_limit);
692+
if (!zlog_stream_buf_alloc_ex(stream, len)) {
693+
return -1;
694+
}
695+
zlog_buf_prefix(
696+
function, line, stream->flags,
697+
stream->buf.data, stream->buf.size, stream->use_syslog);
698+
}
684699
stream->len = stream->prefix_len = len;
685700
if (stream->msg_prefix != NULL) {
686701
zlog_stream_buf_copy_cstr(stream, stream->msg_prefix, stream->msg_prefix_len);
@@ -692,8 +707,8 @@ ssize_t zlog_stream_prefix_ex(struct zlog_stream *stream, const char *function,
692707
} else {
693708
char sbuf[1024];
694709
ssize_t written;
695-
len = zlog_buf_prefix(function, line, stream->flags, sbuf, 1024, stream->use_syslog);
696-
written = zlog_stream_direct_write(stream, sbuf, len);
710+
len = zlog_buf_prefix(function, line, stream->flags, sbuf, sizeof(sbuf), stream->use_syslog);
711+
written = zlog_stream_direct_write(stream, sbuf, MIN(len, sizeof(sbuf)));
697712
if (stream->msg_prefix != NULL) {
698713
written += zlog_stream_direct_write(
699714
stream, stream->msg_prefix, stream->msg_prefix_len);

0 commit comments

Comments
 (0)