Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

v:uri.image and v:image do prefix external image urls with domain of TYPO3 site #1439

Open
maechler opened this issue Jan 19, 2018 · 5 comments · May be fixed by #1470
Open

v:uri.image and v:image do prefix external image urls with domain of TYPO3 site #1439

maechler opened this issue Jan 19, 2018 · 5 comments · May be fixed by #1470

Comments

@maechler
Copy link
Contributor

If you have a storage configured that generates urls for another host, these urls get prefixed with the current domain. For instance this happens with our AWS storage:

Website Domain: www.example.ch
Storage Url: example.ch.s3.amazonaws.com/example.png
Generated Image source: www.example.chexample.ch.s3.amazonaws.com/example.png

The problem is, that the following function always prepends $GLOBALS['TSFE']->absRefPrefix even though its description states, that it would only do so if necessary.
https://github.com/FluidTYPO3/vhs/blob/development/Classes/ViewHelpers/Media/AbstractMediaViewHelper.php#L59-L68

In contrast to this the TYPO3 Core Fluid ViewHelper only prefixes the url if it is not already fully qualified.
See https://github.com/TYPO3-CMS/extbase/blob/master/Classes/Service/ImageService.php#L86-L93

One solution would be to implement a check similar to what TYPO3 Core does:

    public static function preprocessSourceUri($src, array $arguments)
    {
        $parsedUrl = parse_url($src);

        // no prefix in case of an already fully qualified URL
        if (isset($parsedUrl['host'])) {
            if (!isset($parsedUrl['scheme'])) {
                $src = (GeneralUtility::getIndpEnv('TYPO3_SSL') ? 'https:' : 'http:') . $src;
            }
        } else {
            $src = $GLOBALS['TSFE']->absRefPrefix . str_replace('%2F', '/', rawurlencode($src));
        }

        if (false === empty($GLOBALS['TSFE']->tmpl->setup['plugin.']['tx_vhs.']['settings.']['prependPath'])) {
            $src = $GLOBALS['TSFE']->tmpl->setup['plugin.']['tx_vhs.']['settings.']['prependPath'] . $src;
        } elseif ('BE' === TYPO3_MODE || false === (boolean) $arguments['relative']) {
            $src = GeneralUtility::getIndpEnv('TYPO3_SITE_URL') . ltrim($src, '/');
        }
        
        return $src;
    }

What do you think? Are there other ways to fix this?
@NamelessCoder
Copy link
Member

NamelessCoder commented Jan 20, 2018 •
edited

Hi Markus,

Just one question from me: are you accessing these AWS files via a FAL driver, meaning they exist in sys_file and that you pass their UIDs or File objects using the image argument on those ViewHelpers?

If that is not the case, you should simply not be using these ViewHelpers at all. The main reason for using them would be to cause image resizing or create multiple versions - they are not intended to process already absolute URLs. But if the FAL driver you use yields an absolute URL as identifier for the asset then yes, I suppose that could trigger the case above and could use a check - but I'm not sure if 1) that's valid use of FAL, 2) whether Fluid's ViewHelpers has this check for legacy support of absolute URLs or to support an actual, proper FAL use case... your answer would clear that up ;)

@maechler
Copy link
Contributor Author

We are using the extension fal_s3 driver (https://extensions.typo3.org/extension/fal_s3/) which produces absolute URLs: https://github.com/MaxServ/t3ext-fal_s3/blob/master/Classes/Driver/AmazonS3Driver.php#L247-L250

The files exist in sys_file and we need your ViewHelper to be able to resize images.

@NamelessCoder
Copy link
Member

NamelessCoder commented Jan 22, 2018 •
edited

I took a closer look and these ViewHelpers simply don't support FAL objects which use absolute URLs as identifier. It looks like you will need to pass the local version of the file (call getFileForLocalProcessing() in the FAL driver and use the return value as src) since these ViewHelpers explicitly only support string sources of local files. The reason it works with a File instance is that this gets cast to a string (with the value of identifier) and your FAL driver's identifiers are absolute URLs.

These ViewHelpers will simply not be able to resize any such image, which makes the point that they prefix the domain name, rather moot: had the VH been able to resize remote images, the resized version would not be placed on the remote - they would be placed in the local filesystem and the resized file's identifier would be a relative path; which in turn would require the prefixing of absRefPrefix.

It is unlikely that these VHS ViewHelpers will receive extensive FAL supports along these lines. Instead, you should be using the native image ViewHelpers - and if they lack functionality in terms of using the FAL driver correctly, the issues should be reported to TYPO3. You may also wish to request a new feature from TYPO3 to give you a getter method on File which would return the local processing path - then you could pass {image.localPath} or other as src when the VH you must use, only supports a local file.

In your case the right solution would be to either call getFileForLocalProcessing and use the resulting local file path as input when you want your images to be resized by VHS ViewHelpers, or use the core's own image ViewHelpers. That's a fairly quick fix!

@maechler
Copy link
Contributor Author

Thanks for your analysis!

We are not using the core's ViewHelpers because we also want to be able to change the quality and image format, which works perfectly fine with your ViewHelpers. We have a special setup as we do not only use the fal_s3 extension but also a filesystem mount of the S3 Bucket. That is because a lot of extensions do not completely support FAL. So in our case the only problem is getting the url rendered without having it prefixed. But I see that this is rather a rare case and it would be quite some work to support FAL completely. We will probably fix this by extending your ViewHelpers unless you are interested in including this.

@NamelessCoder
Copy link
Member

If you do create custom ViewHelpers for these I'd love to see those changes as a pull request to VHS. If the changes are less extensive than I think they'll be, I wouldn't mind including it. Our priorities would be to not change public API or remove current supports. And the less code there is to maintain, the better.

@Lagerregal Lagerregal mentioned this issue Feb 16, 2018
Closed
@maechler maechler linked a pull request Apr 3, 2018 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Adds support for external image urls
2 participants
@NamelessCoder @maechler