Verified Commit 3186130d authored by Dave Long's avatar Dave Long
Browse files

fix: #3560260 OEmbedIframeController makes responses uncacheable by mistake 

By: arousseau
By: smustgrave
By: jonhattan
(cherry picked from commit 867a3c06)
parent 38f070c8

core/modules/media/src/Controller/OEmbedIframeController.php

+17 −9
Original line number Diff line number Diff line
@@ -5,11 +5,11 @@ 
use Drupal\Component\Utility\Crypt;

use Drupal\Core\Cache\CacheableMetadata;

use Drupal\Core\DependencyInjection\ContainerInjectionInterface;

use Drupal\Core\Http\Exception\CacheableBadRequestHttpException;

use Drupal\Core\Render\BubbleableMetadata;

use Drupal\Core\Render\HtmlResponse;

use Drupal\Core\Render\RenderContext;

use Drupal\Core\Render\RendererInterface;

use Drupal\Core\Url;

use Drupal\media\IFrameMarkup;

use Drupal\media\IFrameUrlHelper;

use Drupal\media\OEmbed\ResourceException;
@@ -18,7 +18,6 @@ 
use Psr\Log\LoggerInterface;

use Symfony\Component\DependencyInjection\ContainerInterface;

use Symfony\Component\HttpFoundation\Request;

use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;



