Skip to content

Commit fb7267c

Browse files
TimWollaTimWolla
committed
uri: Fix normalization memory management for uri_parser_php_parse_url.c
There were two issues with the previous implementation of normalization: - `php_raw_url_decode_ex()` would be used to modify a string with RC >1. - The return value of `php_raw_url_decode_ex()` was not used, resulting in incorrect string lengths when percent-encoded characters are decoded. Additionally there was a bogus assertion that verified that strings returned from the read handlers are RC =2, which was not the case for the `parse_url`-based parser when repeatedly retrieving a component even without normalization happening. Remove that assertion, since its usefulness is questionable. Any obvious data type issues with read handlers should be detectable when testing during development.
1 parent e18ef6c commit fb7267c

File tree

6 files changed

+143
-29
lines changed

6 files changed

+143
-29
lines changed

‎ext/uri/php_uri.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,7 @@ ZEND_ATTRIBUTE_NONNULL static zend_result php_uri_get_property(const uri_interna
137137
return FAILURE;
138138
}
139139

140-
zend_result result = property_handler->read_func(internal_uri, read_mode, zv);
141-
142-
ZEND_ASSERT(result == FAILURE || (Z_TYPE_P(zv) == IS_STRING && GC_REFCOUNT(Z_STR_P(zv)) == 2) || Z_TYPE_P(zv) == IS_NULL || Z_TYPE_P(zv) == IS_LONG);
143-
144-
return result;
140+
return property_handler->read_func(internal_uri, read_mode, zv);
145141
}
146142

147143
ZEND_ATTRIBUTE_NONNULL PHPAPI zend_result php_uri_get_scheme(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *zv)

‎ext/uri/tests/100.phpt

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
--TEST--
2+
Test InvalidUrlException constructor error handling
3+
--EXTENSIONS--
4+
uri
5+
zend_test
6+
--FILE--
7+
<?php
8+
9+
var_dump(zend_test_uri_parser("https://%65xample:%65xample@%65xample.com:123/%65xample.html?%65xample=%65xample#%65xample", "parse_url"));
10+
11+
?>
12+
--EXPECT--
13+
array(2) {
14+
["normalized"]=>
15+
array(8) {
16+
["scheme"]=>
17+
string(5) "https"
18+
["username"]=>
19+
string(7) "example"
20+
["password"]=>
21+
string(7) "example"
22+
["host"]=>
23+
string(11) "example.com"
24+
["port"]=>
25+
int(123)
26+
["path"]=>
27+
string(13) "/example.html"
28+
["query"]=>
29+
string(15) "example=example"
30+
["fragment"]=>
31+
string(7) "example"
32+
}
33+
["raw"]=>
34+
array(8) {
35+
["scheme"]=>
36+
string(5) "https"
37+
["username"]=>
38+
string(9) "%65xample"
39+
["password"]=>
40+
string(9) "%65xample"
41+
["host"]=>
42+
string(13) "%65xample.com"
43+
["port"]=>
44+
int(123)
45+
["path"]=>
46+
string(15) "/%65xample.html"
47+
["query"]=>
48+
string(19) "%65xample=%65xample"
49+
["fragment"]=>
50+
string(9) "%65xample"
51+
}
52+
}

‎ext/uri/uri_parser_php_parse_url.c

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -21,26 +21,28 @@
2121
#include "Zend/zend_exceptions.h"
2222
#include "ext/standard/url.h"
2323

24-
static void decode_component(zval *zv, uri_component_read_mode_t read_mode)
24+
static zend_string *decode_component(zend_string *in, uri_component_read_mode_t read_mode)
2525
{
26-
if (Z_TYPE_P(zv) != IS_STRING) {
27-
return;
28-
}
26+
switch (read_mode) {
27+
case URI_COMPONENT_READ_RAW:
28+
return zend_string_copy(in);
29+
case URI_COMPONENT_READ_NORMALIZED_ASCII:
30+
case URI_COMPONENT_READ_NORMALIZED_UNICODE:
31+
zend_string *out = zend_string_alloc(ZSTR_LEN(in), false);
2932

30-
if (read_mode == URI_COMPONENT_READ_RAW) {
31-
return;
32-
}
33+
ZSTR_LEN(out) = php_raw_url_decode_ex(ZSTR_VAL(out), ZSTR_VAL(in), ZSTR_LEN(in));
3334

34-
php_raw_url_decode(Z_STRVAL_P(zv), Z_STRLEN_P(zv));
35+
return out;
36+
EMPTY_SWITCH_DEFAULT_CASE();
37+
}
3538
}
3639

