Skip to content

Revert "Fix parse_url(): can not recognize port without scheme" #9569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Revert "Fix parse_url(): can not recognize port without scheme" #9569

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
69bd722
Revert "Fix parse_url(): can not recognize port without scheme"
andypost Sep 17, 2022
d59e64d
Add test with expected behaviour
andypost Sep 17, 2022
f7e396f
Extend test with Drupal's internal: url and magnet: one
andypost Sep 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,7 +421,6 @@ PHP NEWS
. Fixed bug GH-7847 (stripos with large haystack has bad performance).
(ilutov)
. New function memory_reset_peak_usage(). (Patrick Allaert)
. Fixed parse_url(): can not recognize port without scheme. (pandaLIU)

- Streams:
. Set IP_BIND_ADDRESS_NO_PORT if available when connecting to remote host.
Expand Down
30 changes: 24 additions & 6 deletions ext/standard/tests/url/parse_url_basic_011.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,42 @@ echo 'parse 127.0.0.1:9999?', PHP_EOL;
var_dump(parse_url('127.0.0.1:9999?'));
echo 'parse 127.0.0.1:9999#', PHP_EOL;
var_dump(parse_url('127.0.0.1:9999#'));
echo 'parse internal:#feeding', PHP_EOL;
var_dump(parse_url('internal:#feeding'));
echo 'parse magnet:?xt=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C', PHP_EOL;
var_dump(parse_url('magnet:?xt=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C'));
?>
--EXPECT--
*** Testing parse_url() :can not recognize port without scheme ***
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the point in keeping this test. It has been introduced to show and verify that the other issue has been fixed; since we're going to revert, we should drop the test as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂️ Since in the original change no other test was touched that's a pretty clear indication that this behavior is untested. Keeping this would make it more obvious if it ever changes again. But I'm fine either way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, it shouldn't hurt to keep it, and it may help for future developement.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to keep the comment as it documents current behavior

parse 127.0.0.1:9999?
array(3) {
["host"]=>
["scheme"]=>
string(9) "127.0.0.1"
["port"]=>
int(9999)
["path"]=>
string(4) "9999"
["query"]=>
string(0) ""
}
parse 127.0.0.1:9999#
array(3) {
["host"]=>
["scheme"]=>
string(9) "127.0.0.1"
["port"]=>
int(9999)
["path"]=>
string(4) "9999"
["fragment"]=>
string(0) ""
}
parse internal:#feeding
array(2) {
["scheme"]=>
string(8) "internal"
["fragment"]=>
string(7) "feeding"
}
parse magnet:?xt=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C
array(2) {
["scheme"]=>
string(6) "magnet"
["query"]=>
string(44) "xt=urn:sha1:YNCKHTQCWBTRNJIV4WNAE52SJUQCZO5C"
}
4 changes: 2 additions & 2 deletions ext/standard/url.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, bool *has_port
p++;
}

if ((p == ue || *p == '/' || *p == '?' || *p == '#') && (p - e) < 7) {
if ((p == ue || *p == '/') && (p - e) < 7) {
goto parse_port;
}

Expand Down Expand Up @@ -190,7 +190,7 @@ PHPAPI php_url *php_url_parse_ex2(char const *str, size_t length, bool *has_port
pp++;
}

if (pp - p > 0 && pp - p < 6 && (pp == ue || *pp == '/' || *pp == '?' || *pp == '#')) {
if (pp - p > 0 && pp - p < 6 && (pp == ue || *pp == '/')) {
zend_long port;
char *end;
memcpy(port_buf, p, (pp - p));
Expand Down