parse_url() and incorrect port definition

[MioVisman]

Description

The following code:

<?php

$url = '//www.examle.com/ddd:65535/';

echo "<pre>\n";
var_export(parse_url($url));
echo "</pre>\n";

exit;

Resulted in this output:

array (
  'host' => 'www.examle.com',
  'port' => 65535,
  'path' => '/ddd:65535/',
)

But I expected this output instead:

array (
  'host' => 'www.examle.com',
  'path' => '/ddd:65535/',
)

If url starts with // and path ends with :{num}/, then:

1. num = 0-65535 -> num gets into port and path
2. num = 65536-99999 -> the function returns fasle instead of an array
3. num < 0 or > 99999 -> all ok

P.S. https://www.php.net/manual/ru/function.parse-url.php#122207

PHP Version

PHP 8.1.1-Win32-vs16-x64, 7.3.33

Operating System

Windows 7 x64

[cmb69]

The documentation states:

Partial and invalid URLs are also accepted, parse_url() tries its best to parse them correctly.

That best effort fails here, and in many other cases, and I don't think there is any reliable way to correctly parse all partial URLs (except maybe for some clear and restricting definition of "partial"). You might be better off using a regex to parse URLs. parse_url() can't do this for BC reasons.

IMO this is "Invalid" or "Won't fix", or maybe a feature request.

[MioVisman]

I believe the parse_url() function should be fixed. url starting with // are used everywhere on the web. And regular expressions are slower + require additional checks if they are made less complex.

This simple regex

%^
(?:([a-z][a-z0-9+.-]+):)?+
(?://
    (?:([^@/?#]*)@)?+
    (\[[^/?#\[\]]+\]|[^:/?#\[\]]+)
    (?::(\d+))?+
    (?=[/?#]|$)
)?
((?!(?(3)^|//))(?!\s)[^?#]+)?
(?:\?([^#]*))?
(?:\#([^\s]*))?
(?<!\s)$%ix

is 2-3 times slower than parse_url() on my computer.

So far I made such a bike:

function url(string $url): string
{
    if (! isset($url[2])) {
        return '';
    } elseif ('#' === $url[0]) {
        $schemeOn = false;
        $hostOn   = false;
        $url      = 'http://a.a' . $url;
    } elseif ('/' === $url[0]) {
        if ('/' === $url[1]) {
            $schemeOn = false;
            $hostOn   = true;
            $url      = 'http:' . $url;
        } else {
            $schemeOn = false;
            $hostOn   = false;
            $url      = 'http://a.a' . $url;
        }
    } elseif (! \preg_match('%^[a-z][a-z0-9+.-]{0,9}:%i', $url)) {
        return '';
    } else {
        $schemeOn = true;
        $hostOn   = true;
    }

    $p = \parse_url($url);

    if (! \is_array($p)) {
        return '';
    }
...

[KapitanOczywisty]

Regex is crazy fast if properly written (backtracking is the common performance killer; debug with https://regex101.com/ or https://www.debuggex.com/), and even 20 times is nothing, unless the only job of your app is parsing urls. Do not optimize prematurely.

parse_url is ok-ish for known urls (e.g. from config file), anything from user should go through packages like league/uri anyway.

As for this specific url: it is technically valid as a relative url, so if parse_url can parse relative paths, should also parse that url. OP is probably referring to use of //... in 3rd-party scripts in the past, but at this point https:// should be used instead. And this works because url is treated as relative to page origin. If you try to parse url in JS you will get error, but if you also provide base url, the parsing is successful.

new URL('//www.examle.com/ddd:65535/')
// TypeError: URL constructor: //www.examle.com/ddd:65535/ is not a valid URL.
new URL('//www.examle.com/ddd:65535/', 'http://foo.com')
// URL { href: "http://www.examle.com/ddd:65535/", origin: "http://www.examle.com", protocol: "http:", username: "", password: "", host: "www.examle.com", hostname: "www.examle.com", port: "", pathname: "/ddd:65535/", search: "" }

Edit:
With regex variant, named groups are pretty useful.

$url = '//www.examle.com/ddd:65535/';
preg_match("%^
(?:(?<scheme>[a-z][a-z0-9+.-]+):)?+
(?://
    (?:(?<userpass>[^@/?#]*)@)?+
    (?<host>\[[^/?#\[\]]+\]|[^:/?#\[\]]+)
    (?::(?<port>\d+))?+
    (?=[/?#]|$)
)?
(?<path>(?!(?(3)^|//))(?!\s)[^?#]+)?
(?:\?(?<query>[^#]*))?
(?:\#(?<fragment>[^\s]*))?
(?<!\s)$%ix", $url, $matches);
print_r($matches);

Array
(
    [0] => //www.examle.com/ddd:65535/
    [scheme] => 
    [1] => 
    [userpass] => 
    [2] => 
    [host] => www.examle.com
    [3] => www.examle.com
    [port] => 
    [4] => 
    [path] => /ddd:65535/
    [5] => /ddd:65535/
)

[cmb69]

As for this specific url: it is technically valid as a relative url, so if parse_url can parse relative paths, should also parse that url.

The problem is the way php_url_parse_ex2() is implemented; it is not a proper parser, but rather does some guesswork (which is supposed to work for absolute URLs, but may fail badly for others). In this case it looks for the first : and checks whether this is preceeded by a valid scheme. Obviously, this is not the case, so it checks whether the following characters form a valid port, what is the case.

Now try to fix that without breaking existing code which may even rely on parsing invalid URLs. Best of luck! :)

So, in my opinion, parse_url() is beyond repair and needs to be replaced by some sane URI parser (adhering to whatever specification). I'm aware that I sound like a broken record.

[KapitanOczywisty]

Before port you can have only double slash - if there is anything more - it's not a port. I think it's still possible to write proper url parsing which can handle invalid urls, but it cannot relay on : as a major part, it should be some state-based algorithm. NodeJS implementation could probably parse invalid urls, but it's way more complex. Even league/uri package is capable of parsing broken URLs. Maybe this is the place for replacement of parse_url with object-based internal extension. new URL(...) with isValid etc, but with performance of native modules.

[ErikThiart]

Can one not add some kind of hard default like if the url starts with // then it is assumed https:// and port 443

or what am I missing here?

I guess safer even is http and port 80 as the onus is then on the web server to redirect to https if it is available.

[iluuu1994]

I agree with @cmb69, especially if there have been previous failed attempts to fix similar issues. Additionally, I think trying to fix invalid URLs is inherently unsafe specifically because the correct behavior is undefined and different parsers might fix URLs in different ways which could be exploitable. IMO it would be better to introduce a new parser that closely follows the specification and rejects invalid URLs.