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

100 Continue on PATCH requests #152

Open
hamishforbes opened this issue Apr 16, 2020 · 5 comments
Open

100 Continue on PATCH requests #152

hamishforbes opened this issue Apr 16, 2020 · 5 comments

Comments

@hamishforbes
Copy link

Is there a reason Expect: 100-continue is no longer mentioned as part of the PATCH request part of the upload flow? Only as part of the creation with upload extension

It looks like it used to be in there after #9 but is now missing.

The tusd reference server also sends HTTP/1.1 100 Continue responses to invalid PATCH requests that do include the Expect header.

It seems like supporting this would be a useful feature allowing errors to be nicely returned to clients without having to drop the connection or cause issues with intermediary proxies etc

@Acconut
Copy link
Member

Acconut commented Apr 17, 2020

Good catch. The reason for removing that paragraph was (if I recall correctly) that the wording was a bit off:

If the clients sends an Expect request-header field with the 100-continue
expectation the server SHOULD return the 100 Continue status code before
reading the request's body and sending the final response.

You can understand the sentence that the server should always send a 100 Continue, even if an error response is going to be sent. Which is obviously wrong behavior and achieves the opposite of what 100-continue is intended for.

Would you mind opening a PR to add a better paragraph back in the specification?

@hamishforbes
Copy link
Author

Sure, can do.

I also have a fix for the tusd server to return error responses instead of 100 Continue and then an error response.

Although this is less useful than I had originally hoped, it turns out nginx is hardcoded to always respond with 100 Continue and that cannot be disabled, so any tusd instance running behind an nginx proxy isn't going to be able to make use of 100-continue anyway

@Acconut
Copy link
Member

Acconut commented Apr 20, 2020

I also have a fix for the tusd server to return error responses instead of 100 Continue and then an error response.

Awesome. Feel free to open a PR for it and we can discuss it!

Although this is less useful than I had originally hoped, it turns out nginx is hardcoded to always respond with 100 Continue and that cannot be disabled, so any tusd instance running behind an nginx proxy isn't going to be able to make use of 100-continue anyway

That's interesting and sad at the same time. Do you have a link or resource to get more information about this limitation?

@hamishforbes
Copy link
Author

@Acconut
Copy link
Member

Acconut commented Apr 23, 2020

Thank you very much for the links and the PR!

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

No branches or pull requests

2 participants