Dynamic page Cache does not works with binary file responses

Created on 6 August 2021, almost 3 years ago
Updated 28 November 2023, 7 months ago

Problem/Motivation

We want to use BinaryFileResponse implement CacheableResponseInterface.

But site crash and throw error 'Symfony\Component\HttpFoundation\File\File' is not allowed in serialize().

check PHP stack, it's from DynamicPageCacheSubscriber::onResponse()

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Postponed: needs info

Version

9.5

Component
Dynamic page cacheΒ  β†’

Last updated 2 days ago

Created by

πŸ‡¨πŸ‡³China Oscaner

Live updates comments and jobs are added and updated live.
  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¬πŸ‡§United Kingdom catch

    What are you trying to cache BinaryFileResponse in that's not the dynamic page cache? BinaryFileResponse is a Symfony class and doesn't implement the interface, so would never enter the cache in the first place due to the CacheableResponse check in ::onRequest(). I'm not sure why we'd special case an interface that already doesn't implement CacheableResponseInterface by default.

    If this is for custom code, you can add a dynamic page cache response policy to exclude BinaryFileResponse - that would allow you to implement the interface but still exclude it from dynamic page cache. I could see someone trying to do similar but also trying to make things serializable too, in which case the hard-coding of the interface here would prevent that.

    Moving to needs more info until the use case is clearer.

  • πŸ‡©πŸ‡ͺGermany donquixote

    I just ran into this.

    The CacheableResponseInterface has two effects:
    1. The page can be cached in Drupal's dynamic page cache.
    2. The page gets cache tag headers and can be cached in Varnish.

    The Media Alias Display β†’ module replaces the media controller and sends a BinaryFileResponse when visiting a media canonical url.

    The module currently does not add any cache info to the response, making it uncacheable.
    Our own varnish config auto-caches everything for anonymous for 8h.
    This means that now these files are not purged on update, because Varnish does not have the cache tags.

    My solution attempt is to create this class:

    
    declare(strict_types=1);
    
    namespace Drupal\media_alias_display\Response;
    
    use Drupal\Core\Cache\CacheableResponseInterface;
    use Drupal\Core\Cache\CacheableResponseTrait;
    use Symfony\Component\HttpFoundation\BinaryFileResponse;
    
    /**
     * A file response that can be cached.
     *
     * See https://www.drupal.org/project/drupal/issues/3227041.
     */
    class CacheableBinaryFileResponse extends BinaryFileResponse implements CacheableResponseInterface {
    
      use CacheableResponseTrait;
    
      /**
       * Constructor.
       *
       * @param string $uri
       *   File uri.
       * @param int $status
       *   The http response code, e.g. 200 for "OK".
       * @param array $headers
       *   An array of response headers.
       * @param bool $public
       *   TRUE to set 'Cache-Control' header to 'public'.
       *   FALSE to set 'Cache-Control' header to 'private'.
       * @param string|null $contentDisposition
       *   A content-disposition header.
       * @param bool $autoEtag
       *   TRUE to set an 'ETag' header based on the file checksum.
       * @param bool $autoLastModified
       *   TRUE to set a 'Last-Modified' header based on the file mtime.
       */
      public function __construct(
        protected string $uri,
        int $status = 200,
        array $headers = [],
        bool $public = TRUE,
        string $contentDisposition = NULL,
        bool $autoEtag = FALSE,
        bool $autoLastModified = TRUE,
      ) {
        parent::__construct($uri, $status, $headers, $public, $contentDisposition, $autoEtag, $autoLastModified);
      }
    
      /**
       * Gets a serializable representation.
       *
       * @return array
       *   Data for serialization.
       */
      public function __serialize(): array {
        $values = (array) $this;
        // The file object cannot be serialized.
        unset($values["\0*\0file"]);
        return $values;
      }
    
      /**
       * Initializes uncacheable properties on unserialize.
       */
      public function __wakeup(): void {
        $this->setFile($this->uri);
      }
    
    }
    
    

    Now I only need to figure out which cache tags and cache contexts to add...

  • πŸ‡©πŸ‡ͺGermany donquixote

    Link to πŸ“Œ Make the response cacheable Active for Media Alias Display.

Production build 0.69.0 2024