Skip to content

Response::withFile() and Response::withFileDownload()#91

Merged
l0gicgate merged 16 commits into
slimphp:masterfrom
l0gicgate:RespondWithFile
May 24, 2019
Merged

Response::withFile() and Response::withFileDownload()#91
l0gicgate merged 16 commits into
slimphp:masterfrom
l0gicgate:RespondWithFile

Conversation

@l0gicgate
Copy link
Copy Markdown
Member

Resuming to adopt the proposed direction in #88

New Response Methods:

  • Response::withFile($file, $contentType)
  • Response::withFileDownload($file, $name, $contentType)

Behavior
The $file parameter can be a string which points to a file, an existing resource handle or a StreamInterface as proposed by @roxblnfk

The $name parameter overrides the default attachment value for the Content-Disposition header. The given $name is filtered through urlencode() to ensure it is a valid header value.

The $contentType parameter accepts a string to override the Content-Type header to a user defined value. Defaults to true which attempts to detect the mime type via mime_content_type() and falls back to application/octet-stream otherwise. If set to false the Content-Type header is not appended at all.

Closes #82

@l0gicgate l0gicgate added this to the 0.8 milestone May 22, 2019
@coveralls
Copy link
Copy Markdown

coveralls commented May 22, 2019

Coverage Status

Coverage increased (+0.07%) to 99.349% when pulling b187cc0 on l0gicgate:RespondWithFile into 36c06df on slimphp:master.

@adriansuter
Copy link
Copy Markdown
Contributor

In Firefox 67.0 (64-Bit) the file name in the "Save as" dialog would be wrong if we use urlencode() for a file name like sale$.json. The presented file name is sale%24.json. In Chrome 74.0.3729.169 (64-Bit), Opera 60.0.3255.95 and MS Edge 42.17134.1.0 the url encode would be decoded. Not sure what the rfc says about that.

If instead we write

            $disposition .= '; filename*=UTF-8\'\'' . urlencode($fileName);

then all the browsers mentioned above display the correct file name. See http://test.greenbytes.de/tech/tc2231/#encoding-2231-char for more information.

Note that this works only, if the $fileName really is in UTF-8.

@adriansuter
Copy link
Copy Markdown
Contributor

If we pass a StreamInterface as $file but no $name to \Slim\Http\Response::withFileDownload, then Firefox presents a "Save as" dialog with no file name at all. If then saved, the file would be saved with a random file name like uM3wqZYH.

public function __invoke(ServerRequest $request, Response $response, array $args = []): Response
    $stream = $psr17Factory->createStreamFromFile('data.json');
    return $response->withFileDownload($stream);
}

Chrome and Opera defaults to the filename Download, Edge could not save the file at all - it tried to save a file named localhost/.

Probably we should raise an exception if a user tries to do that? Or is that the responsibility of a user?

Comment thread src/Response.php
Comment thread src/Response.php
Comment thread src/Response.php Outdated
@roxblnfk
Copy link
Copy Markdown

Probably we should raise an exception if a user tries to do that? Or is that the responsibility of a user?

The filename is always optional

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition

@adriansuter
Copy link
Copy Markdown
Contributor

The filename is always optional
https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition

Of course, if the url contains a path from which the user agent can parse the filename, then this is okay. But obviously, if the download would be triggered by calling https://domain.com/ directly ("no path part"), then the user agents would have no information regarding the file name (Edge fails in this case completely).

By the way, the same holds for any paths ending with a slash (of course that should be avoided). So if we define a route

$app->get('/hello/', function (\Slim\Http\ServerRequest $request, \Slim\Http\Response $response, array $args) {
    $psr17Factory = new Psr17Factory();
    $stream = $psr17Factory->createStream('1234');

    return $response->withFileDownload($stream);
});

then a request to http://localhost/hello/ would start a file download with an empty file name (in Firefox). At least, in Edge the file name would be hello. In Opera and Chrome, the file name would be Download.

So I guess this is a problem that the user agents should solve for themselves. It is not our duty to prevent that, right?

@roxblnfk
Copy link
Copy Markdown

So I guess this is a problem that the user agents should solve for themselves. It is not our duty to prevent that, right?

I think this is not our problem. But we can also set default filename to "Download" (don't throw error)

@l0gicgate
Copy link
Copy Markdown
Member Author

l0gicgate commented May 22, 2019

Probably we should raise an exception if a user tries to do that? Or is that the responsibility of a user?

Let's not throw an error but append a default filename as suggested like "download" or "attachment" when a StreamInterface is passed but no name is specified.

@adriansuter
Copy link
Copy Markdown
Contributor

Let's not throw an error but append a default filename as suggested like "download" or "attachment" when a StreamInterface is passed but no name is specified.

Would it make sense to try to get the file name from the metadata uri of the stream?

@l0gicgate
Copy link
Copy Markdown
Member Author

l0gicgate commented May 22, 2019

Would it make sense to try to get the file name from the metadata uri of the stream?

Yea for sure why not.

@l0gicgate l0gicgate closed this May 24, 2019
@l0gicgate l0gicgate deleted the RespondWithFile branch May 24, 2019 00:07
@l0gicgate l0gicgate restored the RespondWithFile branch May 24, 2019 00:07
@l0gicgate l0gicgate reopened this May 24, 2019
@l0gicgate l0gicgate merged commit 1c96f39 into slimphp:master May 24, 2019
@l0gicgate l0gicgate changed the title [WIP] Response::withFile() and Response::withFileDownload() Response::withFile() and Response::withFileDownload() May 24, 2019
@l0gicgate l0gicgate deleted the RespondWithFile branch May 24, 2019 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add helper to simplify streaming a file download

4 participants