Project

General

Profile

Actions
Copy link

Bug #97446

closed

VimeoHelper and VimeoRenderer build wrong URLs

Added by Gerald Rintisch about 2 years ago. Updated 7 months ago.

Status:
Resolved
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
Copy link
 #1

Updated by Frank Nägler almost 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
 #2

Updated by Leonie Philine almost 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
 #3

Updated by Leonie Philine almost 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
 #4

Updated by Markus Klein over 1 year ago

  • Status changed from New to Accepted
  • Target version deleted (Candidate for patchlevel)
  • TYPO3 Version changed from 11 to 10
Actions
Copy link
 #5

Updated by Dmitry Dulepov over 1 year 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
 #6

Updated by Gerrit Code Review over 1 year 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
 #7

Updated by Gerrit Code Review over 1 year 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
 #8

Updated by Gerrit Code Review almost 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
 #9

Updated by Gerrit Code Review 7 months 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 7 months 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 7 months 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

Updated by Philipp Kitzberger 7 months ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
Actions
Copy link

Also available in: Atom PDF