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

Proposal: truncate files before uploading from liveview #1181

Closed
wants to merge 1 commit into from

Conversation

valyagolev
Copy link
Contributor

In liveview, even when onchange is not set for an input type="file", every chosen file is serialised and sent as one big WS message. In case of choosing a large file, even accidentally, this leads to JS errors (for hundreds-of-megabytes-big files), WS disconnections (for tens of megabytes), or UI freezes (for couples of megabytes).

Eventually and ideally, file uploads would be streamed piece by piece. However, probably even in that case there should be a modicum of control over those uploads, e.g. a limitation on the size of the uploaded file, otherwise one can see a potential for DDoS.

This PR is very modest, but would go a long way toward helping me to overcome this limitation until a better solution is devised. I think the approach I take here would be quite useful even when the streaming uploads are done.

I'm really not sure about the exact API and naming, and whether it makes sense to limit this attribute to liveview-only.

The PR adds an attribute called _liveview_truncate_at, with a default of 1 mb. The files chosen in the file-input are truncated to this size prior to sending them along the WS. I'd also propose to send and expose the original file sizes (the client side of this is also implemented in the PR), to make sure that the truncated files aren't accidentally taken to be complete. However, I wanted to discuss this before I start reworking major APIs across the different engines.

(A simple way to expose the size, as commented out in the PR, is through exposing the sizes as a method. A type-safer method could perhaps return a file as an enum: Complete(Contents), Truncated(usize, Contents), Streamed(impl Stream, Contents)...)

@ealmloff ealmloff added the liveview Related to the liveview renderer label Sep 5, 2023
@ealmloff
Copy link
Member

ealmloff commented Jan 3, 2024

DDoS needs to be handled on the server. You can't trust a client check to prevent uploading a large file. Adding support for streaming file uploads to dioxus-liveview would be great in the future, but I don't think exposing an attribute that limits the size of uploads is a good solution currently:

  • It makes it difficult to give feedback to the user if the upload is too big
  • It should be depreciated once streaming uploads are supported
  • Running into the default 1mb limit would be very confusing because there is no logging

@ealmloff ealmloff closed this Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
liveview Related to the liveview renderer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants