Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFC] Adding a controller that proxy to the image #142

Open
Nek- opened this issue Jun 3, 2023 · 3 comments
Open

[RFC] Adding a controller that proxy to the image #142

Nek- opened this issue Jun 3, 2023 · 3 comments

Comments

@Nek-
Copy link

Nek- commented Jun 3, 2023

I think making a controller that will proxy the original image is a very common case that can be handle by the bundle (not by default though). What do you think? (I can make that PR)

@mdeboer
Copy link

mdeboer commented Jun 23, 2023

I don't think you should make it into a controller but a response class.

Depending on the needs you can do it in two ways at the moment:

A) Using a StreamedResponse by using the stream from flysystem directly
B) Using a BinaryFileResponse by first downloading the file locally (e.g. to /tmp).

A is very easy to implement and you don't need to worry about temporary files but it lacks Range, If-Range and X-Sendfile handling.

B is a bit harder to implement as you need to manage the temporary files yourself. You don't want to download the file, send it and then delete the temporary file straight away as that negates the benefits of for example Range, where you send only a smaller range of bytes. You want to keep the temporary file in between requests to be able to read the file straight away and serve only the requested range of bytes ... but also not keep them too long as your disk will fill up.

Unless you're handling really big files and Range handling is really a must, most people would be fine with a simple implementation of A.

As an example with extra additions to allow caching (ETag, last modified etc.):

<?php

declare(strict_types=1);

namespace App\Controller;

use League\Flysystem\ChecksumAlgoIsNotSupported;
use League\Flysystem\FilesystemOperator;
use League\Flysystem\UnableToProvideChecksum;
use League\Flysystem\UnableToRetrieveMetadata;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\StreamedResponse;
use Symfony\Component\Mime\MimeTypes;
use Symfony\Component\Routing\Annotation\Route;

class FileController extends AbstractController
{
    public function __construct(
        private readonly FilesystemOperator $fileStorage
    ) {
    }

    #[Route(
        path: '/download/{file}',
        name: 'download',
        methods: [Request::METHOD_GET]
    )]
    public function __invoke(
        string $file,
        Request $request
    ): Response {
        if ($this->fileStorage->fileExists($file) === false) {
            return new Response('', Response::HTTP_NOT_FOUND);
        }

        // Create response.
        $response = new StreamedResponse(
            function () use ($file) {
                // Open stream to image.
                $fp = $this->fileStorage->readStream($file);

                // Read in chunks of 16KB.
                $chunkSize = 16 * 1024;

                while (feof($fp) === false) {
                    $read = fread($fp, $chunkSize);

                    if ($read === false) {
                        break;
                    }

                    echo $read;
                    flush();
                }

                fclose($fp);
            }
        );

        // Allow caching.
        $response->setPublic();

        // Set content type.
        $this->setContentType($response, $file);

        // Set last modified.
        $this->setLastModified($response, $file);

        // Set ETag.
        $this->setETag($response, $file);

        // Check if response is modified (etag etc.).
        $response->isNotModified($request);

        // Send response.
        return $response;
    }

    public function setContentType(StreamedResponse $response, string $file): void
    {
        try {
            $mime = $this->imageStorage->mimeType($file);
        } catch (UnableToRetrieveMetadata) {
            $mime = MimeTypes::getDefault()->guessMimeType($file);
        }

        if (empty($mime)) {
            return;
        }

        $response->headers->set('Content-Type', $mime);
    }

    public function setLastModified(StreamedResponse $response, string $file): void
    {
        try {
            $lastModifiedTimestamp = $this->imageStorage->lastModified($file);

            $response->setLastModified(
                \DateTime::createFromFormat('U', $lastModifiedTimestamp)
            );
        } catch (UnableToRetrieveMetadata) {
            // Intentionally blank.
        }
    }

    public function setETag(StreamedResponse $response, string $file): void
    {
        try {
            $etag = $this->imageStorage->checksum($file);
        } catch (UnableToProvideChecksum|ChecksumAlgoIsNotSupported) {
            // Generate a weak ETag from the file path.
            $etag = 'W/' . hash('xxh128', $file);
        }

        $response->setEtag($etag);
    }
}

All these things like setting the content type, ETag and last modified date can all be included in the Response class like BinaryFileResponse does and you would end up with something like this:

<?php

declare(strict_types=1);

namespace App\Controller;

use League\Flysystem\FilesystemOperator;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\Routing\Annotation\Route;

class FileController extends AbstractController
{
    public function __construct(
        private readonly FilesystemOperator $fileStorage
    ) {
    }

    #[Route(
        path: '/download/{file}',
        name: 'download',
        methods: [Request::METHOD_GET]
    )]
    public function __invoke(
        string $file,
        Request $request
    ): Response {
        if ($this->fileStorage->fileExists($file) === false) {
            return new Response('', Response::HTTP_NOT_FOUND);
        }

        return new FlysystemResponse(
            filesystem: $this->fileStorage,
            path: $file,
            status: 200,
            headers: [],
            public: true,
            autoEtag: true,
            autoLastModified: true
        );
    }
}

@Nek-
Copy link
Author

Nek- commented Jun 26, 2023

Hello,

Thank you very much for your answer, it's an interesting approach.

But why not combine both BinaryFile and Streamed responses? We may suggest a by default stream with cache and everything (which I think is the best), and with a simple config using a cache file.

WDYT?

@maxhelias
Copy link
Collaborator

Here's a related article : https://dev.to/rubenrubiob/serve-a-file-stream-in-symfony-3ei3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants