sitegeist / sms-responsive-images

This TYPO3 extension provides ViewHelpers and configuration to render valid responsive images based on TYPO3's image cropping tool.
GNU General Public License v2.0
34 stars 19 forks source link

[BUGFIX] Fix #45: bad srcset is generated for some images #46

Closed dmitryd closed 5 years ago

dmitryd commented 5 years ago

Fixes the issue by encoding every segment of the image path.

s2b commented 5 years ago

Oops, nice edge case... Thanks for discovering it and for providing a patch! 👍 I'm not convinced yet that modifying the url like this is the best solution as TYPO3 doesn't seem to do it for image urls. How do you think about a minimal solution like this?

    /**
     * Ensures that the provided url can be used safely in a srcset attribute
     *
     * @param string $url
     *
     * @return string
     */
    public function sanitizeSrcsetUrl(string $url): string
    {
        return strtr($url, [
            ' ' => rawurlencode(' '),
            ',' => rawurlencode(',')
        ]);
    }

This would address the problem at hand without "messing" with other parts of the url.

dmitryd commented 5 years ago

@s2b

I'm not convinced yet that modifying the url like this is the best solution as TYPO3 doesn't seem to do it for image urls

TYPO3 does not use srcset. The problem is that space and comma is ok in src bit not allowed in srcset. So TYPO3 does not have to encode it though it should...

The function you propose should also work. Except that I would not call rawurlencode for fixed characters but simply use %20 and %2C:

     return strtr($url, [
            ' ' => %20,
            ',' => %2C
     ]);

I used rawurlencode because of possible special unicode characters that may be treated wrong. But this is even more extreme case then we have here :)