3740
static zend_result uri_parser_php_parse_url_scheme_read(const uri_internal_t *internal_uri, uri_component_read_mode_t read_mode, zval *retval)
3841
{
3942
const php_url *parse_url_uri = internal_uri->uri;
4043

4144
if (parse_url_uri->scheme) {
42-
ZVAL_STR_COPY(retval, parse_url_uri->scheme);
43-
decode_component(retval, read_mode);
45+
ZVAL_STR(retval, decode_component(parse_url_uri->scheme, read_mode));
4446
} else {
4547
ZVAL_NULL(retval);
4648
}
@@ -53,8 +55,7 @@ static zend_result uri_parser_php_parse_url_username_read(const uri_internal_t *
5355
const php_url *parse_url_uri = internal_uri->uri;
5456

5557
if (parse_url_uri->user) {
56-
ZVAL_STR_COPY(retval, parse_url_uri->user);
57-
decode_component(retval, read_mode);
58+
ZVAL_STR(retval, decode_component(parse_url_uri->user, read_mode));
5859
} else {
5960
ZVAL_NULL(retval);
6061
}
@@ -67,8 +68,7 @@ static zend_result uri_parser_php_parse_url_password_read(const uri_internal_t *
6768
const php_url *parse_url_uri = internal_uri->uri;
6869

6970
if (parse_url_uri->pass) {
70-
ZVAL_STR_COPY(retval, parse_url_uri->pass);
71-
decode_component(retval, read_mode);
71+
ZVAL_STR(retval, decode_component(parse_url_uri->pass, read_mode));
7272
} else {
7373
ZVAL_NULL(retval);
7474
}
@@ -81,8 +81,7 @@ static zend_result uri_parser_php_parse_url_host_read(const uri_internal_t *inte
8181
const php_url *parse_url_uri = internal_uri->uri;
8282

8383
if (parse_url_uri->host) {
84-
ZVAL_STR_COPY(retval, parse_url_uri->host);
85-
decode_component(retval, read_mode);
84+
ZVAL_STR(retval, decode_component(parse_url_uri->host, read_mode));
8685
} else {
8786
ZVAL_NULL(retval);
8887
}
@@ -96,7 +95,6 @@ static zend_result uri_parser_php_parse_url_port_read(const uri_internal_t *inte
9695

9796
if (parse_url_uri->port) {
9897
ZVAL_LONG(retval, parse_url_uri->port);
99-
decode_component(retval, read_mode);
10098
} else {
10199
ZVAL_NULL(retval);
102100
}
@@ -109,8 +107,7 @@ static zend_result uri_parser_php_parse_url_path_read(const uri_internal_t *inte
109107
const php_url *parse_url_uri = internal_uri->uri;
110108

111109
if (parse_url_uri->path) {
112-
ZVAL_STR_COPY(retval, parse_url_uri->path);
113-
decode_component(retval, read_mode);
110+
ZVAL_STR(retval, decode_component(parse_url_uri->path, read_mode));
114111
} else {
115112
ZVAL_NULL(retval);
116113
}
@@ -123,8 +120,7 @@ static zend_result uri_parser_php_parse_url_query_read(const uri_internal_t *int
123120
const php_url *parse_url_uri = internal_uri->uri;
124121

125122
if (parse_url_uri->query) {
126-
ZVAL_STR_COPY(retval, parse_url_uri->query);
127-
decode_component(retval, read_mode);
123+
ZVAL_STR(retval, decode_component(parse_url_uri->query, read_mode));
128124
} else {
129125
ZVAL_NULL(retval);
130126
}
@@ -137,8 +133,7 @@ static zend_result uri_parser_php_parse_url_fragment_read(const uri_internal_t *
137133
const php_url *parse_url_uri = internal_uri->uri;
138134

139135
if (parse_url_uri->fragment) {
140-
ZVAL_STR_COPY(retval, parse_url_uri->fragment);
141-
decode_component(retval, read_mode);
136+
ZVAL_STR(retval, decode_component(parse_url_uri->fragment, read_mode));
142137
} else {
143138
ZVAL_NULL(retval);
144139
}

‎ext/zend_test/test.c

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
#include "zend_call_stack.h"
4141
#include "zend_exceptions.h"
4242
#include "zend_mm_custom_handlers.h"
43+
#include "ext/uri/php_uri.h"
4344

4445
#if defined(HAVE_LIBXML) && !defined(PHP_WIN32)
4546
# include <libxml/globals.h>
@@ -724,6 +725,67 @@ static ZEND_FUNCTION(zend_test_crash)
724725
php_printf("%s", invalid);
725726
}
726727

728+
static ZEND_FUNCTION(zend_test_uri_parser)
729+
{
730+
zend_string *uri_string;
731+
zend_string *parser_name;
732+
733+
ZEND_PARSE_PARAMETERS_START(2, 2)
734+
Z_PARAM_STR(uri_string)
735+
Z_PARAM_STR(parser_name)
736+
ZEND_PARSE_PARAMETERS_END();
737+
738+
uri_parser_t *parser = php_uri_get_parser(parser_name);
739+
uri_internal_t *uri = php_uri_parse(parser, ZSTR_VAL(uri_string), ZSTR_LEN(uri_string), false);
740+
if (uri == NULL) {
741+
RETURN_THROWS();
742+
}
743+
744+
zval value;
745+
746+
array_init(return_value);
747+
zval normalized;
748+
array_init(&normalized);
749+
php_uri_get_scheme(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value);
750+
zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_SCHEME), &value);
751+
php_uri_get_username(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value);
752+
zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_USERNAME), &value);
753+
php_uri_get_password(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value);
754+
zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_PASSWORD), &value);
755+
php_uri_get_host(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value);
756+
zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_HOST), &value);
757+
php_uri_get_port(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value);
758+
zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_PORT), &value);
759+
php_uri_get_path(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value);
760+
zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_PATH), &value);
761+
php_uri_get_query(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value);
762+
zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_QUERY), &value);
763+
php_uri_get_fragment(uri, URI_COMPONENT_READ_NORMALIZED_ASCII, &value);
764+
zend_hash_add(Z_ARR(normalized), ZSTR_KNOWN(ZEND_STR_FRAGMENT), &value);
765+
zend_hash_str_add(Z_ARR_P(return_value), "normalized", strlen("normalized"), &normalized);
766+
zval raw;
767+
array_init(&raw);
768+
php_uri_get_scheme(uri, URI_COMPONENT_READ_RAW, &value);
769+
zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_SCHEME), &value);
770+
php_uri_get_username(uri, URI_COMPONENT_READ_RAW, &value);
771+
zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_USERNAME), &value);
772+
php_uri_get_password(uri, URI_COMPONENT_READ_RAW, &value);
773+
zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_PASSWORD), &value);
774+
php_uri_get_host(uri, URI_COMPONENT_READ_RAW, &value);
775+
zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_HOST), &value);
776+
php_uri_get_port(uri, URI_COMPONENT_READ_RAW, &value);
777+
zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_PORT), &value);
778+
php_uri_get_path(uri, URI_COMPONENT_READ_RAW, &value);
779+
zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_PATH), &value);
780+
php_uri_get_query(uri, URI_COMPONENT_READ_RAW, &value);
781+
zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_QUERY), &value);
782+
php_uri_get_fragment(uri, URI_COMPONENT_READ_RAW, &value);
783+
zend_hash_add(Z_ARR(raw), ZSTR_KNOWN(ZEND_STR_FRAGMENT), &value);
784+
zend_hash_str_add(Z_ARR_P(return_value), "raw", strlen("raw"), &raw);
785+
786+
php_uri_free(uri);
787+
}
788+
727789
static bool has_opline(zend_execute_data *execute_data)
728790
{
729791
return execute_data

‎ext/zend_test/test.stub.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -342,6 +342,8 @@ function zend_test_compile_to_ast(string $str): string {}
342342
function zend_test_gh18756(): void {}
343343

344344
function zend_test_opcache_preloading(): bool {}
345+
346+
function zend_test_uri_parser(string $uri, string $parser): array { }
345347
}
346348

347349
namespace ZendTestNS {

‎ext/zend_test/test_arginfo.h

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

0 commit comments

Comments
 (0)