Download files with non-ascii characters v2

[ejoseca]

We can't set the ContentDisposition 'attachment' in the constructor.
If the filename contains non-ascii characters, the constructor will throw an exception.
In this revision, if you don't specify a name, it is taken from the filename (this is what Symphony does if you don't provide a name, but here we can convert the filename to an ascii string so it won't throw any exception).

[taylorotwell]

I need more information with specific examples of what is broken and why this fixes it. This seems to remove the ASCII conversion?

[ejoseca]

If you try to download a file with a name whose name contains non-ascii characters (for example, "Lucky☆Star" or "être") and you don't provide an ascii filenameFallback, Symphony throws an expection.

Step by step:

//Original Response::download code
public static function download($file, $name = null, array $headers = array(), $disposition = 'attachment')
{
    $response = new BinaryFileResponse($file, 200, $headers, true, $disposition);
    if ( ! is_null($name))
    {
        return $response->setContentDisposition($disposition, $name, Str::ascii($name));
    }
    return $response;
}

So, when we provide a name for the file to download, we call to the function setContentDisposition:

//path: vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/BinaryFileResponse.php
public function setContentDisposition($disposition, $filename = '', $filenameFallback = '')
{
    if ($filename === '') {
        $filename = $this->file->getFilename();
    }
    $dispositionHeader = $this->headers->makeDisposition($disposition, $filename, $filenameFallback);
    $this->headers->set('Content-Disposition', $dispositionHeader);
    return $this;
}

In this function, the method makeDisposition is called:

//path: vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/ResponseHeaderBag.php
public function makeDisposition($disposition, $filename, $filenameFallback = '')
{
...
    if ('' == $filenameFallback) {
        $filenameFallback = $filename;
    }
    // filenameFallback is not ASCII.
    if (!preg_match('/^[\x20-\x7e]*$/', $filenameFallback)) {
        throw new \InvalidArgumentException('The filename fallback must only contain ASCII characters.');
...
}

This function throw an error if the third argument filenameFallback contains non-ascii characters or if filename contains non-ascii characters and filenameFallback is not specified. That is why we use Str::ascii() in the Response::download function.

The problem is the initialization of the variable $response:

public static function download($file, $name = null, array $headers = array(), $disposition = 'attachment')
{
    $response = new BinaryFileResponse($file, 200, $headers, true, $disposition);
    if ( ! is_null($name))
    {
        return $response->setContentDisposition($disposition, $name, Str::ascii($name));
    }
    return $response;
}

It creates a new BinaryFileResponse object with the contentDisposition argument in the constructor. The constructor is:

//path: vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/BinaryFileResponse.php
public function __construct($file, $status = 200, $headers = array(), $public = true, $contentDisposition = null, $autoEtag = false, $autoLastModified = true)
{
    parent::__construct(null, $status, $headers);
    $this->setFile($file, $contentDisposition, $autoEtag, $autoLastModified);
    if ($public) {
        $this->setPublic();
    }
}

It calls the setFile method with the contentDisposition argument:

//path: vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/BinaryFileResponse.php
public function setFile($file, $contentDisposition = null, $autoEtag = false, $autoLastModified = true)
{
...
    if ($contentDisposition) {
        $this->setContentDisposition($contentDisposition);
    }
...
}

So here the setContentDisposition() method is called without the filename and the filenameFallback arguments:

//path: vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/BinaryFileResponse.php
public function setContentDisposition($disposition, $filename = '', $filenameFallback = '')
{
    if ($filename === '') {
        $filename = $this->file->getFilename();
    }
   $dispositionHeader = $this->headers->makeDisposition($disposition, $filename, $filenameFallback);
...
}

It has taken the real name of the file and called makeDisposition:

//path: vendor/symfony/http-foundation/Symfony/Component/HttpFoundation/ResponseHeaderBag.php
public function makeDisposition($disposition, $filename, $filenameFallback = '')
{
...
    if ('' == $filenameFallback) {
        $filenameFallback = $filename;
    }
    // filenameFallback is not ASCII.
    if (!preg_match('/^[\x20-\x7e]*$/', $filenameFallback)) {
        throw new \InvalidArgumentException('The filename fallback must only contain ASCII characters.');
...
}

So, if the name of the file has non-ascii characters, it will throw an expection. We can evit this problem if we don't specify the contentDisposition in the call to the BinaryFileResponse constructor and allways specify the 3 arguments of the setContentDisposition function:

//New Response::download code
public static function download($file, $name = null, array $headers = array(), $disposition = 'attachment')
{
    $response = new BinaryFileResponse($file, 200, $headers, true);
    if (is_null($name))
    {
        $name = basename($file); // symfony uses the file name if user doesn't provide one
    }
    return $response->setContentDisposition($disposition, $name, Str::ascii($name));
}

[Guuzen]

Started to learn php and laravel and got this error. Will be good if someone fix this bug, cause it's still here in laravel 5 and this is annoying.

[jee7]

Would also like that to be fixed. Nowadays it shouldn't be any problem if there are non-ASCII characters in the file name. Why is this even checked?

[gwilgen]

Thanks @ejoseca for such a complete response.

I changed mi code not to use Response::download(), because it doesn't work (with files with non ascii characters). I directly use your proposition for the download code directly in my Controllers.

Hope it wil be added to Laravel soon!

[ejoseca]

If it is useful for someone:

In Laravel 5, you can extend the \Illuminate\Routing\ResponseFactory class and override the download() method. Then, in a ServiceProvider, register your custom ResponseFactory as the default one:

// We extends \Illuminate\Routing\ResponseFactory class, that implements Illuminate\Contracts\Routing\ResponseFactory
$this->app->singleton('Illuminate\Contracts\Routing\ResponseFactory', function ($app) {
    return new MyCustomResponseFactoryExtendedFromResponseFactory($app['Illuminate\Contracts\View\Factory'], $app['redirect']);
});

Doing this, you can use the default methods (Response::json(), respose()->view(), etc) and your custom methods in the same way: Response::download(), _response()->download

[elbic]

👍

[jakjr]

+1

[localdisk]

👍

[mochetts]

We faced the same issue. So far we decided to implement our own Response facade and Response service provider extending and overriding Laravels ones to fix this issue.
Would be nice that @taylorotwell or @GrahamCampbell, or any one else in charge of Laravel maintenance, takes a look at this as so many people faced the same issue. And also because @ejoseca put some good effort fixing it and explaining what was going on. Thanks!

[Rattone]

Hi! Was this fixed in Laravel 6, 7, 8...?