rosell-dk / webp-express

Wordpress plugin for serving autogenerated WebP images instead of jpeg/png to browsers that supports WebP
GNU General Public License v3.0
227 stars 64 forks source link

File that contains an & breaks the webp-on-demand.php script #430

Open andreiglingeanu opened 4 years ago

andreiglingeanu commented 4 years ago

Hi!

I just discovered that when you try to load a file that has an & in its path, the webp-on-demand.php script fails with this:

Sanity check failed for source (passed as absolute path on nginx): File does not exist or is outside restricted basedir

I've debugged this a bit and the cause seems to be that when you pass xsource=x$request_filename query string and it contains an ampersand -- it breaks the xsource query param into multiple query params. The full nginx rewrite rule that I currently use:

rewrite ^/site/wp-content/.*\.(jpe?g|png)$ /site/wp-content/plugins/webp-express/wod/webp-on-demand.php?xsource=x$request_filename&wp-content=/site/wp-content last;

URI path to the file: /site/wp-content/uploads/brizy/395/assets/images/iW=5000&iH=any/andrew-neel-jtsW-Z6bFw-unsplash.jpg File system path to the same file: /var/www/html/site/wp-content/uploads/brizy/395/assets/images/iW=5000&iH=any/andrew-neel-jtsW-Z6bFw-unsplash.jpg

// print_r($_GET) in webp-express/lib/classes/WebPOnDemand.php
Array
--
  | (
  | [xsource] => x/var/www/html/site/wp-content/uploads/brizy/395/assets/images/iW=5000
  | [iH] => any/andrew-neel-jtsW-Z6bFw-unsplash.jpg
  | [wp-content] => /site/wp-content
  | )

Possible solutions:

  1. Achievable in userland: install Lua module and with it do a urlencode on $request_filename
  2. Allow xsource to be passed in as a CGI param, and thus not interfere with the whole URL thing.

Do you think there are other ways of mitigating this problem? The & and = signs are unfortunately very hard to remove, they're coming from a popular page builder...

cheuksing commented 4 years ago

I think this is an url encode issue. I can also confirm this issue occurs on non-English file name images.

Except & and =, % should also be handled.

For example, if you have /uploads/煙絲.JPG its actual URL within WordPress is /uploads/%E7%85%99%E7%B5%B2.JPG

andreiglingeanu commented 4 years ago

@cheuksing Yeah, this was quite annoying and hard to spot at the beginning... I imagine this is something that's not easily fixable either.

WPopt commented 3 years ago

Found that this also happens with images with a "+" in the filename too (for example "beautiful+flower.jpg".

FaisalAhmed123 commented 1 year ago

I had this happen when I bulk converted images, and now all imaged come with the error "Sanity check failed for destination path: String expected"

Is there any way to resolve this ? Is there any backup of original photos