Url::ensureScheme does not work in some cases

[SamMousa]

What steps will reproduce the problem?

Url::ensureScheme('google.nl/test?param=https://someother.url/', 'https');

What is the expected result?

https://google.nl/test?param=https://someother.url/

What do you get instead?

google.nl/test?param=https://someother.url/

Additional info

This happens because this check is done:

if (($pos = strpos($url, '://')) !== false) {
            if ($scheme === '') {
                $url = substr($url, $pos + 1);
            } else {
                $url = $scheme . substr($url, $pos);
            }
        }

We should use parse_url to get the correct parts instead of using strpos.
If parse_url is not available then we could at least split the string by ? to ensure we ignore the query part.

[rob006]

google.nl/test?param=https://someother.url/ is relative URL, so ensureScheme() should return it as is, without any change. But I agree that there is a bug in isRelative() - it looks like it treats blablatest?param=https://someother.url/ as absolute URL.

[SamMousa]

Interesting, because even though isRelative is wrong, the same error is made in ensureScheme(), which in that case actually negates the erroneous behavior.

public static function ensureScheme($url, $scheme)
    {
        if (static::isRelative($url) || !is_string($scheme)) {
            return $url;
        }

        if (substr($url, 0, 2) === '//') {
            // e.g. //example.com/path/to/resource
            return $scheme === '' ? $url : "$scheme:$url";
        }

        if (($pos = strpos($url, '://')) !== false) {
            if ($scheme === '') {
                $url = substr($url, $pos + 1);
            } else {
                $url = $scheme . substr($url, $pos);
            }
        }

        return $url;
    }

[SamMousa]

It seems this approach is used more often, so my bug report was bad but there is an actual issue :-/

public function createAbsoluteUrl($params, $scheme = null)
    {
        $params = (array) $params;
        $url = $this->createUrl($params);
        if (strpos($url, '://') === false) {
            $hostInfo = $this->getHostInfo();
            if (strncmp($url, '//', 2) === 0) {
                $url = substr($hostInfo, 0, strpos($hostInfo, '://')) . ':' . $url;
            } else {
                $url = $hostInfo . $url;
            }
        }

        return Url::ensureScheme($url, $scheme);
    }

Basically strpos is not something to use on URLs.

[rob006]

ensureScheme() implementation will be correct if we fix isRelative(). createAbsoluteUrl() should probably also rely on isRelative() to make this behavior more consistent.

[SamMousa]

Yeah working on it