From aca597cd8422640088686a85c9773c70e3639930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Mon, 25 Aug 2025 20:53:55 +0200 Subject: [PATCH 1/2] uri: Simplify memory-management in `php_uri_parse()` We can try parsing before allocating the `uri_internal_t` struct. --- ext/uri/php_uri.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/ext/uri/php_uri.c b/ext/uri/php_uri.c index abdaa3eebe958..94debbf91f938 100644 --- a/ext/uri/php_uri.c +++ b/ext/uri/php_uri.c @@ -118,15 +118,16 @@ PHPAPI 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) { - uri_internal_t *internal_uri = emalloc(sizeof(*internal_uri)); - internal_uri->parser = uri_parser; - internal_uri->uri = uri_parser->parse_uri(uri_str, uri_str_len, NULL, NULL, silent); + void *parsed = uri_parser->parse_uri(uri_str, uri_str_len, NULL, NULL, silent); - if (UNEXPECTED(internal_uri->uri == NULL)) { - efree(internal_uri); + if (parsed == NULL) { return NULL; } + uri_internal_t *internal_uri = emalloc(sizeof(*internal_uri)); + internal_uri->parser = uri_parser; + internal_uri->uri = parsed; + return internal_uri; } From 80e51a7f5af928bd7c4f65b1b6fbd9002fefbcbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Mon, 25 Aug 2025 20:57:32 +0200 Subject: [PATCH 2/2] uri: Remove `php_uri_parse()` and `php_uri_free()` This API is both less powerful as well as less efficient compared to just calling the `->parse_uri()` handler of the parser directly. With regard to efficiency it needlessly allocates 32 byte of memory to store two pointers, of which one is already known, because it's the input. While it would be possible to just return the resulting `uri_internal_t` struct (instead of a pointer to a freshly allocated one), which would be returned as a register pair, users of the API can also just create the struct themselves for even more flexibility in allocations. The API is also less powerful, because it does not support base URIs. --- ext/openssl/xp_ssl.c | 13 ++++++---- ext/uri/php_uri.c | 57 ++++++++++++++++---------------------------- ext/uri/php_uri.h | 9 ------- ext/zend_test/test.c | 44 +++++++++++++++++++--------------- 4 files changed, 55 insertions(+), 68 deletions(-) diff --git a/ext/openssl/xp_ssl.c b/ext/openssl/xp_ssl.c index 6fbdb506133c7..6ed975eca3b13 100644 --- a/ext/openssl/xp_ssl.c +++ b/ext/openssl/xp_ssl.c @@ -2640,14 +2640,19 @@ static char *php_openssl_get_url_name(const char *resourcename, return NULL; } - uri_internal_t *internal_uri = php_uri_parse(uri_parser, resourcename, resourcenamelen, true); - if (internal_uri == NULL) { + void *parsed = uri_parser->parse_uri(resourcename, resourcenamelen, + /* base_url */ NULL, /* errors */ NULL, /* silent */ true); + if (parsed == NULL) { return NULL; } + uri_internal_t internal_uri = { + .parser = uri_parser, + .uri = parsed, + }; char * url_name = NULL; zval host_zv; - zend_result result = php_uri_get_host(internal_uri, URI_COMPONENT_READ_RAW, &host_zv); + zend_result result = php_uri_get_host(&internal_uri, URI_COMPONENT_READ_RAW, &host_zv); if (result == SUCCESS && Z_TYPE(host_zv) == IS_STRING) { const char * host = Z_STRVAL(host_zv); size_t len = Z_STRLEN(host_zv); @@ -2662,7 +2667,7 @@ static char *php_openssl_get_url_name(const char *resourcename, } } - php_uri_free(internal_uri); + uri_parser->free_uri(parsed); zval_ptr_dtor(&host_zv); return url_name; diff --git a/ext/uri/php_uri.c b/ext/uri/php_uri.c index 94debbf91f938..636b2ab9707a1 100644 --- a/ext/uri/php_uri.c +++ b/ext/uri/php_uri.c @@ -116,21 +116,6 @@ PHPAPI uri_parser_t *php_uri_get_parser(const zend_string *uri_parser_name) return uri_parser_by_name(ZSTR_VAL(uri_parser_name), ZSTR_LEN(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) -{ - void *parsed = uri_parser->parse_uri(uri_str, uri_str_len, NULL, NULL, silent); - - if (parsed == NULL) { - return NULL; - } - - uri_internal_t *internal_uri = emalloc(sizeof(*internal_uri)); - internal_uri->parser = uri_parser; - internal_uri->uri = parsed; - - return internal_uri; -} - ZEND_ATTRIBUTE_NONNULL static zend_result php_uri_get_property(const uri_internal_t *internal_uri, uri_property_name_t property_name, uri_component_read_mode_t read_mode, zval *zv) { const uri_property_handler_t *property_handler = uri_property_handler_from_internal_uri(internal_uri, property_name); @@ -181,27 +166,25 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI zend_result php_uri_get_fragment(const uri_interna return php_uri_get_property(internal_uri, URI_PROPERTY_NAME_FRAGMENT, read_mode, zv); } -ZEND_ATTRIBUTE_NONNULL PHPAPI void php_uri_free(uri_internal_t *internal_uri) -{ - internal_uri->parser->free_uri(internal_uri->uri); - internal_uri->uri = NULL; - internal_uri->parser = NULL; - efree(internal_uri); -} - ZEND_ATTRIBUTE_NONNULL PHPAPI php_uri *php_uri_parse_to_struct( const uri_parser_t *uri_parser, const char *uri_str, size_t uri_str_len, uri_component_read_mode_t read_mode, bool silent ) { - uri_internal_t *uri_internal = php_uri_parse(uri_parser, uri_str, uri_str_len, silent); - if (uri_internal == NULL) { + void *parsed = uri_parser->parse_uri(uri_str, uri_str_len, + /* base_url */ NULL, /* errors */ NULL, silent); + if (parsed == NULL) { return NULL; } + uri_internal_t uri_internal = { + .parser = uri_parser, + .uri = parsed, + }; + php_uri *uri = ecalloc(1, sizeof(*uri)); zval tmp; zend_result result; - result = php_uri_get_scheme(uri_internal, read_mode, &tmp); + result = php_uri_get_scheme(&uri_internal, read_mode, &tmp); if (result == FAILURE) { goto error; } @@ -209,7 +192,7 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI php_uri *php_uri_parse_to_struct( uri->scheme = Z_STR(tmp); } - result = php_uri_get_username(uri_internal, read_mode, &tmp); + result = php_uri_get_username(&uri_internal, read_mode, &tmp); if (result == FAILURE) { goto error; } @@ -217,7 +200,7 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI php_uri *php_uri_parse_to_struct( uri->user = Z_STR(tmp); } - result = php_uri_get_password(uri_internal, read_mode, &tmp); + result = php_uri_get_password(&uri_internal, read_mode, &tmp); if (result == FAILURE) { goto error; } @@ -225,7 +208,7 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI php_uri *php_uri_parse_to_struct( uri->password = Z_STR(tmp); } - result = php_uri_get_host(uri_internal, read_mode, &tmp); + result = php_uri_get_host(&uri_internal, read_mode, &tmp); if (result == FAILURE) { goto error; } @@ -233,7 +216,7 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI php_uri *php_uri_parse_to_struct( uri->host = Z_STR(tmp); } - result = php_uri_get_port(uri_internal, read_mode, &tmp); + result = php_uri_get_port(&uri_internal, read_mode, &tmp); if (result == FAILURE) { goto error; } @@ -241,7 +224,7 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI php_uri *php_uri_parse_to_struct( uri->port = Z_LVAL(tmp); } - result = php_uri_get_path(uri_internal, read_mode, &tmp); + result = php_uri_get_path(&uri_internal, read_mode, &tmp); if (result == FAILURE) { goto error; } @@ -249,7 +232,7 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI php_uri *php_uri_parse_to_struct( uri->path = Z_STR(tmp); } - result = php_uri_get_query(uri_internal, read_mode, &tmp); + result = php_uri_get_query(&uri_internal, read_mode, &tmp); if (result == FAILURE) { goto error; } @@ -257,7 +240,7 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI php_uri *php_uri_parse_to_struct( uri->query = Z_STR(tmp); } - result = php_uri_get_fragment(uri_internal, read_mode, &tmp); + result = php_uri_get_fragment(&uri_internal, read_mode, &tmp); if (result == FAILURE) { goto error; } @@ -265,12 +248,13 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI php_uri *php_uri_parse_to_struct( uri->fragment = Z_STR(tmp); } - php_uri_free(uri_internal); + uri_parser->free_uri(parsed); return uri; error: - php_uri_free(uri_internal); + + uri_parser->free_uri(parsed); php_uri_struct_free(uri); return NULL; @@ -347,7 +331,8 @@ ZEND_ATTRIBUTE_NONNULL_ARGS(1, 2) PHPAPI void php_uri_instantiate_uri( base_url = internal_base_url->uri; } - void *uri = uri_parser->parse_uri(ZSTR_VAL(uri_str), ZSTR_LEN(uri_str), base_url, should_throw || errors_zv != NULL ? &errors : NULL, !should_throw); + void *uri = uri_parser->parse_uri(ZSTR_VAL(uri_str), ZSTR_LEN(uri_str), + base_url, should_throw || errors_zv != NULL ? &errors : NULL, !should_throw); if (UNEXPECTED(uri == NULL)) { if (should_throw) { zval_ptr_dtor(&errors); diff --git a/ext/uri/php_uri.h b/ext/uri/php_uri.h index 2b4394b60d49c..60f8c65ebab58 100644 --- a/ext/uri/php_uri.h +++ b/ext/uri/php_uri.h @@ -49,8 +49,6 @@ PHPAPI zend_result php_uri_parser_register(const uri_parser_t *uri_parser); */ PHPAPI 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); - /** * Retrieves the scheme component based on the read_mode and passes it to the zv ZVAL in case of success. * @@ -171,13 +169,6 @@ ZEND_ATTRIBUTE_NONNULL PHPAPI zend_result php_uri_get_query(const uri_internal_t */ ZEND_ATTRIBUTE_NONNULL PHPAPI zend_result php_uri_get_fragment(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *zv); -/** - * Frees the uri member within the provided internal URI. - * - * @param internal_uri The internal URI - */ -ZEND_ATTRIBUTE_NONNULL PHPAPI void php_uri_free(uri_internal_t *internal_uri); - /** * Creates a new php_uri struct containing all the URI components. The components are retrieved based on the read_mode parameter. * diff --git a/ext/zend_test/test.c b/ext/zend_test/test.c index 2f5eae7b16c18..4a529b47a0ddf 100644 --- a/ext/zend_test/test.c +++ b/ext/zend_test/test.c @@ -741,54 +741,60 @@ static ZEND_FUNCTION(zend_test_uri_parser) RETURN_THROWS(); } - uri_internal_t *uri = php_uri_parse(parser, ZSTR_VAL(uri_string), ZSTR_LEN(uri_string), false); - if (uri == NULL) { + void *parsed = parser->parse_uri(ZSTR_VAL(uri_string), ZSTR_LEN(uri_string), + /* base_url */ NULL, /* errors */ NULL, /* silent */ false); + if (parsed == NULL) { RETURN_THROWS(); } + uri_internal_t uri = { + .parser = parser, + .uri = parsed, + }; + zval value; array_init(return_value); zval normalized; array_init(&normalized); - php_uri_get_scheme(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); + php_uri_get_scheme(&uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_SCHEME), &value); - php_uri_get_username(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); + php_uri_get_username(&uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_USERNAME), &value); - php_uri_get_password(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); + php_uri_get_password(&uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_PASSWORD), &value); - php_uri_get_host(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); + php_uri_get_host(&uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_HOST), &value); - php_uri_get_port(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); + php_uri_get_port(&uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_PORT), &value); - php_uri_get_path(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); + php_uri_get_path(&uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_PATH), &value); - php_uri_get_query(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); + php_uri_get_query(&uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_QUERY), &value); - php_uri_get_fragment(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); + php_uri_get_fragment(&uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value); zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_FRAGMENT), &value); zend_hash_str_add(Z_ARR_P(return_value), "normalized", strlen("normalized"), &normalized); zval raw; array_init(&raw); - php_uri_get_scheme(uri, URI_COMPONENT_READ_RAW, &value); + php_uri_get_scheme(&uri, URI_COMPONENT_READ_RAW, &value); zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_SCHEME), &value); - php_uri_get_username(uri, URI_COMPONENT_READ_RAW, &value); + php_uri_get_username(&uri, URI_COMPONENT_READ_RAW, &value); zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_USERNAME), &value); - php_uri_get_password(uri, URI_COMPONENT_READ_RAW, &value); + php_uri_get_password(&uri, URI_COMPONENT_READ_RAW, &value); zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_PASSWORD), &value); - php_uri_get_host(uri, URI_COMPONENT_READ_RAW, &value); + php_uri_get_host(&uri, URI_COMPONENT_READ_RAW, &value); zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_HOST), &value); - php_uri_get_port(uri, URI_COMPONENT_READ_RAW, &value); + php_uri_get_port(&uri, URI_COMPONENT_READ_RAW, &value); zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_PORT), &value); - php_uri_get_path(uri, URI_COMPONENT_READ_RAW, &value); + php_uri_get_path(&uri, URI_COMPONENT_READ_RAW, &value); zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_PATH), &value); - php_uri_get_query(uri, URI_COMPONENT_READ_RAW, &value); + php_uri_get_query(&uri, URI_COMPONENT_READ_RAW, &value); zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_QUERY), &value); - php_uri_get_fragment(uri, URI_COMPONENT_READ_RAW, &value); + php_uri_get_fragment(&uri, URI_COMPONENT_READ_RAW, &value); zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_FRAGMENT), &value); zend_hash_str_add(Z_ARR_P(return_value), "raw", strlen("raw"), &raw); - php_uri_free(uri); + parser->free_uri(parsed); } static bool has_opline(zend_execute_data *execute_data)