/**

 * Controller which renders an oEmbed resource in a bare page (without blocks).
@@ -124,12 +123,18 @@ public static function create(ContainerInterface $container) { 
   */

  public function render(Request $request) {

    // @todo Move domain check logic to a separate method.

    $allowed_domain = \Drupal::config('media.settings')->get('iframe_domain');

    $media_settings = \Drupal::config('media.settings');



    // The response will vary based on the module's settings and the request

    // url's scheme and host.

    $cache = (new CacheableMetadata())->addCacheableDependency($media_settings)->addCacheContexts(['url.site']);



    $allowed_domain = $media_settings->get('iframe_domain');

    if ($allowed_domain) {

      $allowed_host = parse_url($allowed_domain, PHP_URL_HOST);

      $host = parse_url($request->getSchemeAndHttpHost(), PHP_URL_HOST);

      if ($allowed_host !== $host) {

        throw new BadRequestHttpException('This resource is not available');

        throw new CacheableBadRequestHttpException($cache, 'This resource is not available');

      }

    }
@@ -137,20 +142,23 @@ public function render(Request $request) { 
    $max_width = $request->query->getInt('max_width');

    $max_height = $request->query->getInt('max_height');



    // The hash parameter is generated using the whole URL, so add it as a cache

    // context.

    $cache->addCacheContexts(['url']);



    // Hash the URL and max dimensions, and ensure it is equal to the hash

    // parameter passed in the query string.

    $hash = $this->iFrameUrlHelper->getHash($url, $max_width, $max_height);

    if (!hash_equals($hash, $request->query->get('hash', ''))) {

      throw new BadRequestHttpException('This resource is not available');

      throw new CacheableBadRequestHttpException($cache, 'This resource is not available');

    }



    // Return a response instead of a render array so that the frame content

    // will not have all the blocks and page elements normally rendered by

    // Drupal.

    $response = new HtmlResponse('', HtmlResponse::HTTP_OK, [

    $response = (new HtmlResponse('', HtmlResponse::HTTP_OK, [

      'Content-Type' => 'text/html; charset=utf-8',

    ]);

    $response->addCacheableDependency(Url::createFromRequest($request));

    ]))->addCacheableDependency($cache);



    try {

      $resource_url = $this->urlResolver->getResourceUrl($url, $max_width, $max_height);
@@ -207,7 +215,7 @@ public function render(Request $request) { 
    }

    catch (ResourceException $e) {

      // Prevent the response from being cached.

      $response->setMaxAge(0);

      $response->getCacheableMetadata()->setCacheMaxAge(0);



      // The oEmbed system makes heavy use of exception wrapping, so log the

      // entire exception chain to help with troubleshooting.

core/modules/media/tests/src/Kernel/OEmbedIframeControllerTest.php

+118 −0
Original line number Diff line number Diff line
@@ -8,6 +8,9 @@ 
use Drupal\media\Controller\OEmbedIframeController;

use Drupal\media\OEmbed\Provider;

use Drupal\media\OEmbed\Resource;

use Drupal\media\OEmbed\ResourceException;

use Drupal\media\OEmbed\ResourceFetcherInterface;

use Drupal\media\OEmbed\UrlResolverInterface;

use Drupal\TestTools\Random;

use PHPUnit\Framework\Attributes\CoversClass;

use PHPUnit\Framework\Attributes\DataProvider;
@@ -116,4 +119,119 @@ public function testResourcePassedToPreprocess(): void { 
    $this->assertStringContainsString('text/html', $response->headers->get('Content-Type'));

  }



  /**

   * Tests that the response max age is set to the same value as that of the resource.

   *

   * @return void

   *   No return value.

   */

  public function testResponseCacheMaxAge(): void {

    $hash = $this->container->get('media.oembed.iframe_url_helper')

      ->getHash('', 0, 0);



    $url_resolver = $this->prophesize(UrlResolverInterface::class);

    $resource_fetcher = $this->prophesize(ResourceFetcherInterface::class);



    $provider = new Provider('YouTube', 'https://youtube.com', [

      [

        'url' => 'https://youtube.com/foo',

      ],

    ]);

    $resource = Resource::rich(

      '<iframe src="https://youtube.com/watch?feature=oembed"></iframe>',

      320,

      240,

      $provider,

      cache_age: 1234

    );



    $resource_fetcher->fetchResource(Argument::cetera())->willReturn($resource);



    $this->container->set('media.oembed.url_resolver', $url_resolver->reveal());

    $this->container->set('media.oembed.resource_fetcher', $resource_fetcher->reveal());



    $response = OEmbedIframeController::create($this->container)

      ->render(new Request([

        'url' => '',

        'hash' => $hash,

      ]));

    assert($response instanceof HtmlResponse);



    $this->assertEquals(1234, $response->getCacheableMetadata()->getCacheMaxAge());

  }



  /**

   * Tests that the response max age is set to 0 when a ResourceException is raised while fetching the resource.

   *

   * @return void

   *   No return value.

   */

  public function testResponseCacheMaxAgeUponResourceException(): void {

    $hash = $this->container->get('media.oembed.iframe_url_helper')

      ->getHash('', 0, 0);



    $url_resolver = $this->prophesize(UrlResolverInterface::class);

    $resource_fetcher = $this->prophesize(ResourceFetcherInterface::class);



    $resource_fetcher->fetchResource(Argument::cetera())->willThrow(new ResourceException(

      'Error while fetching resource.',

      'https://youtube.com/foo',

    ));



    $this->container->set('media.oembed.url_resolver', $url_resolver->reveal());

    $this->container->set('media.oembed.resource_fetcher', $resource_fetcher->reveal());



    $response = OEmbedIframeController::create($this->container)

      ->render(new Request([

        'url' => '',

        'hash' => $hash,

      ]));

    assert($response instanceof HtmlResponse);



    $this->assertEquals(0, $response->getCacheableMetadata()->getCacheMaxAge());

  }



  /**

   * Tests that the response max age is 5 years when the max age returned by the provider is greater than 5 years.

   *

   * @return void

   *   No return value.

   */

  public function testResponseCacheMaxAgeGreaterThanFiveYears(): void {

    $hash = $this->container->get('media.oembed.iframe_url_helper')

      ->getHash('', 0, 0);



    $url_resolver = $this->prophesize(UrlResolverInterface::class);

    $resource_fetcher = $this->prophesize(ResourceFetcherInterface::class);



    $provider = new Provider('YouTube', 'https://youtube.com', [

      [

        'url' => 'https://youtube.com/foo',

      ],

    ]);

    $resource = Resource::rich(

      '<iframe src="https://youtube.com/watch?feature=oembed"></iframe>',

      320,

      240,

      $provider,

      // Five years and one second.

      cache_age: 157680001

    );



    $resource_fetcher->fetchResource(Argument::cetera())->willReturn($resource);



    $this->container->set('media.oembed.url_resolver', $url_resolver->reveal());

    $this->container->set('media.oembed.resource_fetcher', $resource_fetcher->reveal());



    $response = OEmbedIframeController::create($this->container)

      ->render(new Request([

        'url' => '',

        'hash' => $hash,

      ]));

    assert($response instanceof HtmlResponse);



    // The max age should be 5 years.

    $this->assertEquals(157680000, $response->getCacheableMetadata()->getCacheMaxAge());

  }



}