Improve santizer->httpUrl

[gRegorLove]

Short description of the issue

The $sanitizer->httpUrl() method lets through some invalid URLs unless you set the allowRelative option to false.

Expected behavior

I would expect this method to always return a string starting with http/https, or an empty string.

When $sanitizer->httpUrl() is called with an invalid URL and no options set, it should return an empty string.

E.g. $sanitizer->httpUrl('invalid url') should return an empty string.

Actual behavior

$sanitizer->httpUrl('invalid url') returns string invalid url

Optional: Suggestion for a possible fix

$sanitizer->httpUrl('invalid url', ['allowRelative' => false]) returns the expected empty string.

Could explicitly set the allowRelative option if it's not already set here:

public function httpUrl($value, $options = array())
{
    $options['requireScheme'] = true; 
    if (empty($options['allowRelative'])) {
        $options['allowRelative'] = false;
    } 
    if (empty($options['allowSchemes'])) {
        $options['allowSchemes'] = array('http', 'https');
    }
    return $this->url($value, $options);
}

Aside: I used PSR-12 coding style which requires braces around control structure bodies, and the body to be on the line after the opening brace.

Steps to reproduce the issue

$url = 'invalid url';

echo 'input url:' . PHP_EOL;
var_dump($url);

echo PHP_EOL . '$sanitizer->url():' . PHP_EOL;
var_dump($sanitizer->url($url));

echo PHP_EOL . '$sanitizer->httpUrl():' . PHP_EOL;
var_dump($sanitizer->httpUrl($url));

echo PHP_EOL . '$sanitizer->httpUrl() with allowRelative = false:' . PHP_EOL;
var_dump($sanitizer->httpUrl($url, ['allowRelative' => false]));

Output:

input url:
string(11) "invalid url"

$sanitizer->url():
string(11) "invalid url"

$sanitizer->httpUrl():
string(11) "invalid url"

$sanitizer->httpUrl() with allowRelative = false:
string(0) ""

Setup/Environment

• ProcessWire version: 3.0.165
• (Optional) PHP version: 7.4.15

[ryancramerdesign]

Thanks @gRegorLove -- that allowRelative should have been false from the beginning, so it looks like that was an oversight on my part. I've pushed your suggested fix for this.

[matjazpotocnik]

@gRegorLove based on your reaction, I'm assuming this is fixed, so I'm closing it.