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: 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. Previous Comments: ------------------------------------------------------------------------ [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. ------------------------------------------------------------------------ [2011-06-29 21:37:39] lenzai2004-dev at yahoo dot com The point is not about wether the patch is relevant or not. But for this bug and other cases, parse_url is returning corrupt result. It could be fixed in 2 ways: - patch it - or detect invalid url and return error. I've been trying to use this function and after significant volume of URLs I always find cases where it returns incorrect data. I had to rewrite everything in PHP and it's quite slow. ------------------------------------------------------------------------ [2011-05-17 20:12:50] tomas dot brastavicius at quantum dot lt Changed report name as described in the bug report spec. ------------------------------------------------------------------------ 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