From 49faa4df7567f92f73a025ded949568f611d4f83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 30 Aug 2025 21:04:50 +0200 Subject: [PATCH 1/8] uri: Always use `const` pointers when referring to `uri_parser_t` (#19623) The actual parser definitions are all `const` and must never be modified. Make sure to always use `const` pointers. --- ext/filter/logical_filters.c | 2 +- ext/openssl/xp_ssl.c | 2 +- ext/soap/php_http.c | 4 ++-- ext/standard/ftp_fopen_wrapper.c | 4 ++-- ext/standard/http_fopen_wrapper.c | 2 +- ext/uri/php_uri.c | 4 ++-- ext/uri/php_uri.h | 2 +- ext/zend_test/test.c | 2 +- main/streams/php_stream_context.h | 2 +- main/streams/streams.c | 2 +- 10 files changed, 13 insertions(+), 13 deletions(-) diff --git a/ext/filter/logical_filters.c b/ext/filter/logical_filters.c index def5331720d43..e6fbaf86b370a 100644 --- a/ext/filter/logical_filters.c +++ b/ext/filter/logical_filters.c @@ -612,7 +612,7 @@ zend_result php_filter_validate_url(PHP_INPUT_FILTER_PARAM_DECL) /* {{{ */ int parser_name_set; FETCH_STR_OPTION(parser_name, URL_OPTION_URI_PARSER_CLASS); - uri_parser_t *uri_parser = php_uri_get_parser(parser_name_set ? parser_name : NULL); + const uri_parser_t *uri_parser = php_uri_get_parser(parser_name_set ? parser_name : NULL); if (uri_parser == NULL) { zend_value_error("%s(): \"uri_parser_class\" option has invalid value", get_active_function_name()); RETURN_VALIDATION_FAILED diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 6fbdb506133c7..e76f99de6df6f 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -2634,7 +2634,7 @@ static char *php_openssl_get_url_name(const char *resourcename, return NULL; } - uri_parser_t *uri_parser = php_stream_context_get_uri_parser("ssl", context); + const uri_parser_t *uri_parser = php_stream_context_get_uri_parser("ssl", context); if (uri_parser == NULL) { zend_value_error("%s(): Provided stream context has invalid value for the \"uri_parser_class\" option", get_active_function_name()); return NULL; diff --git a/ext/soap/php_http.c b/ext/soap/php_http.c index 854db4c928cbf..71082aef7f902 100644 --- a/ext/soap/php_http.c +++ b/ext/soap/php_http.c @@ -431,7 +431,7 @@ int make_http_soap_request( } if (location != NULL && ZSTR_VAL(location)[0] != '\000') { - uri_parser_t *uri_parser = php_uri_get_parser(uri_parser_class); + const uri_parser_t *uri_parser = php_uri_get_parser(uri_parser_class); if (uri_parser == NULL) { zend_argument_value_error(6, "must be a valid URI parser name"); return FALSE; @@ -1148,7 +1148,7 @@ int make_http_soap_request( char *loc; if ((loc = get_http_header_value(ZSTR_VAL(http_headers), "Location:")) != NULL) { - uri_parser_t *uri_parser = php_uri_get_parser(uri_parser_class); + const uri_parser_t *uri_parser = php_uri_get_parser(uri_parser_class); if (uri_parser == NULL) { efree(loc); zend_argument_value_error(6, "must be a valid URI parser name"); diff --git a/ext/standard/ftp_fopen_wrapper.c b/ext/standard/ftp_fopen_wrapper.c index 0a3b261d39c24..f4508b4657c10 100644 --- a/ext/standard/ftp_fopen_wrapper.c +++ b/ext/standard/ftp_fopen_wrapper.c @@ -135,7 +135,7 @@ static php_stream *php_ftp_fopen_connect(php_stream_wrapper *wrapper, const char char *transport; int transport_len; - uri_parser_t *uri_parser = php_stream_context_get_uri_parser("ftp", context); + const uri_parser_t *uri_parser = php_stream_context_get_uri_parser("ftp", context); if (uri_parser == NULL) { zend_value_error("%s(): Provided stream context has invalid value for the \"uri_parser_class\" option", get_active_function_name()); return NULL; @@ -956,7 +956,7 @@ static int php_stream_ftp_rename(php_stream_wrapper *wrapper, const char *url_fr int result; char tmp_line[512]; - uri_parser_t *uri_parser = php_stream_context_get_uri_parser("ftp", context); + const uri_parser_t *uri_parser = php_stream_context_get_uri_parser("ftp", context); if (uri_parser == NULL) { zend_value_error("%s(): Provided stream context has invalid value for the \"uri_parser_class\" option", get_active_function_name()); return 0; diff --git a/ext/standard/http_fopen_wrapper.c b/ext/standard/http_fopen_wrapper.c index 81d688a2f0458..30057b70eea09 100644 --- a/ext/standard/http_fopen_wrapper.c +++ b/ext/standard/http_fopen_wrapper.c @@ -393,7 +393,7 @@ static php_stream *php_stream_url_wrap_http_ex(php_stream_wrapper *wrapper, return NULL; } - uri_parser_t *uri_parser = php_stream_context_get_uri_parser("http", context); + const uri_parser_t *uri_parser = php_stream_context_get_uri_parser("http", context); if (uri_parser == NULL) { zend_value_error("%s(): Provided stream context has invalid value for the \"uri_parser_class\" option", get_active_function_name()); return NULL; diff --git a/ext/uri/php_uri.c b/ext/uri/php_uri.c index abdaa3eebe958..0a1abd7de4edd 100644 --- a/ext/uri/php_uri.c +++ b/ext/uri/php_uri.c @@ -52,7 +52,7 @@ static const zend_module_dep uri_deps[] = { static zend_array uri_parsers; -static uri_parser_t *uri_parser_by_name(const char *uri_parser_name, size_t uri_parser_name_len) +static const uri_parser_t *uri_parser_by_name(const char *uri_parser_name, size_t uri_parser_name_len) { return zend_hash_str_find_ptr(&uri_parsers, uri_parser_name, uri_parser_name_len); } @@ -107,7 +107,7 @@ static HashTable *uri_get_debug_properties(zend_object *object) return result; } -PHPAPI uri_parser_t *php_uri_get_parser(const zend_string *uri_parser_name) +PHPAPI const uri_parser_t *php_uri_get_parser(const zend_string *uri_parser_name) { if (uri_parser_name == NULL) { return uri_parser_by_name(PHP_URI_PARSER_PHP_PARSE_URL, sizeof(PHP_URI_PARSER_PHP_PARSE_URL) - 1); diff --git a/ext/uri/php_uri.h b/ext/uri/php_uri.h index 2b4394b60d49c..19ba97dc4f386 100644 --- a/ext/uri/php_uri.h +++ b/ext/uri/php_uri.h @@ -47,7 +47,7 @@ PHPAPI zend_result php_uri_parser_register(const uri_parser_t *uri_parser); * @param uri_parser_name The URI parser name * @return The URI parser */ -PHPAPI uri_parser_t *php_uri_get_parser(const zend_string *uri_parser_name); +PHPAPI const uri_parser_t *php_uri_get_parser(const zend_string *uri_parser_name); ZEND_ATTRIBUTE_NONNULL PHPAPI uri_internal_t *php_uri_parse(const uri_parser_t *uri_parser, const char *uri_str, size_t uri_str_len, bool silent); diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 2f5eae7b16c18..df51fa2189891 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -735,7 +735,7 @@ static ZEND_FUNCTION(zend_test_uri_parser) Z_PARAM_STR(parser_name) ZEND_PARSE_PARAMETERS_END(); - uri_parser_t *parser = php_uri_get_parser(parser_name); + const uri_parser_t *parser = php_uri_get_parser(parser_name); if (parser == NULL) { zend_argument_value_error(1, "Unknown parser"); RETURN_THROWS(); diff --git a/main/streams/php_stream_context.h b/main/streams/php_stream_context.h index 1890b32eaea24..eb082eb1402fa 100644 --- a/main/streams/php_stream_context.h +++ b/main/streams/php_stream_context.h @@ -68,7 +68,7 @@ void php_stream_context_unset_option(php_stream_context *context, struct uri_parser_t; -PHPAPI struct uri_parser_t *php_stream_context_get_uri_parser(const char *wrappername, php_stream_context *context); +PHPAPI const struct uri_parser_t *php_stream_context_get_uri_parser(const char *wrappername, php_stream_context *context); PHPAPI php_stream_notifier *php_stream_notification_alloc(void); PHPAPI void php_stream_notification_free(php_stream_notifier *notifier); END_EXTERN_C() diff --git a/main/streams/streams.c b/main/streams/streams.c index fdebc19e28245..88d982b760cd8 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -2458,7 +2458,7 @@ void php_stream_context_unset_option(php_stream_context *context, } /* }}} */ -PHPAPI struct uri_parser_t *php_stream_context_get_uri_parser(const char *wrappername, php_stream_context *context) +PHPAPI const struct uri_parser_t *php_stream_context_get_uri_parser(const char *wrappername, php_stream_context *context) { if (context == NULL) { return php_uri_get_parser(NULL); From 097963f867fbc15a154be8993efc41d9e6770da5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 30 Aug 2025 21:12:12 +0200 Subject: [PATCH 2/8] uri: Use `has_text_range()` in `php_uri_parser_rfc3986_scheme_read()` (#19621) Unfortunately missed that one in 119382354107006b1f24462c964ff44b4f960c3b. --- ext/uri/uri_parser_rfc3986.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/uri/uri_parser_rfc3986.c b/ext/uri/uri_parser_rfc3986.c index c80952a80c388..cf7235b071b4c 100644 --- a/ext/uri/uri_parser_rfc3986.c +++ b/ext/uri/uri_parser_rfc3986.c @@ -103,7 +103,7 @@ ZEND_ATTRIBUTE_NONNULL static zend_result php_uri_parser_rfc3986_scheme_read(con { const UriUriA *uriparser_uri = get_uri_for_reading(internal_uri->uri, read_mode); - if (uriparser_uri->scheme.first != NULL && uriparser_uri->scheme.afterLast != NULL) { + if (has_text_range(&uriparser_uri->scheme)) { ZVAL_STRINGL(retval, uriparser_uri->scheme.first, get_text_range_length(&uriparser_uri->scheme)); } else { ZVAL_NULL(retval); From 94678d99ceafd93ffc0dcfed5138f182e71e97d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 30 Aug 2025 21:21:57 +0200 Subject: [PATCH 3/8] uri: Fix memory safety violations when assigning `$errors` by reference fails (#19628) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * uri: Fix double-free when assigning `$errors` by reference fails `ZEND_TRY_ASSIGN_REF_ARR()` apparently consumes the to-be-assigned value even when it fails. * uri: Fix leak of parsed URI when assigning soft errors by reference fails This is not reproducible, because the URI object will still be referenced by Lexbor’s mraw instance and then cleanly destroyed at the end of the request. * NEWS --- NEWS | 2 ++ ext/uri/php_uri.c | 2 +- ext/uri/tests/057.phpt | 21 +++++++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 ext/uri/tests/057.phpt diff --git a/NEWS b/NEWS index 35d5c34b6fcd9..bf80ab696b090 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,8 @@ PHP NEWS . Fixed memory management of Uri\WhatWg\Url objects. (timwolla) . Fixed memory management of the internal "parse_url" URI parser. (timwolla) + . Fixed double-free when assigning to $errors fails when using + the Uri\WhatWg\Url parser. (timwolla) . Clean up naming of internal API. (timwolla) 28 Aug 2025, PHP 8.5.0beta2 diff --git a/ext/uri/php_uri.c b/ext/uri/php_uri.c index 0a1abd7de4edd..8ef25c020171e 100644 --- a/ext/uri/php_uri.c +++ b/ext/uri/php_uri.c @@ -325,7 +325,6 @@ static zend_result pass_errors_by_ref_and_free(zval *errors_zv, zval *errors) ZEND_TRY_ASSIGN_REF_ARR(errors_zv, Z_ARRVAL_P(errors)); if (EG(exception)) { - zval_ptr_dtor(errors); return FAILURE; } @@ -360,6 +359,7 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2) PHPAPI void php_uri_instantiate_uri( } if (pass_errors_by_ref_and_free(errors_zv, &errors) == FAILURE) { + uri_parser->free_uri(uri); RETURN_THROWS(); } diff --git a/ext/uri/tests/057.phpt b/ext/uri/tests/057.phpt new file mode 100644 index 0000000000000..e2a109ccdacb9 --- /dev/null +++ b/ext/uri/tests/057.phpt @@ -0,0 +1,21 @@ +--TEST-- +Test assigning errors by reference fails +--EXTENSIONS-- +uri +--FILE-- +x); +} catch (Throwable $e) { + echo $e::class, ": ", $e->getMessage(), PHP_EOL; +} + +?> +--EXPECT-- +TypeError: Cannot assign array to reference held by property Foo::$x of type string From 22252b03fea49b6e6925fa11ab93ea499f1713bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 30 Aug 2025 22:05:00 +0200 Subject: [PATCH 4/8] =?UTF-8?q?.github/labeler.yml:=20Add=20=E2=80=9CABI?= =?UTF-8?q?=20break=E2=80=9D=20label=20for=20ext/uri=20header=20changes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/labeler.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/labeler.yml b/.github/labeler.yml index 92f9c50962349..b9f0f36e147d4 100644 --- a/.github/labeler.yml +++ b/.github/labeler.yml @@ -492,6 +492,7 @@ - 'ext/spl/spl_iterators.h' - 'ext/spl/spl_observer.h' - 'ext/standard/*.h' + - 'ext/uri/*.h' - 'ext/xml/expat_compat.h' - 'ext/xml/php_xml.h' - 'main/*.h' From 243aedd3aae6843fd0107b37a83812bb85d19199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 30 Aug 2025 22:06:29 +0200 Subject: [PATCH 5/8] uri: Remove useless `uri_property_handlers_t` struct (#19622) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * uri: Do not copy the `property_handlers` struct in `uri_get_debug_properties()` * uri: Remove now-useless `uri_property_handlers_t` struct This struct was just used for “namespacing” within the `uri_parser_t` struct. It is no longer referenced anywhere. --- ext/uri/php_uri.c | 18 +++++++++--------- ext/uri/php_uri_common.h | 22 ++++++++++------------ 2 files changed, 19 insertions(+), 21 deletions(-) diff --git a/ext/uri/php_uri.c b/ext/uri/php_uri.c index 8ef25c020171e..4c5a496c3eec2 100644 --- a/ext/uri/php_uri.c +++ b/ext/uri/php_uri.c @@ -69,38 +69,38 @@ static HashTable *uri_get_debug_properties(zend_object *object) return result; } - const uri_property_handlers_t property_handlers = internal_uri->parser->property_handlers; + const uri_parser_t *parser = internal_uri->parser; zval tmp; - if (property_handlers.scheme.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { + if (parser->property_handlers.scheme.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { zend_hash_update(result, ZSTR_KNOWN(ZEND_STR_SCHEME), &tmp); } - if (property_handlers.username.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { + if (parser->property_handlers.username.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { zend_hash_update(result, ZSTR_KNOWN(ZEND_STR_USERNAME), &tmp); } - if (property_handlers.password.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { + if (parser->property_handlers.password.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { zend_hash_update(result, ZSTR_KNOWN(ZEND_STR_PASSWORD), &tmp); } - if (property_handlers.host.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { + if (parser->property_handlers.host.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { zend_hash_update(result, ZSTR_KNOWN(ZEND_STR_HOST), &tmp); } - if (property_handlers.port.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { + if (parser->property_handlers.port.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { zend_hash_update(result, ZSTR_KNOWN(ZEND_STR_PORT), &tmp); } - if (property_handlers.path.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { + if (parser->property_handlers.path.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { zend_hash_update(result, ZSTR_KNOWN(ZEND_STR_PATH), &tmp); } - if (property_handlers.query.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { + if (parser->property_handlers.query.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { zend_hash_update(result, ZSTR_KNOWN(ZEND_STR_QUERY), &tmp); } - if (property_handlers.fragment.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { + if (parser->property_handlers.fragment.read_func(internal_uri, URI_COMPONENT_READ_RAW, &tmp) == SUCCESS) { zend_hash_update(result, ZSTR_KNOWN(ZEND_STR_FRAGMENT), &tmp); } diff --git a/ext/uri/php_uri_common.h b/ext/uri/php_uri_common.h index fb465bed502ab..2c461ab3fb0a4 100644 --- a/ext/uri/php_uri_common.h +++ b/ext/uri/php_uri_common.h @@ -64,17 +64,6 @@ typedef struct uri_property_handler_t { uri_write_t write_func; } uri_property_handler_t; -typedef struct uri_property_handlers_t { - uri_property_handler_t scheme; - uri_property_handler_t username; - uri_property_handler_t password; - uri_property_handler_t host; - uri_property_handler_t port; - uri_property_handler_t path; - uri_property_handler_t query; - uri_property_handler_t fragment; -} uri_property_handlers_t; - typedef struct uri_parser_t { /** * Name (the FQCN) of the URI parser. The "" name is reserved for the handler of the legacy parse_url(). @@ -138,7 +127,16 @@ typedef struct uri_parser_t { */ void (*free_uri)(void *uri); - const uri_property_handlers_t property_handlers; + struct { + uri_property_handler_t scheme; + uri_property_handler_t username; + uri_property_handler_t password; + uri_property_handler_t host; + uri_property_handler_t port; + uri_property_handler_t path; + uri_property_handler_t query; + uri_property_handler_t fragment; + } property_handlers; } uri_parser_t; typedef struct uri_internal_t { From d24eab58e1ed81ee1f325046b8a1ad6e2fef0f7e Mon Sep 17 00:00:00 2001 From: Niels Dossche <7771979+nielsdos@users.noreply.github.com> Date: Sat, 30 Aug 2025 22:27:31 +0200 Subject: [PATCH 6/8] Use RETURN_EMPTY_ARRAY() on empty glob returns (#19642) --- ext/standard/dir.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ext/standard/dir.c b/ext/standard/dir.c index 2eb9d853eeb16..918b92fab456f 100644 --- a/ext/standard/dir.c +++ b/ext/standard/dir.c @@ -476,8 +476,7 @@ PHP_FUNCTION(glob) #ifdef PHP_GLOB_NOMATCH no_results: #endif - array_init(return_value); - return; + RETURN_EMPTY_ARRAY(); } array_init(return_value); From 3bf495b5cc9449772e46426f1f6e41ca87595b3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sat, 30 Aug 2025 23:37:39 +0200 Subject: [PATCH 7/8] =?UTF-8?q?uri:=20Retrieve=20=E2=80=9Craw=E2=80=9D=20r?= =?UTF-8?q?epresentation=20for=20`->getPort()`=20(#19646)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The implementation only supports one possible representation of the port. Retrieve the raw representation to avoid unnecessarily normalizing the URI for uri_parser_rfc3986. --- ext/uri/php_uri.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ext/uri/php_uri.c b/ext/uri/php_uri.c index 4c5a496c3eec2..17278c0f422e8 100644 --- a/ext/uri/php_uri.c +++ b/ext/uri/php_uri.c @@ -555,7 +555,7 @@ PHP_METHOD(Uri_Rfc3986_Uri, getRawHost) PHP_METHOD(Uri_Rfc3986_Uri, getPort) { - uri_read_component(INTERNAL_FUNCTION_PARAM_PASSTHRU, URI_PROPERTY_NAME_PORT, URI_COMPONENT_READ_NORMALIZED_ASCII); + uri_read_component(INTERNAL_FUNCTION_PARAM_PASSTHRU, URI_PROPERTY_NAME_PORT, URI_COMPONENT_READ_RAW); } PHP_METHOD(Uri_Rfc3986_Uri, getPath) From 2a086e4e73a212773a6f2addbed868209013169c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Sun, 31 Aug 2025 00:32:42 +0200 Subject: [PATCH 8/8] =?UTF-8?q?opcache:=20Improve=20error=20messages=20whe?= =?UTF-8?q?n=20=E2=80=9Ctemporarily=20enabling=20OPcache=E2=80=9D=20(#1961?= =?UTF-8?q?9)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * opcache: Do not emit “temporary enabling” message when OPcache is already active An easy way to accidentally enable OPcache “temporarily” is by using `php_admin_value[opcache.enable]=1` within a FPM pool’s configuration, since the `php_admin_value` settings mostly behave like settings in php.ini, with many OPcache INI settings being a notable exception. As long as OPcache is already enabled within php.ini (or simply by default), emitting a warning for `php_admin_value[opcache.enable]=1` or similar is going to be confusing, since is not actually temporarily enabling anything. A follow-up commit will also try to detect this kind of incorrect configuration and try to provide better advice for cases where OPcache is actually not yet enabled. * opcache: Improve error message when OPcache is enabled dynamically The error message will now advice on the `php_admin_value[opcache.enable]=1` mistake. It will also send the message to OPcache’s logging facility instead of the regular error handling logic during startup so that it will not be made available to `error_get_last()`, since it is related to a specific request and thus not actionable by a script either. php/php-src#19146 made a related change to `opcache.memory_consumption`. * opcache: Fix typo in warning message * opcache: Use more formal language in warning message --- .../tests/opcache_enable_noop_001.phpt | 24 +++++++++ .../tests/opcache_enable_noop_002.phpt | 18 +++++++ ext/opcache/zend_accelerator_module.c | 17 ++++++- .../fpm/tests/opcache_enable_admin_value.phpt | 49 +++++++++++++++++++ 4 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 ext/opcache/tests/opcache_enable_noop_001.phpt create mode 100644 ext/opcache/tests/opcache_enable_noop_002.phpt create mode 100644 sapi/fpm/tests/opcache_enable_admin_value.phpt diff --git a/ext/opcache/tests/opcache_enable_noop_001.phpt b/ext/opcache/tests/opcache_enable_noop_001.phpt new file mode 100644 index 0000000000000..0e7f5bbc98793 --- /dev/null +++ b/ext/opcache/tests/opcache_enable_noop_001.phpt @@ -0,0 +1,24 @@ +--TEST-- +Dynamically setting opcache.enable does not warn when noop +--INI-- +opcache.enable=1 +opcache.enable_cli=1 +--EXTENSIONS-- +opcache +--FILE-- + +--EXPECTF-- +Should not warn: +Disabling: +Should warn: + +Warning: Zend OPcache can't be temporarily enabled (it may be only disabled until the end of request) in %s on line %d diff --git a/ext/opcache/tests/opcache_enable_noop_002.phpt b/ext/opcache/tests/opcache_enable_noop_002.phpt new file mode 100644 index 0000000000000..994f2d22f4e45 --- /dev/null +++ b/ext/opcache/tests/opcache_enable_noop_002.phpt @@ -0,0 +1,18 @@ +--TEST-- +Dynamically setting opcache.enable warns when not a noop +--INI-- +opcache.enable=0 +opcache.enable_cli=1 +--EXTENSIONS-- +opcache +--FILE-- + +--EXPECTF-- +Should warn, since the INI was initialized to 0: + +Warning: Zend OPcache can't be temporarily enabled (it may be only disabled until the end of request) in %s on line %d diff --git a/ext/opcache/zend_accelerator_module.c b/ext/opcache/zend_accelerator_module.c index 9a168b38baaf8..6f668af9b714d 100644 --- a/ext/opcache/zend_accelerator_module.c +++ b/ext/opcache/zend_accelerator_module.c @@ -159,10 +159,23 @@ static ZEND_INI_MH(OnEnable) stage == ZEND_INI_STAGE_DEACTIVATE) { return OnUpdateBool(entry, new_value, mh_arg1, mh_arg2, mh_arg3, stage); } else { - /* It may be only temporary disabled */ + /* It may be only temporarily disabled */ bool *p = (bool *) ZEND_INI_GET_ADDR(); if (zend_ini_parse_bool(new_value)) { - zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporary enabled (it may be only disabled till the end of request)"); + if (*p) { + /* Do not warn if OPcache is enabled, as the update would be a noop anyways. */ + return SUCCESS; + } + + if (stage == ZEND_INI_STAGE_ACTIVATE) { + if (strcmp(sapi_module.name, "fpm-fcgi") == 0) { + zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled. Are you using php_admin_value[opcache.enable]=1 in an individual pool's configuration?"); + } else { + zend_accel_error(ACCEL_LOG_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled (it may be only disabled until the end of request)"); + } + } else { + zend_error(E_WARNING, ACCELERATOR_PRODUCT_NAME " can't be temporarily enabled (it may be only disabled until the end of request)"); + } return FAILURE; } else { *p = 0; diff --git a/sapi/fpm/tests/opcache_enable_admin_value.phpt b/sapi/fpm/tests/opcache_enable_admin_value.phpt new file mode 100644 index 0000000000000..2c1081aed9eb8 --- /dev/null +++ b/sapi/fpm/tests/opcache_enable_admin_value.phpt @@ -0,0 +1,49 @@ +--TEST-- +Setting opcache.enable via php_admin_value fails gracefully +--SKIPIF-- + +--FILE-- +start(iniEntries: [ + 'opcache.enable' => '0', + 'opcache.log_verbosity_level' => '2', +]); +$tester->expectLogStartNotices(); +$tester->expectLogPattern("/Zend OPcache can't be temporarily enabled. Are you using php_admin_value\\[opcache.enable\\]=1 in an individual pool's configuration?/"); +echo $tester + ->request() + ->getBody(); +$tester->terminate(); +$tester->close(); + +?> +--EXPECT-- +NULL +--CLEAN-- +