-
Notifications
You must be signed in to change notification settings - Fork 165
resumable: clarify client behavior on 413 response #3312
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
base: main
Are you sure you want to change the base?
Conversation
| Servers, including intermediaries, can (and often do) apply restrictions on the size of individual request message content. There is no standard mechanism to communicate such existing size restriction. A server that implements one can respond with a 413 Content Too Large status code; see {{Section 15.5.14 of HTTP}}. Appending to an upload resource, as a series of appends, can be used to upload data up to the `max-size` limit without encountering per-message limits. The `min-append-size` and `max-append-size` limits apply to the upload resource. Servers might apply restrictions that are smaller than the append limits, which would also result in a failed request. Clients could deal with such situations by retrying an upload append using a smaller size, as long as the new size resides between `min-append-size` and `max-append-size`. Cases where an append uses `min-append-size` yet fails with a 413 Content Too Large response might indicate a deployment mismatch that cannot be recovered from. | ||
| Servers, including intermediaries, can (and often do) apply restrictions on the size of individual request message content. There is no standard mechanism to communicate such existing size restriction. A server that implements one can respond with a 413 Content Too Large status code; see {{Section 15.5.14 of HTTP}}. Appending to an upload resource, as a series of appends, can be used to upload data up to the `max-size` limit without encountering per-message limits. Servers might apply restrictions that are smaller than the append limits, which would also result in a failed request. | ||
|
|
||
| When a client receives a response with a 413 Content Too Large status code during upload creation ({{upload-creation}}) or append ({{upload-appending}}), it SHOULD retry the request with a smaller size residing between `min-append-size` and `max-append-size`. If these limits are unknown at the time of receiving such a response, the client SHOULD attempt to discover them through an `OPTIONS` request. Cases where a request uses `min-append-size` yet fails with a 413 Content Too Large response might indicate a deployment mismatch that cannot be recovered from. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably require 413 to carry an Upload-Limit header if it's from a resumable upload server. Upload creation can be attempted without knowledge of server support, and clients can't retry a non-resumable upload.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that a proxy might respond with 413 based on the request's Content-Length, so even if the server is resumable it can't add the header in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OPTIONS request to learn the limits also doubles as a way to discover resumability. Maybe it should be emphasized here more, so as to avoid retrying without knowing about resumability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As an implementor of a generic HTTP client, we can attempt resumable uploads for all uploads since it has little overhead and minimum risk, but we cannot perform an OPTIONS discovery step for all uploads that fail with 413. We need to know that the other side supports resumable uploads before doing something expensive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OPTIONS request wouldn't be a MUST - it's more that they MUST NOT retry if they don't know the server supports resumability. I suppose the only other way for a generic library to handle this proxy scenario would be to configure it to assume the server is resumable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SHOULD is a recommendation and means that we think it's a good idea. I don't think it's feasible for us since you know there are unupgradable printers that would crash when it receives an OPTIONS request. We have to be as safe as possible when discovering resumable upload support.
Yeah, this has to be an opt-in feature, although a more desirable opt-in behavior probably is to always send OPTIONS to detect Upload-Limit first instead of relying on 413.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. To summarize:
- If 413 contains Upload-Limit header, the client SHOULD try again within these limits
- If not, the client MAY try OPTIONS to learn limits and try again
- Client MUST NOT retry if it isn't aware of server resumable upload support, either from the above or from prior knowledge
Do those changes sound good?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, looks good. I believe the reason we didn't tackle upload creation originally was the complexity of its interaction with discovery. Upload appending can only be performed if the server support is confirmed so its behavior is easier to define.
Minor nit: we stopped using the phrase "prior knowledge" based on the feedback in #3215.
…pport, soften OPTIONS comment
|
Something to consider is whether the server can make the resource and respond with 413 at some point later, or if it needs to respond upfront without doing anything. In the former case, the 413 response would need to include the Location header and maybe the Upload-Offset. A similar question is for upload append, as to whether the server can accept some of the body before responding or if it must reject immediately. It would probably be simpler for the client to know these requests are no-ops, but there has been discussion on having the server accept up to its limits and ignore the rest, which implies that it can respond after processing part of the request. |
|
Would prefer the server sending 104 with Location prior to 413. The only thing the client needs to do in that case is to allow 413 retry. Based on the current spec, the server could send a 5xx after processing the request body to the limit, which would prompt the client to retry and resume. It's a hacky workaround though. |
|
The change looks OK to me. @Acconut what do you think? |
Co-authored-by: Marius Kleidl <[email protected]>
This PR aims to address #3193 (comment)
These changes should make clients more resilient to server limits without giving up right away.
This is my first PR here, so sorry in advance if I made a mistake!