Using space in Theme Name breaks down generated thumbnails

[vallka]

I made a copy of "Niara" theme and name it... "Niara Gel" (with a space). Regenerated images for products (didn't try anything else) include Theme Name, and the space inside the name has not be replaced with "_" or anything else. Thus srcset attributes became invalid (with space inside) and images are not showing on the homepage

[wakabayashi]

19 minutes ago, vallka said:

"Niara Gel"

This is aksing for a lot of trouble. Spacing in webfolders is never a good idea... Just go for "niaragel"... or "niara-gel" is probably also working.

Edited January 14, 2023 by wakabayashi

[Mediacom87]

Hi,

Your site is hosted on a Unix server and it is forbidden to use spaces.

If you use spaces, they must be encoded to be interpreted properly by Unix.

Perhaps a simple variable escape to treat them as URLs might work, but this is bad practice in any case, no capital letters and no spaces or special characters.

[vallka]

22 hours ago, wakabayashi said:

This is aksing for a lot of trouble. Spacing in webfolders is never a good idea... Just go for "niaragel"... or "niara-gel" is probably also working.

Of course it is very easy to fix on a local base, of course I have change the name of the theme immediately. Just wanted to point you to a bug in your product, it's very easy to fix if you know where the name is generated. You can replace the space with _ as already implemented, as I understand, in image file names. Or just use theme folder name instead of name here - chances are much less likely that anyone will use space in linux system directory name. And BTW, working with Prestashop before, I do not  remember that theme name has been use in these thumbnails names, just filenames with various suffixes... so probably this is a bug added by ThirtyBees

[e-com]

32 minutes ago, vallka said:

And BTW, working with Prestashop before, I do not  remember that theme name has been use in these thumbnails names, just filenames with various suffixes... so probably this is a bug added by ThirtyBees

This is a prestashop bug fixed in thirty bees. In multishop option, installing a new theme for other stores would mess up the image formats of other themes.

[datakick]

I don't think this is bug in core. It's bug in theme, though.

srcset="{$imageUrl|escape:'html':'UTF-8'} 211w"

The url in srcset attribute is escaped using 'html' escape. Spaces are allowed in html. It should be escaped using 'url'

srcset="{$imageUrl|escape:'url':'UTF-8'} 211w"

Space in url would be url-encoded to %20

You are right that this could be prevented by forcing image types to not allow spaces. It might make sense to do so, but it would also be very hard to roll out. Change like this would break a lot of stuff, like .htaccess rules, nginx config, force merchants to regenerate all images (and prune old ones), etc...

Also, it would only hide the actually bug, which is the escaping in theme (and other templates).

Note that his bug is not related to image types only. For example, if you deployed thirty bees in subdirectory that contains space, you would have the same problem. This time, invalid url would look like this:

https://www.server.com/subdirectory with space/img/1234-Niara_home.jpg

It would break srcset as well. And there might be other uses cases when wrong escaping can play a role. 

[vallka]

On 1/15/2023 at 7:21 PM, e-com said:

This is a prestashop bug fixed in thirty bees. In multishop option, installing a new theme for other stores would mess up the image formats of other themes.

This is exactly what I wanted to say. Fixing one bug has introduced yet another bug, what is absolutely normal in any software development. This new-introduced bug needs to be fixed in its turn, it's a normal working process.

BTW again. In my live Prestashop-1.7 based shop I'm using a free service named https://imagekit.io/ It takes ALL the headache of managing various sizes and formats of images and even free you from saving them on your system. I like it very much. Didn't actually measure the performance, seems to be quite ok. Anyone interested? :)

 

 

[vallka]

9 hours ago, datakick said:

I don't think this is bug in core. It's bug in theme, though.

srcset="{$imageUrl|escape:'html':'UTF-8'} 211w"

The url in srcset attribute is escaped using 'html' escape. Spaces are allowed in html. It should be escaped using 'url'

srcset="{$imageUrl|escape:'url':'UTF-8'} 211w"

Space in url would be url-encoded to %20

 

Thank you very much for this, datakick, this is exactly what I wanted to hear. I'll put this fix in my (from now on called "NiaraGel") theme, hope you'll do the same :)