Skip to content

Commit

Permalink
Fix #80266: parse_url silently drops port number 0
Browse files Browse the repository at this point in the history
As of commit 81b2f3e[1], `parse_url()` accepts URLs with a zero port,
but does not report that port, what is wrong in hindsight.

Since the port number is stored as `unsigned short` there is no way to
distinguish between port zero and no port.  For BC reasons, we thus
introduce `parse_url_ex2()` which accepts an output parameter that
allows that distinction, and use the new function to fix the behavior.

The introduction of `parse_url_ex2()` has been suggested by Nikita.

[1] <http://git.php.net/?p=php-src.git;a=commit;h=81b2f3e5d9fcdffd87a4fcd12bd8c708a97091e1>

Closes GH-6399.
  • Loading branch information
cmb69 committed Nov 4, 2020
1 parent 00ba784 commit 2cab085
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 8 deletions.
3 changes: 3 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ PHP NEWS
. Fixed bug #70461 (disable md5 code when it is not supported in net-snmp).
(Alexander Bergmann, cmb)

- Standard:
. Fixed bug #80266 (parse_url silently drops port number 0). (cmb, Nikita)

29 Oct 2020, PHP 7.3.24

- Core:
Expand Down
4 changes: 3 additions & 1 deletion ext/standard/tests/url/parse_url_basic_001.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -859,11 +859,13 @@ echo "Done";
string(3) "%:x"
}

--> https://example.com:0/: array(3) {
--> https://example.com:0/: array(4) {
["scheme"]=>
string(5) "https"
["host"]=>
string(11) "example.com"
["port"]=>
int(0)
["path"]=>
string(1) "/"
}
Expand Down
2 changes: 1 addition & 1 deletion ext/standard/tests/url/parse_url_basic_004.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ echo "Done";
--> / : NULL
--> /rest/Users?filter={"id":"123"} : NULL
--> %:x : NULL
--> https://example.com:0/ : NULL
--> https://example.com:0/ : int(0)
--> http:///blah.com : bool(false)
--> http://:80 : bool(false)
--> http://user@:80 : bool(false)
Expand Down
4 changes: 3 additions & 1 deletion ext/standard/tests/url/parse_url_unterminated.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -861,11 +861,13 @@ echo "Done";
string(3) "%:x"
}

--> https://example.com:0/: array(3) {
--> https://example.com:0/: array(4) {
["scheme"]=>
string(5) "https"
["host"]=>
string(11) "example.com"
["port"]=>
int(0)
["path"]=>
string(1) "/"
}
Expand Down
20 changes: 15 additions & 5 deletions ext/standard/url.c
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,21 @@ static const char *binary_strcspn(const char *s, const char *e, const char *char
return e;
}

/* {{{ php_url_parse
*/
PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
{
zend_bool has_port;
return php_url_parse_ex2(str, length, &has_port);
}

/* {{{ php_url_parse_ex2
*/
PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, zend_bool *has_port)
{
char port_buf[6];
php_url *ret = ecalloc(1, sizeof(php_url));
char const *s, *e, *p, *pp, *ue;

*has_port = 0;
s = str;
ue = s + length;

Expand Down Expand Up @@ -199,6 +206,7 @@ PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
port_buf[pp - p] = '\0';
port = ZEND_STRTOL(port_buf, &end, 10);
if (port >= 0 && port <= 65535 && end != port_buf) {
*has_port = 1;
ret->port = (unsigned short) port;
if (s + 1 < ue && *s == '/' && *(s + 1) == '/') { /* relative-scheme URL */
s += 2;
Expand Down Expand Up @@ -264,6 +272,7 @@ PHPAPI php_url *php_url_parse_ex(char const *str, size_t length)
port_buf[e - p] = '\0';
port = ZEND_STRTOL(port_buf, &end, 10);
if (port >= 0 && port <= 65535 && end != port_buf) {
*has_port = 1;
ret->port = (unsigned short)port;
} else {
php_url_free(ret);
Expand Down Expand Up @@ -332,14 +341,15 @@ PHP_FUNCTION(parse_url)
php_url *resource;
zend_long key = -1;
zval tmp;
zend_bool has_port;

ZEND_PARSE_PARAMETERS_START(1, 2)
Z_PARAM_STRING(str, str_len)
Z_PARAM_OPTIONAL
Z_PARAM_LONG(key)
ZEND_PARSE_PARAMETERS_END();

resource = php_url_parse_ex(str, str_len);
resource = php_url_parse_ex2(str, str_len, &has_port);
if (resource == NULL) {
/* @todo Find a method to determine why php_url_parse_ex() failed */
RETURN_FALSE;
Expand All @@ -354,7 +364,7 @@ PHP_FUNCTION(parse_url)
if (resource->host != NULL) RETVAL_STR_COPY(resource->host);
break;
case PHP_URL_PORT:
if (resource->port != 0) RETVAL_LONG(resource->port);
if (has_port) RETVAL_LONG(resource->port);
break;
case PHP_URL_USER:
if (resource->user != NULL) RETVAL_STR_COPY(resource->user);
Expand Down Expand Up @@ -390,7 +400,7 @@ PHP_FUNCTION(parse_url)
ZVAL_STR_COPY(&tmp, resource->host);
zend_hash_add_new(Z_ARRVAL_P(return_value), ZSTR_KNOWN(ZEND_STR_HOST), &tmp);
}
if (resource->port != 0) {
if (has_port) {
ZVAL_LONG(&tmp, resource->port);
zend_hash_add_new(Z_ARRVAL_P(return_value), ZSTR_KNOWN(ZEND_STR_PORT), &tmp);
}
Expand Down
1 change: 1 addition & 0 deletions ext/standard/url.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ typedef struct php_url {
PHPAPI void php_url_free(php_url *theurl);
PHPAPI php_url *php_url_parse(char const *str);
PHPAPI php_url *php_url_parse_ex(char const *str, size_t length);
PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, zend_bool *has_port);
PHPAPI size_t php_url_decode(char *str, size_t len); /* return value: length of decoded string */
PHPAPI size_t php_raw_url_decode(char *str, size_t len); /* return value: length of decoded string */
PHPAPI zend_string *php_url_encode(char const *s, size_t len);
Expand Down

0 comments on commit 2cab085

Please sign in to comment.