diff --git a/NEWS b/NEWS index a819612795f..bce75111ae7 100644 --- a/NEWS +++ b/NEWS @@ -56,6 +56,10 @@ PHP NEWS - Filter: . Fixed bug GH-16523 (FILTER_FLAG_HOSTNAME accepts ending hyphen). (cmb) +- FPM: + . Fixed bug GH-16628 (FPM logs are getting corrupted with this log + statement). (nielsdos) + - GD: . Fixed bug GH-16334 (imageaffine overflow on matrix elements). (David Carlier) @@ -97,6 +101,9 @@ PHP NEWS - PHPDBG: . Fixed bug GH-16174 (Empty string is an invalid expression for ev). (cmb) +- Reflection: + . Fixed bug GH-16601 (Memory leak in Reflection constructors). (nielsdos) + - Session: . Fixed bug GH-16385 (Unexpected null returned by session_set_cookie_params). (nielsdos) diff --git a/ext/reflection/php_reflection.c b/ext/reflection/php_reflection.c index e1d6b84c58e..8cecc108339 100644 --- a/ext/reflection/php_reflection.c +++ b/ext/reflection/php_reflection.c @@ -217,18 +217,26 @@ static void _free_function(zend_function *fptr) /* {{{ */ } /* }}} */ +static void reflection_free_property_reference(property_reference *reference) +{ + zend_string_release_ex(reference->unmangled_name, 0); + efree(reference); +} + +static void reflection_free_parameter_reference(parameter_reference *reference) +{ + _free_function(reference->fptr); + efree(reference); +} + static void reflection_free_objects_storage(zend_object *object) /* {{{ */ { reflection_object *intern = reflection_object_from_obj(object); - parameter_reference *reference; - property_reference *prop_reference; if (intern->ptr) { switch (intern->ref_type) { case REF_TYPE_PARAMETER: - reference = (parameter_reference*)intern->ptr; - _free_function(reference->fptr); - efree(intern->ptr); + reflection_free_parameter_reference(intern->ptr); break; case REF_TYPE_TYPE: { @@ -243,9 +251,7 @@ static void reflection_free_objects_storage(zend_object *object) /* {{{ */ _free_function(intern->ptr); break; case REF_TYPE_PROPERTY: - prop_reference = (property_reference*)intern->ptr; - zend_string_release_ex(prop_reference->unmangled_name, 0); - efree(intern->ptr); + reflection_free_property_reference(intern->ptr); break; case REF_TYPE_ATTRIBUTE: { attribute_reference *attr_ref = intern->ptr; @@ -2521,6 +2527,10 @@ ZEND_METHOD(ReflectionParameter, __construct) } } + if (intern->ptr) { + reflection_free_parameter_reference(intern->ptr); + } + ref = (parameter_reference*) emalloc(sizeof(parameter_reference)); ref->arg_info = &arg_info[position]; ref->offset = (uint32_t)position; @@ -2530,11 +2540,15 @@ ZEND_METHOD(ReflectionParameter, __construct) intern->ptr = ref; intern->ref_type = REF_TYPE_PARAMETER; intern->ce = ce; + zval_ptr_dtor(&intern->obj); if (reference && is_closure) { ZVAL_COPY_VALUE(&intern->obj, reference); + } else { + ZVAL_UNDEF(&intern->obj); } prop_name = reflection_prop_name(object); + zval_ptr_dtor(prop_name); if (has_internal_arg_info(fptr)) { ZVAL_STRING(prop_name, ((zend_internal_arg_info*)arg_info)[position].name); } else { @@ -4032,10 +4046,12 @@ static void reflection_class_object_ctor(INTERNAL_FUNCTION_PARAMETERS, int is_ob object = ZEND_THIS; intern = Z_REFLECTION_P(object); + /* Note: class entry name is interned, no need to destroy them */ if (arg_obj) { ZVAL_STR_COPY(reflection_prop_name(object), arg_obj->ce->name); intern->ptr = arg_obj->ce; if (is_object) { + zval_ptr_dtor(&intern->obj); ZVAL_OBJ_COPY(&intern->obj, arg_obj); } } else { @@ -5527,13 +5543,20 @@ ZEND_METHOD(ReflectionProperty, __construct) } } - ZVAL_STR_COPY(reflection_prop_name(object), name); + zval *prop_name = reflection_prop_name(object); + zval_ptr_dtor(prop_name); + ZVAL_STR_COPY(prop_name, name); + /* Note: class name are always interned, no need to destroy them */ if (dynam_prop == 0) { ZVAL_STR_COPY(reflection_prop_class(object), property_info->ce->name); } else { ZVAL_STR_COPY(reflection_prop_class(object), ce->name); } + if (intern->ptr) { + reflection_free_property_reference(intern->ptr); + } + reference = (property_reference*) emalloc(sizeof(property_reference)); reference->prop = dynam_prop ? NULL : property_info; reference->unmangled_name = zend_string_copy(name); @@ -5982,7 +6005,9 @@ ZEND_METHOD(ReflectionExtension, __construct) RETURN_THROWS(); } free_alloca(lcname, use_heap); - ZVAL_STRING(reflection_prop_name(object), module->name); + zval *prop_name = reflection_prop_name(object); + zval_ptr_dtor(prop_name); + ZVAL_STRING(prop_name, module->name); intern->ptr = module; intern->ref_type = REF_TYPE_OTHER; intern->ce = NULL; diff --git a/ext/reflection/tests/ReflectionExtension_double_construct.phpt b/ext/reflection/tests/ReflectionExtension_double_construct.phpt new file mode 100644 index 00000000000..7bff11c4b17 --- /dev/null +++ b/ext/reflection/tests/ReflectionExtension_double_construct.phpt @@ -0,0 +1,20 @@ +--TEST-- +ReflectionExtension double construct call +--FILE-- +__construct('standard'); +var_dump($r); + +?> +--EXPECT-- +object(ReflectionExtension)#1 (1) { + ["name"]=> + string(8) "standard" +} +object(ReflectionExtension)#1 (1) { + ["name"]=> + string(8) "standard" +} diff --git a/ext/reflection/tests/ReflectionObject_double_construct.phpt b/ext/reflection/tests/ReflectionObject_double_construct.phpt new file mode 100644 index 00000000000..536b145243b --- /dev/null +++ b/ext/reflection/tests/ReflectionObject_double_construct.phpt @@ -0,0 +1,21 @@ +--TEST-- +ReflectionObject double construct call +--FILE-- +__construct($obj); +var_dump($r); + +?> +--EXPECT-- +object(ReflectionObject)#2 (1) { + ["name"]=> + string(8) "stdClass" +} +object(ReflectionObject)#2 (1) { + ["name"]=> + string(8) "stdClass" +} diff --git a/ext/reflection/tests/ReflectionParameter_double_construct.phpt b/ext/reflection/tests/ReflectionParameter_double_construct.phpt new file mode 100644 index 00000000000..cc69e6b10e3 --- /dev/null +++ b/ext/reflection/tests/ReflectionParameter_double_construct.phpt @@ -0,0 +1,27 @@ +--TEST-- +ReflectionParameter double construct call +--FILE-- +__construct($closure, 'x'); +var_dump($r); +$r->__construct('ord', 'character'); +var_dump($r); + +?> +--EXPECT-- +object(ReflectionParameter)#2 (1) { + ["name"]=> + string(1) "x" +} +object(ReflectionParameter)#2 (1) { + ["name"]=> + string(1) "x" +} +object(ReflectionParameter)#2 (1) { + ["name"]=> + string(9) "character" +} diff --git a/ext/reflection/tests/ReflectionProperty_double_construct.phpt b/ext/reflection/tests/ReflectionProperty_double_construct.phpt new file mode 100644 index 00000000000..086f2f781a5 --- /dev/null +++ b/ext/reflection/tests/ReflectionProperty_double_construct.phpt @@ -0,0 +1,24 @@ +--TEST-- +ReflectionProperty double construct call +--FILE-- +__construct(Exception::class, 'message'); +var_dump($r); + +?> +--EXPECT-- +object(ReflectionProperty)#1 (2) { + ["name"]=> + string(7) "message" + ["class"]=> + string(9) "Exception" +} +object(ReflectionProperty)#1 (2) { + ["name"]=> + string(7) "message" + ["class"]=> + string(9) "Exception" +} diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 472ea12d92e..e2e697c89cc 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -793,6 +793,17 @@ static ZEND_FUNCTION(zend_test_is_zend_ptr) RETURN_BOOL(is_zend_ptr((void*)addr)); } +static ZEND_FUNCTION(zend_test_log_err_debug) +{ + zend_string *str; + + ZEND_PARSE_PARAMETERS_START(1, 1) + Z_PARAM_STR(str); + ZEND_PARSE_PARAMETERS_END(); + + php_log_err_with_severity(ZSTR_VAL(str), LOG_DEBUG); +} + static zend_object *zend_test_class_new(zend_class_entry *class_type) { zend_object *obj = zend_objects_new(class_type); diff --git a/ext/zend_test/test.stub.php b/ext/zend_test/test.stub.php index f8ef25f9204..c9477eef527 100644 --- a/ext/zend_test/test.stub.php +++ b/ext/zend_test/test.stub.php @@ -260,6 +260,8 @@ function zend_test_override_libxml_global_state(): void {} function zend_test_cast_fread($stream): void {} function zend_test_is_zend_ptr(int $addr): bool {} + + function zend_test_log_err_debug(string $str): void {} } namespace ZendTestNS { diff --git a/ext/zend_test/test_arginfo.h b/ext/zend_test/test_arginfo.h index 7d0343c7e8c..5947a6587bb 100644 --- a/ext/zend_test/test_arginfo.h +++ b/ext/zend_test/test_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 4b349d5ac8e8daba565b57a091fc463d3132d37d */ + * Stub hash: 9ddaf4d659226c55d49221c71702fa373d42695e */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0) ZEND_END_ARG_INFO() @@ -158,6 +158,10 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_is_zend_ptr, 0, 1, _IS ZEND_ARG_TYPE_INFO(0, addr, IS_LONG, 0) ZEND_END_ARG_INFO() +ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_log_err_debug, 0, 1, IS_VOID, 0) + ZEND_ARG_TYPE_INFO(0, str, IS_STRING, 0) +ZEND_END_ARG_INFO() + #define arginfo_ZendTestNS2_namespaced_func arginfo_zend_test_is_pcre_bundled #define arginfo_ZendTestNS2_namespaced_deprecated_func arginfo_zend_test_void_return @@ -287,6 +291,7 @@ static ZEND_FUNCTION(zend_test_set_fmode); #endif static ZEND_FUNCTION(zend_test_cast_fread); static ZEND_FUNCTION(zend_test_is_zend_ptr); +static ZEND_FUNCTION(zend_test_log_err_debug); static ZEND_FUNCTION(ZendTestNS2_namespaced_func); static ZEND_FUNCTION(ZendTestNS2_namespaced_deprecated_func); static ZEND_FUNCTION(ZendTestNS2_ZendSubNS_namespaced_func); @@ -366,6 +371,7 @@ static const zend_function_entry ext_functions[] = { #endif ZEND_FE(zend_test_cast_fread, arginfo_zend_test_cast_fread) ZEND_FE(zend_test_is_zend_ptr, arginfo_zend_test_is_zend_ptr) + ZEND_FE(zend_test_log_err_debug, arginfo_zend_test_log_err_debug) ZEND_NS_FALIAS("ZendTestNS2", namespaced_func, ZendTestNS2_namespaced_func, arginfo_ZendTestNS2_namespaced_func) ZEND_NS_DEP_FALIAS("ZendTestNS2", namespaced_deprecated_func, ZendTestNS2_namespaced_deprecated_func, arginfo_ZendTestNS2_namespaced_deprecated_func) ZEND_NS_FALIAS("ZendTestNS2", namespaced_aliased_func, zend_test_void_return, arginfo_ZendTestNS2_namespaced_aliased_func) diff --git a/sapi/fpm/fpm/zlog.c b/sapi/fpm/fpm/zlog.c index 3903c6eb241..39c6eec885b 100644 --- a/sapi/fpm/fpm/zlog.c +++ b/sapi/fpm/fpm/zlog.c @@ -153,6 +153,7 @@ static inline void zlog_external( } /* }}} */ +/* Returns the length if the print were complete, this can be larger than buf_size. */ static size_t zlog_buf_prefix( const char *function, int line, int flags, char *buf, size_t buf_size, int use_syslog) /* {{{ */ @@ -189,6 +190,7 @@ static size_t zlog_buf_prefix( } } + /* Important: snprintf returns the number of bytes if the print were complete. */ return len; } /* }}} */ @@ -411,6 +413,7 @@ static inline ssize_t zlog_stream_unbuffered_write( static inline ssize_t zlog_stream_buf_copy_cstr( struct zlog_stream *stream, const char *str, size_t str_len) /* {{{ */ { + ZEND_ASSERT(stream->len <= stream->buf.size); if (stream->buf.size - stream->len <= str_len && !zlog_stream_buf_alloc_ex(stream, str_len + stream->len)) { return -1; @@ -425,6 +428,7 @@ static inline ssize_t zlog_stream_buf_copy_cstr( static inline ssize_t zlog_stream_buf_copy_char(struct zlog_stream *stream, char c) /* {{{ */ { + ZEND_ASSERT(stream->len <= stream->buf.size); if (stream->buf.size - stream->len < 1 && !zlog_stream_buf_alloc_ex(stream, 1)) { return -1; } @@ -681,6 +685,17 @@ ssize_t zlog_stream_prefix_ex(struct zlog_stream *stream, const char *function, len = zlog_buf_prefix( function, line, stream->flags, stream->buf.data, stream->buf.size, stream->use_syslog); + if (!EXPECTED(len + 1 <= stream->buf.size)) { + /* If the buffer was not large enough, try with a larger buffer. + * Note that this may still truncate if the zlog_limit is reached. */ + len = MIN(len + 1, zlog_limit); + if (!zlog_stream_buf_alloc_ex(stream, len)) { + return -1; + } + zlog_buf_prefix( + function, line, stream->flags, + stream->buf.data, stream->buf.size, stream->use_syslog); + } stream->len = stream->prefix_len = len; if (stream->msg_prefix != NULL) { 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, } else { char sbuf[1024]; ssize_t written; - len = zlog_buf_prefix(function, line, stream->flags, sbuf, 1024, stream->use_syslog); - written = zlog_stream_direct_write(stream, sbuf, len); + len = zlog_buf_prefix(function, line, stream->flags, sbuf, sizeof(sbuf), stream->use_syslog); + written = zlog_stream_direct_write(stream, sbuf, MIN(len, sizeof(sbuf))); if (stream->msg_prefix != NULL) { written += zlog_stream_direct_write( stream, stream->msg_prefix, stream->msg_prefix_len); diff --git a/sapi/fpm/tests/gh16628.phpt b/sapi/fpm/tests/gh16628.phpt new file mode 100644 index 00000000000..e2df0c8cb84 --- /dev/null +++ b/sapi/fpm/tests/gh16628.phpt @@ -0,0 +1,53 @@ +--TEST-- +GH-16628 (FPM logs are getting corrupted with this log statement) +--EXTENSIONS-- +zend_test +--SKIPIF-- + +--FILE-- +start(); +$tester->expectLogStartNotices(); +$tester->request()->expectEmptyBody(); +for ($i = 1; $i < 100; $i++) { + $tester->expectLogNotice("%sPHP message: " . str_repeat("a", $i)); +} +$tester->terminate(); +$tester->expectLogTerminatingNotices(); +$tester->close(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- +