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

Resumable Upload: Indicate length during creation #2916

Closed
wants to merge 7 commits into from

Conversation

Acconut
Copy link
Member

@Acconut Acconut commented Oct 15, 2024

Closes #2832.

During the IETF 120 session, we saw support for a header to indicate the length during upload creation. A server can deduce the length from Content-Length, but this is not possible of chunked transfer coding is used or the upload is split across multiple requests. This PR adds the advisory Upload-Length header to fill this gap.

@guoye-zhang
Copy link
Contributor

guoye-zhang commented Oct 15, 2024

  • Update the examples
  • Mention that the server should reject the request in situations where Content-Length and Upload-Length mismatch (Content-Length > Upload-Length, or Upload-Offset + Content-Length != Upload-Length if complete is true)

@Acconut
Copy link
Member Author

Acconut commented Oct 15, 2024

  • Update the examples

Done!

  • Mention that the server should reject the request in situations where Content-Length and Upload-Length mismatch (Content-Length > Upload-Length, or Upload-Offset + Content-Length != Upload-Length if complete is true)

For upload creation, this PR requires Content-Length and Upload-Length to be consistent:

The request can indicate the upload's final size in two different ways. Both indicators may be present in the same request as long as they convey the same size. If the sizes are inconsistent, the server MUST reject the request by responding with a 400 (Bad Request) status code.

For upload appending, we already have this text:

If the request includes the Upload-Complete field value set to true and a valid Content-Length header field, the client attempts to upload the remaining resource in one request. In this case, the upload's final size is the sum of the upload's offset and the Content-Length header field. If the server does not have a record of the upload's final size from creation or the previous append, the server MUST record the upload's final size to ensure its consistency. If the server does have a previous record, that value MUST match the upload's final size. If they do not match, the server MUST reject the request with a 400 (Bad Request) status code.

I think this covers all cases where Content-Length and Upload-Length mismatch. Did we miss anything else so far?

@guoye-zhang
Copy link
Contributor

Ah yeah, we already mentioned final size. That should do it.

draft-ietf-httpbis-resumable-upload.md Outdated Show resolved Hide resolved
@@ -447,6 +454,10 @@ The `Upload-Complete` request and response header field indicates whether the co

The `Upload-Complete` header field MUST only be used if support by the resource is known to the client ({{feature-detection}}).

## Upload-Length

The `Upload-Length` request and response header field indicates the number of bytes to be uploaded for the corresponding upload resource, counted in bytes. The `Upload-Length` field value is an Integer.

Choose a reason for hiding this comment

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

In other parts of the document we have written down how to calculate the upload offset. Do we need something similar for Upload-Length and how it is calculated? E.g. without transfer codings.

Do we need to add something on what will happen if the client sends more data than the Upload-Length?

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 other parts of the document we have written down how to calculate the upload offset. Do we need something similar for Upload-Length and how it is calculated? E.g. without transfer codings.

Done in ffbbb01.

Do we need to add something on what will happen if the client sends more data than the Upload-Length?

As mentioned in #2916 (comment), the document includes text that requires the server to stop the upload if the client exceeds its own upload length. Does that cover what you had in mind?

Choose a reason for hiding this comment

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

As mentioned in #2916 (comment), the document includes text that requires the server to stop the upload if the client exceeds its own upload length. Does that cover what you had in mind?

It's not the exact case but maybe close enough? The case I mean is the client creates a file with Upload-Length: 100 and then tries to upload 200 bytes. For Content-Length this is already handled by existing standards but given that Upload-Length is a new header it's not explicit what should happen. The quoted part in #2916 (comment) is more about how to handle mismatches between the header values. Maybe we don't have to be explicit about such things?

Copy link
Member Author

@Acconut Acconut Oct 21, 2024

Choose a reason for hiding this comment

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

Added d83a56f to clarify this. Let me know if this doesn't cover what you had in mind.

Choose a reason for hiding this comment

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

Perfect, thank you.

@Acconut
Copy link
Member Author

Acconut commented Oct 16, 2024

Thank you for the feedback, @smatsson and @guoye-zhang!

@Acconut Acconut closed this Oct 21, 2024
@Acconut Acconut deleted the resumable-upload/length-upfront branch October 21, 2024 15:25
@Acconut
Copy link
Member Author

Acconut commented Oct 21, 2024

This work has been merged in #2927.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Resumable Upload: Indicating upload length upfront
3 participants