Bug #54369 [Com]: [PATCH] parse_url() incorrectly determines the start of query and fragment parts

Edit report at https://bugs.php.net/bug.php?id=54369&edit=1 ID: 54369 Comment by: acm at tweakers dot net 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: > 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.. Previous Comments: ------------------------------------------------------------------------ [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> ------------------------------------------------------------------------ [2013-05-31 13:09:55] woody dot gilk at gmail dot com According to RFC, the URL http://www.example.com?foo=bar is a completely valid URL. To quote: > For example, the URI <mailto:fred@example.com> has a path > of "fred@example.com", whereas the URI <foo://info.example.com?fred> has an empty path. There is nothing in the RFC spec that says a path must be included in the URL. Please fix this bug. ------------------------------------------------------------------------ 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