Edit report at https://bugs.php.net/bug.php?id=54369&edit=1
ID: 54369
Comment by: tomas dot brastavicius at quantum dot lt
Reported by: tomas dot brastavicius at quantum dot lt
Summary: [PATCH] parse_url() incorrectly determines the start
of query and fragment parts
Status: Open
Type: Bug
Package: URL related
PHP Version: Irrelevant
Block user comment: N
Private report: N
New Comment:
This bug has been fixed in https://github.com/php/php-src/commit/b061fa909de77085d3822a89ab901b934d0362c4
The patch has been applied in >=5.6.28, >=7.0.13 and >=7.1.0 versions
One can check this on https://3v4l.org/
Previous Comments:
------------------------------------------------------------------------
[2016-08-21 15:54:25] acm at tweakers dot net
> It's not (only) about the given cases, but rather a general matter
> of BC break. Applying the given patch against current PHP-5.6
> lets 9 out of 42 test cases fail.
I was reading your comment as arguing against the reported issue, rather than the provided patch ;)
> It would be necessary to investigate on that further, and perhaps
> the patch would need to be updated/corrected. A PR would be
> welcome!
If the patch doesn't just fix the issue, but breaks on valid url's, than it should
obviously be fixed or replaced by another patch.
Unfortunately, my C-knowledge is limited. The php_url_parse_ex-function is therefore way to
complicated for me to fully understand, let alone fix it..
------------------------------------------------------------------------
[2016-08-21 11:42:10] cmb@php.net
> I'm not sure why you'd think users of parse_url would expect the
> reported outcome [â¦]
It's not (only) about the given cases, but rather a general matter
of BC break. Applying the given patch against current PHP-5.6
lets 9 out of 42 test cases fail.
It would be necessary to investigate on that further, and perhaps
the patch would need to be updated/corrected. A PR would be
welcome!
------------------------------------------------------------------------
[2016-08-08 21:28:48] acm at tweakers dot net
Apparently, this hasn't been fixed in 7.0
The reference to (deprecated) RFC1738, is actually interesting. In that RFC, a '?' or
'#' is not a valid part of the 'hostname' (only alphanum, '.' and
'-' are valid). So regardless of which of the two are supported, the host should not
contain those (i.e. the url is invalid according to RFC1738)
RFC 1738 seems to require a non-empty (i.e. '/') path if there is also a
'search'. And it has no support for fragments (so why is that in parse_url? ;) )
Anyway, I'd consider this report to be both valid against RFC1738 and RFC3986.
I'm not sure why you'd think users of parse_url would expect the reported outcome - that
is simply not a valid hostname (not in RFC1738 nor in RFC3986) - rather than either a false (invalid
url) or host+query or host+fragment.
------------------------------------------------------------------------
[2015-06-05 14:45:44] cmb@php.net
Oh, I forgot: <http://3v4l.org/PKG0q>.
------------------------------------------------------------------------
[2015-06-05 14:43:56] cmb@php.net
I'm not sure whether the issue qualifies as *bug* in PHP. The
documentation contains the following note[1]:
> This function is intended specifically for the purpose of
> parsing URLs and not URIs.
Therefore RFC 1738 is relevant, not RFC 3986. However, RFC 1738 is
obsolete...
Anyhow, I had a look at the tests patch, and quite obviously the
main patch enforces some behavioral changes. That might be
considered a BC break for PHP 5, so perhaps it's best to treat
this ticket as feature request, and to improve parse_url() for PHP
7 only.
As the behavioral changes don't appear to cause profound BC
breaks, it seems to me that these changes don't require an RFC. A
PR[2] might be helpful to get more attention to this issue,
though. Are you willing to make a PR, Tomas?
[1] <http://php.net/manual/en/function.parse-url.php#refsect1-function.parse-url-notes>
[2] <https://github.com/php/php-src/pulls>
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=54369
--
Edit this bug report at https://bugs.php.net/bug.php?id=54369&edit=1