Base-url mismatch on domain with country suffixes

[lphilippo]

Hello all,

Although we have used this module for various sites without any issue, we only just stumbled upon a challenge that we wanted to share here. We found that the current rules that determine whether or not an image URL qualifies for conversion or not, go wrong in a specific use case, involving multiple languages.

If we have various shops with a shared media folder, such as:

https/www.domain.com/en
https/www.domain.com/de

And the media base URL being:

https/www.domain.com/media

None of the media catalog images are being replaced, as they will all disqualify in:

\Yireo\NextGenImages\Image\HtmlReplacer::isAllowedByImageUrl

The base URLs are in this situation never part of the image URL, as the conflict is caused by the country suffix.

There are various straightforward possibilities :

1. Using $this->url->getBaseUrl(['_type' => UrlInterface::URL_TYPE_MEDIA]); here. However, this would have the downside that any images coming from your theme would no longer be replaced.

2. Introduce a helper function to retrieve only the domain of the baseUrl. This would sort out our situation, but could still cause similar issues if you use a separate subdomain for your media.

3. Extend the check to determine if either the baseUrl or the media baseUrl matches with the provided $imageUrl. This is the one we went for on our environment.

We are more than happy to provide a PR for this, once we know if you agree with the adjustment.

Kind regards

[barryvdh]

I created https://github.com/yireo/Yireo_NextGenImages/pull/9 But you are right, we could probably check both the media or the theme url.

[tdgroot]

Perhaps checking both URLs is better, yes.

Resizing images from themes are a bit weird though, because the resize would need to happen each deployment (as the files are located in the 'generated' pub directory). It would be better to include webp images in the theme rather than relying on the WebP module.

[barryvdh]

I think I agree that it's better to just modify the media images.

[lphilippo]

The reason why we checked both URLs, was that the we didn't want to limit how much the base and media URLs could differ (only prefix, subdomain etc...).

The reason why we kept the theme images (base URL) as well, was that those images can be quite significant (think footer/header images) and we did not want to force webp directly in the theme.

With regards to the deployment, the same concern would apply if you work with remote storage for catalog images, as after each deploy these images are "re-created" in the new pub directory, including the webp generation. Currently the module does not have an explicit distinction between theme and media images, and personally I would not introduce that now.

[jissereitsma]

Currently, 0.2.5 is released with a separate class that should be able to convert URLs to filenames (and there is a CLI command next-gen-images:test-uri to see if this actually works). Does this work to solve the issue with country suffixes as well?

[lphilippo]

Without trying to get to too many edge cases here, I think release 0.2.5 almost resolve all possibilities here.

The only issue at hand is that with a country suffix the /static/ directory is no longer recognised, where without country suffix this works correctly.

Without suffix: https://www.example.com/ has the static url at https://www.example.com/static/ which passes the checks in `isLocal'.

With suffix: https://www.example.com/nl/, and media at https://www.example.com/media/, the static url remains https://www.example.com/static/, however this time it does not pass the checks in `isLocal'.

So to really allow the same files in both situation, the method Image/UrlConvertor::isLocal should keep in mind both UrlInterface::URL_TYPE_MEDIA as UrlInterface::URL_TYPE_STATIC...

[jissereitsma]

While this has been open for a long time, this is now fixed in NextGenImages 0.3.0. Does this work for you too?

[jissereitsma]

I'm closing this ticket due to inactivity.