Bug #97446: VimeoHelper and VimeoRenderer build wrong URLs - TYPO3 Core - TYPO3 Forge

Actions

Copy link

Bug #97446

closed

VimeoHelper and VimeoRenderer build wrong URLs

Added by Gerald Rintisch over 2 years ago. Updated 5 months ago.

Status:

Closed

Priority:

Should have

Assignee:

Category:

File Abstraction Layer (FAL)

Target version:

Start date:

2022-04-22

Due date:

% Done:

100%

Estimated time:

TYPO3 Version:

PHP Version:

8.1

Tags:

vimeo, video

Complexity:

easy

Is Regression:

Sprint Focus:

Description

In Vimeo you can copy a link to a video in two different (relevant) ways:

Copy Video Link returns scheme

vimeo.com/<videocode>/<optionalPrivateCode>

Copy embed code returns scheme

<iframe src="https://player.vimeo.com/video/<videocode>?h=<optionalPrivateCode>"  ... ></iframe>

So here are 2 problems to solve:
1. The regex in \TYPO3\CMS\Core\Resource\OnlineMedia\Helpers\VimeoHelper::transformUrlToFile must adapted to also recognize the code of the embed snippet.
2. Storing (in \TYPO3\CMS\Core\Resource\Rendering\VimeoHelper::transformUrlToFile()) our outputting (in VimeoRenderer::createVimeoUrl()) must be adapted so that the created URL contains the paramater h (so .../<videocode>?h=<optionalPrivateCode>&... instead of .../<videocode>/<optionalPrivateCode>?...

Under the condition that the two parts are still stored with / a solution would be to write https://github.com/TYPO3/typo3/blob/main/typo3/sysext/core/Classes/Resource/Rendering/VimeoRenderer.php#L155

$videocode = $videoId;
if(str_contains('/', $videoId)){
    $videoIdElements = explode('/',$videoId);
    $videocode = $videoIdElements[0];
    $optionalPrivateCode = $videoIdElements[1];
    $urlParams[] = 'h=' . $optionalPrivateCode;
}

return sprintf('https://player.vimeo.com/video/%s?%s', $videocode, implode('&', $urlParams));

But I am sure here are people capable to make it more elegant...

Actions

Copy link

Updated by Frank Nägler over 2 years ago

Here is a kind of reference for possible URL schema: https://developer.vimeo.com/api/oembed/videos
The whole parser looks a bit wired and should maybe be refactored. E.g. by using an URL object to parse the data instead of doing the string parsing with RegExp.
Also, the tests should be checked and verified.

Actions

Copy link

Updated by Leonie Philine over 2 years ago

There's another issue in https://github.com/TYPO3/typo3/blob/ae00ad424a1b04ab30712d4558acab276396cb13/typo3/sysext/core/Classes/Resource/OnlineMedia/Helpers/VimeoHelper.php#L99

rawurlencode(sprintf('https://vimeo.com/%s', rawurlencode($mediaId)))

The media ID is double URL encoded. This causes the slash between video ID and video hash to be encoded as "%252F" rather than "%2F". (The added "%25" is the second-time-encoded percent sigil.)

Correct would be:

rawurlencode(sprintf('https://vimeo.com/%s', $mediaId))

Example with a fake 'unlisted' video URL:

Original URL:
https://vimeo.com/123456789/abcdef1234

Correct (succeeding) encoded URL to attach to "https://vimeo.com/api/oembed.json?width=2048&url=":
https%3A%2F%2Fvimeo.com%2F7123456789%2Fabcdef1234

Incorrect (failing) encoded URL which TYPO3 actually attaches instead (with "%252F" rather than "%2F"):
https%3A%2F%2Fvimeo.com%2F123456789%252Fabcdef1234

Actions

Copy link

Updated by Leonie Philine over 2 years ago

Proposed elegant solution for the vimeo renderer (replacing lines 136-147 https://github.com/TYPO3/typo3/blob/main/typo3/sysext/core/Classes/Resource/Rendering/VimeoRenderer.php#L136-L137):

protected function createVimeoUrl(array $options, FileInterface $file)
    {
        [$videoId, $videoHash] = explode('/', $this->getVideoIdFromFile($file), 2);

        $urlParams = [];

        if (!empty($videoHash)) {
            $urlParams[] = 'h=' . $videoHash;
        }

        // Rest of the method remains unchanged.
    }

Actions

Copy link

Updated by Markus Klein almost 2 years ago

Status changed from New to Accepted
Target version deleted (~~Candidate for patchlevel~~)
TYPO3 Version changed from 11 to 10

Actions

Copy link

Updated by Dmitry Dulepov almost 2 years ago

I came to a similar solution:

         $videoId = $this->getVideoIdFromFile($file);
         $urlParams = [];
+        if (str_contains($videoId, '/')) {
+            [$videoId, $privateId] = explode('/', $videoId, 2);
+            $urlParams[] = 'h=' . htmlspecialchars($privateId);
+        }
         if (!empty($options['autoplay'])) {
             $urlParams[] = 'autoplay=1';
         }

Actions

Copy link

Updated by Gerrit Code Review almost 2 years ago

Status changed from Accepted to Under Review

Patch set 1 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/77609

Actions

Copy link

Updated by Gerrit Code Review almost 2 years ago

Patch set 2 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/77609

Actions

Copy link

Updated by Gerrit Code Review over 1 year ago

Patch set 3 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/77609

Actions

Copy link

Updated by Gerrit Code Review about 1 year ago

Patch set 4 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/77609

Actions

Copy link

#10

Updated by Gerrit Code Review about 1 year ago

Patch set 5 for branch main of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/77609

Actions

Copy link

#11

Updated by Gerrit Code Review about 1 year ago

Patch set 1 for branch 12.4 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/81070

Actions

Copy link

#12