ryancramerdesign / ProcessWire

Our repository has moved to https://github.com/processwire – please head there for the latest version.
https://processwire.com
Other
727 stars 201 forks source link

Sanitizer url() method fails to sanitize correctly URLs with file extension (/products/tshirts.html) #836

Closed hwmaier closed 9 years ago

hwmaier commented 9 years ago

I am implementing a legacy site using URLs with extensions and came across an issue with the $sanitizer->url() method.

The following test snippet shows the issues:

echo wire()->sanitizer->url('/products/tshirts') . PHP_EOL; // OK
echo wire()->sanitizer->url('/products/tshirts.html') . PHP_EOL; // <== fails: empty string returned
echo wire()->sanitizer->url('products/tshirts') . PHP_EOL; // OK
echo wire()->sanitizer->url('products/tshirts.html') . PHP_EOL; // <== fails: scheme is wrongly added

It's the domain name regex match in the url() method which fails when using an extension. A simple fix I applied to get going is changing line 507 from:

if(strpos($value, '.') && preg_match('{^([^\s_.]+\.)?[^-_\s.][^\s_.]+\.([a-z]{2,6})([./:#]|$)}i', $value, $matches)) {

to:

if($value[0] != '/' && strpos($value, '.') && preg_match('{^([^\s_.]+\.)?[^-_\s.][^\s_.]+\.([a-z]{2,6})([./:#]|$)}i', $value, $matches)) {

but this fixes the issue only for URLs starting with a / character.

I came across the problem was detected when using the Redirect plugin which sanitizes URLs using this method.

ryancramerdesign commented 9 years ago

Thanks, I have pushed a fix for this that should cover both types of URLs you've mentioned.

hwmaier commented 9 years ago

Great, thanks for the fix, I can confirm its working in our test bed.