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

Max Request Size (Chunk size) #353

Open
navossoc opened this issue Feb 24, 2020 · 3 comments
Open

Max Request Size (Chunk size) #353

navossoc opened this issue Feb 24, 2020 · 3 comments

Comments

@navossoc
Copy link
Contributor

Problem:
As described on: https://github.com/tus/tusd/blob/master/docs/faq.md

Adjust maximum request size. Some proxies have default values for how big a request may be in order to protect your services. Be sure to check these settings to match the requirements of your application.

Sometimes we can't change these values and we need to deal with these limitations ourselves.

Propose:
It would be great if we can chunk requests smaller than the upload thresholds.

Describe alternatives you've considered
Sure, we can implement this on the client like (uppy), but shouldn't the server enforce this value?
I mean we have -max-size, why not something like -max-chunk?

This will allow the server to negotiate with the client a reasonable value.
Otherwise we will receive a 413 Request Entity Too Large (Cloudflare for example) and the upload will just fail.

Can you provide help with implementing this feature?
I think we have all it's needed to easy implement it.
It is only necessary to discuss the idea to see if there is any problem with it.

Let me know what you guys think about it.

@Acconut
Copy link
Member

Acconut commented Feb 24, 2020

Interesting idea but I don't fully understand what you are proposing.

why not something like -max-chunk?

What you that flag actually do? How should a server "enforce this value"?

This will allow the server to negotiate with the client a reasonable value.

For smaller uploads, it's a benefit that no negotiation is necessary as that would decrease performance. For bigger uploads, that's neglectable but we have to keep all scenarios in mind.

@navossoc
Copy link
Contributor Author

navossoc commented Feb 24, 2020

@Acconut

What you that flag actually do?

A header to describe the maximum allowed chunk size, something like this:

if r.Method == "OPTIONS" {
if handler.config.MaxSize > 0 {
header.Set("Tus-Max-Size", strconv.FormatInt(handler.config.MaxSize, 10))
}

How should a server "enforce this value"?

Like it does for the -max-size (maximum file size), the server rejects the request:

if handler.config.MaxSize > 0 && size > handler.config.MaxSize {
handler.sendError(w, r, ErrMaxSizeExceeded)
return
}

For smaller uploads, it's a benefit that no negotiation is necessary as that would decrease performance. For bigger uploads, that's neglectable but we have to keep all scenarios in mind.

I know, the best scenario will always be a single request with the whole file, but how can you do that if you are using a CDN that has strict limits? like Cloudflare that limits upload size (HTTP POST request size) with a 100mb hard limit.

When I say negotiate, I mean before the upload starts the client (uppy for example) just query the server about their limitations (max file size, max chunk size, etc). (Option #1)

(or)
Option #2: Try to upload the file in one single request, if it fails, then try to query the server if it has limits configured and then retry the upload.

For example: You could set on your tus server the max chunk size as 99mb limit and now you can upload files larger than 100mb to your endpoint proxied by cloudflare, because it will be chunked on the client before hit the CDN proxy.

Otherwise, what happens now without this proposal is:
You get a 413 http response and can't upload anything bigger than 100mb.

I hope that my intention is clearer now.

@navossoc navossoc changed the title Max Requet Size (Chunks) Max Request Size (Chunk size) Feb 24, 2020
@Acconut
Copy link
Member

Acconut commented Mar 8, 2020

Yes, thanks for the detailed response, @navossoc!

We had proposal for the tus protocol to add Tus-Min/Max-Chunk-Size headers over at tus/tus-resumable-upload-protocol#93 but never followed through with it since it was not apparent for us whether this is actually needed. Maybe you can comment over there.

Regarding the adding the limit to tusd and the query functionality to tus-js-client etc, we are always open for PRs. Let me know if you need any help there.

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

2 participants