Project

General

Profile

Actions

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:
10
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 #1

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 #2

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 #3

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 #4

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 #5

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 #6

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 #7

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 #8

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 #9

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 #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 #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 #12

Updated by Philipp Kitzberger about 1 year ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions #13

Updated by Benni Mack 5 months ago

  • Status changed from Resolved to Closed
Actions

Also available in: Atom PDF