Fix #80114: parse_url does not accept URLs with port 0#6152
Fix #80114: parse_url does not accept URLs with port 0#6152cmb69 wants to merge 4 commits intophp:PHP-7.3from
Conversation
URIs with a 0 port are generally valid, so `parse_url()` should recognize such URIs, but still report the port as missing.
ext/standard/url.c
Outdated
| port_buf[e - p] = '\0'; | ||
| port = ZEND_STRTOL(port_buf, NULL, 10); | ||
| if (port > 0 && port <= 65535) { | ||
| if ((port > 0 && port <= 65535) || (port == 0 && *port_buf == '0' && e - p == 1)) { |
There was a problem hiding this comment.
Why the additional checks here? If we accept :001 for other port numbers, I don't think zero should get different treatment than that.
There was a problem hiding this comment.
The problem is that port==0 also if there is no digit (e.g. http://example.com:foo/). We could drop the e-p==1 check to be more liberal, though.
There was a problem hiding this comment.
Hi, I was just checking how the contributing process with PHP works and found this bug quite interesting for a beginner like me.
Just one question, why not just check if the port >= 0 in this case? In the case that the port is ":foo" I believe it's invalid no? Since the RFC says that the port number should be a decimal number
There was a problem hiding this comment.
I suggest that you try that change (checking for port >= 0) and run the test in ext/standard/tests/url/ – there'll be several failures; check the *.diff files. :) (Hint: ZEND_STROL() is similar to the (int) cast in PHP: both ignore any trailing garbage.
There was a problem hiding this comment.
Nice I will try to play a bit with the source code, tks man
How about this way, and it also needs to be applied to `//example.com:0`
Another way
nikic
left a comment
nikic
left a comment
There was a problem hiding this comment.
It might make more sense to add the test to url.inc instead.
|
Oh, that makes sense. Thanks! |
4 participants
URIs with a 0 port are generally valid, so
parse_url()shouldrecognize such URIs, but still report the port as missing.