Url::ensureScheme('google.nl/test?param=https://someother.url/', 'https');
https://google.nl/test?param=https://someother.url/
google.nl/test?param=https://someother.url/
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.
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.
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;
}
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.
ensureScheme() implementation will be correct if we fix isRelative(). createAbsoluteUrl() should probably also rely on isRelative() to make this behavior more consistent.
Yeah working on it
Most helpful comment
Yeah working on it