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

[LiveComponent] Add support for downloading files from LiveActions (Experimental) #2483

Open
wants to merge 10 commits into
base: 2.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/LiveComponent/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
## 2.23.0

- Allow configuring the secret used to compute fingerprints and checksums.
- [EXPERIMENTAL] Add `LiveDownloadResponse` and enable file downloads from
a `LiveAction`.

## 2.22.0

Expand Down
1 change: 1 addition & 0 deletions src/LiveComponent/assets/dist/Backend/BackendResponse.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ export default class {
private body;
constructor(response: Response);
getBody(): Promise<string>;
getBlob(): Promise<Blob>;
}
37 changes: 36 additions & 1 deletion src/LiveComponent/assets/dist/live_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ class BackendResponse {
}
return this.body;
}
async getBlob() {
return await this.response.blob();
}
}

function getElementAsTagText(element) {
Expand Down Expand Up @@ -2119,11 +2122,43 @@ class Component {
this.isRequestPending = false;
this.backendRequest.promise.then(async (response) => {
const backendResponse = new BackendResponse(response);
const html = await backendResponse.getBody();
for (const input of Object.values(this.pendingFiles)) {
input.value = '';
}
const headers = backendResponse.response.headers;
if (headers.get('X-Live-Download')) {
if (!(headers.get('Content-Disposition')?.includes('attachment') ||
headers.get('Content-Disposition')?.includes('inline')) ||
!headers.get('Content-Disposition')?.includes('filename=')) {
throw new Error('Invalid LiveDownload response');
}
const fileSize = Number.parseInt(headers.get('Content-Length') || '0');
if (fileSize > 10000000) {
throw new Error('File is too large to download (10MB limit)');
}
const fileName = headers.get('Content-Disposition')?.split('filename=')[1];
if (!fileName) {
throw new Error('No filename found in Content-Disposition header');
}
const blob = await backendResponse.getBlob();
const link = Object.assign(window.document.createElement('a'), {
target: '_blank',
style: 'display: none',
href: window.URL.createObjectURL(blob),
download: fileName,
});
this.element.appendChild(link);
link.click();
this.element.removeChild(link);
this.backendRequest = null;
thisPromiseResolve(backendResponse);
if (this.isRequestPending) {
this.isRequestPending = false;
this.performRequest();
}
return response;
}
const html = await backendResponse.getBody();
if (!headers.get('Content-Type')?.includes('application/vnd.live-component+html') &&
!headers.get('X-Live-Redirect')) {
const controls = { displayError: true };
Expand Down
4 changes: 4 additions & 0 deletions src/LiveComponent/assets/src/Backend/BackendResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,8 @@ export default class {

return this.body;
}

async getBlob(): Promise<Blob> {
return this.response.blob();
}
}
48 changes: 46 additions & 2 deletions src/LiveComponent/assets/src/Component/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -300,15 +300,59 @@ export default class Component {

this.backendRequest.promise.then(async (response) => {
const backendResponse = new BackendResponse(response);
const html = await backendResponse.getBody();

// clear sent files inputs
for (const input of Object.values(this.pendingFiles)) {
input.value = '';
}

// if the response does not contain a component, render as an error
const headers = backendResponse.response.headers;
if (headers.get('X-Live-Download')) {
const headerContentDisposition = headers.get('Content-Disposition');
if (
!headerContentDisposition
|| !(headerContentDisposition?.includes('attachment') || headerContentDisposition?.includes('inline'))
|| !headerContentDisposition?.includes('filename=')
) {
throw new Error('Invalid LiveDownload response');
}

const fileSize = Number.parseInt(headers.get('Content-Length') || '0');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to assume that if Content-Length header is absent, then it is equal to 0?
In which case the header can be absent? (I didn't see the rest of the PR yet!)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which case the header can be absent?

It should not be. This method is not made to stream movies, but files generated using the liveprops values.

Elsewhen, developers should send an URL to the JS and then open some stream, or redirect to another page, etc...

that if Content-Length header is absent, then it is equal to 0

No, absolutely not. But if it is unknown, we need to compute it on the fly then. Not implemented yet, but linked to the other questions below :)

if (fileSize > 10000000) {
throw new Error('File is too large to download (10MB limit)');
}
Comment on lines +321 to +323
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More correct and easier to understand:

Suggested change
if (fileSize > 10000000) {
throw new Error('File is too large to download (10MB limit)');
}
if (fileSize > 10 * 1024 * 1024) {
throw new Error('File is too large to download (10MB limit)');
}

Also, why do you want to limit the file size? It can be a nice feature, but shouldn't it be configurable by the developer?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the part we need to discuss. Don't forget the user has NOT accepted this download in any way for now.

Also, you cannot store 1GB in the DOM before saving the file.

We have options after:

  • streaming toward the FileStorage api (where user would then decide where and how to save the file)
  • allowing this only on button with attribute "download" (my preference for now)


const fileName = headerContentDisposition.split('filename=')[1];
if (!fileName) {
throw new Error('No filename found in Content-Disposition header');
}

const blob = await backendResponse.getBlob();
const link = Object.assign(window.document.createElement('a'), {
target: '_blank',
style: 'display: none',
href: window.URL.createObjectURL(blob),
download: fileName,
});
this.element.appendChild(link);
link.click();
this.element.removeChild(link);

this.backendRequest = null;
thisPromiseResolve(backendResponse);

// do we already have another request pending?
if (this.isRequestPending) {
this.isRequestPending = false;
this.performRequest();
}

return response;
}

const html = await backendResponse.getBody();

// if the response does not contain a component, render as an error
if (
!headers.get('Content-Type')?.includes('application/vnd.live-component+html') &&
!headers.get('X-Live-Redirect')
Expand Down
10 changes: 10 additions & 0 deletions src/LiveComponent/src/EventListener/LiveComponentSubscriber.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Psr\Container\ContainerInterface;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpFoundation\BinaryFileResponse;
use Symfony\Component\HttpFoundation\Exception\JsonException;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\HttpFoundation\Response;
Expand Down Expand Up @@ -43,7 +44,9 @@
class LiveComponentSubscriber implements EventSubscriberInterface, ServiceSubscriberInterface
{
private const HTML_CONTENT_TYPE = 'application/vnd.live-component+html';

private const REDIRECT_HEADER = 'X-Live-Redirect';
private const DOWNLOAD_HEADER = 'X-Live-Download';

public function __construct(
private ContainerInterface $container,
Expand Down Expand Up @@ -255,6 +258,13 @@ public function onKernelView(ViewEvent $event): void
return;
}

if ($event->getControllerResult() instanceof BinaryFileResponse) {
if (!$event->getControllerResult()->headers->has(self::DOWNLOAD_HEADER)) {

}
Comment on lines +262 to +264
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code

$event->setResponse(new Response());
}

$event->setResponse($this->createResponse($request->attributes->get('_mounted_component')));
}

Expand Down
38 changes: 38 additions & 0 deletions src/LiveComponent/src/LiveDownloadResponse.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<?php

namespace Symfony\UX\LiveComponent;

use SplFileInfo;
use SplTempFileObject;
use Symfony\Component\HttpFoundation\BinaryFileResponse;
use Symfony\Component\HttpFoundation\HeaderUtils;

/**
* @author Simon André <[email protected]>
*/
final class LiveDownloadResponse extends BinaryFileResponse
{
public const HEADER_LIVE_DOWNLOAD = 'X-Live-Download';

public function __construct(string|SplFileInfo $file, ?string $filename = null)
Kocal marked this conversation as resolved.
Show resolved Hide resolved
{
if (\is_string($file)) {
$file = new SplFileInfo($file);
}

if ((!$file instanceof SplFileInfo)) {
throw new \InvalidArgumentException(sprintf('The file "%s" does not exist.', $file));
}
Comment on lines +23 to +25
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure to understand when this code can be executed, $file can either be a string or SplFileInfo, and when it's a string, we re-assign $file to a SplFileInfo.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the condition is wrong?


if ($file instanceof SplTempFileObject) {
$file->rewind();
}

parent::__construct($file, 200, [
self::HEADER_LIVE_DOWNLOAD => 1,
'Content-Disposition' => HeaderUtils::makeDisposition(HeaderUtils::DISPOSITION_ATTACHMENT, $filename ?? basename($file)),
'Content-Type' => 'application/octet-stream',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we use the $file's mime type?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hesitated (keep dep light) and chose not to...

But in fact it's a good idea and Mime is a very small component.

So let's make it a requirement for LiveComponent ? Or only when using downloads / uploads ?

wdyt ?

Copy link
Member

@Kocal Kocal Jan 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest to put it in suggest composer.json section (I don't remember what is the rule in Symfony, cc @nicolas-grekas), and then runtime check if symfony/mime exists (when using downloads/uploads)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure there is no suggest in Symfony composer packages..

We have other situations in UX where we do not require a package (i.e. in Autocomplete for Form ..)

My question was more: should we require it for everyone, or keep it as an "runtime" dependency ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

runtime dependency

'Content-Length' => $file instanceof SplTempFileObject ? 0 : $file->getSize(),
], false, HeaderUtils::DISPOSITION_ATTACHMENT);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<?php

/*
* This file is part of the Symfony package.
*
* (c) Fabien Potencier <[email protected]>
*
* For the full copyright and license information, please view the LICENSE
* file that was distributed with this source code.
*/

namespace Symfony\UX\LiveComponent\Tests\Fixtures\Component;

use Symfony\Component\HttpFoundation\BinaryFileResponse;
use Symfony\UX\LiveComponent\Attribute\AsLiveComponent;
use Symfony\UX\LiveComponent\Attribute\LiveAction;
use Symfony\UX\LiveComponent\Attribute\LiveArg;
use Symfony\UX\LiveComponent\DefaultActionTrait;
use Symfony\UX\LiveComponent\LiveDownloadResponse;

/**
* @author Simon André <[email protected]>
*/
#[AsLiveComponent('download_file', template: 'components/download_file.html.twig')]
class DownloadFileComponent
{
use DefaultActionTrait;

private const FILE_DIRECTORY = __DIR__.'/../files/';

#[LiveAction]
public function download(): BinaryFileResponse
{
$file = new \SplFileInfo(self::FILE_DIRECTORY.'/foo.json');

return new LiveDownloadResponse($file);
}

#[LiveAction]
public function generate(): BinaryFileResponse
{
$file = new \SplTempFileObject();
$file->fwrite(file_get_contents(self::FILE_DIRECTORY.'/foo.json'));

return new LiveDownloadResponse($file, 'foo.json');
}

#[LiveAction]
public function heavyFile(#[LiveArg] int $size): BinaryFileResponse
{
$file = new \SplFileInfo(self::FILE_DIRECTORY.'heavy.txt');

$response = new BinaryFileResponse($file);
$response->headers->set('Content-Length', 10000000); // 10MB
}
}
9 changes: 9 additions & 0 deletions src/LiveComponent/tests/Fixtures/files/foo.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<html lang="en">
<head>
<title>Foo</title>
</head>
<body>
<h1>Bar</h1>
</body>
</html>
3 changes: 3 additions & 0 deletions src/LiveComponent/tests/Fixtures/files/foo.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"foo": "bar"
}
3 changes: 3 additions & 0 deletions src/LiveComponent/tests/Fixtures/files/foo.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Foo

## Bar
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<div {{ attributes }}>

</div>
Loading
Loading