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

Respond with 411 Length Required when missing #2233

Closed
wants to merge 1 commit into from

Conversation

gregersrygg
Copy link
Contributor

What does this PR do and why is it necessary?

Changes HTTP response code for file upload to 411 (Length Required) instead of 400 (Bad Request) when Content-Length is missing. It is very hard for the client to guess that length is required by the server when the response doesn't say anything about why it's a bad request, and it's perfectly valid to do a file upload without a Content-Length in the header. Using 411 response code tells the client why it's a bad request.

How was it tested? How can it be tested by the reviewer?

I don't know python, and the documentation is not good enough to explain how to setup and run the unit tests. The only thing this change will possibly break is an already broken client, or a unit test that is hard-coded to expect 400 (which is essentially the same as 411 - just less informative).

Any background context you want to provide?

https://tools.ietf.org/html/rfc7231#section-6.5.10

@foosel
Copy link
Member

foosel commented Nov 24, 2017

Very good point. I took the liberty to cherry-pick this to the maintenance branch since it's not a new feature but rather an improvement or even a bug fix and should hence go with the next stable release 1.3.6. Github doesn't properly detect cherry picks, so I'm closing this PR manually. Your commit is merged as b002e41.

the documentation is not good enough to explain how to setup and run the unit tests

I've also fixed that on the maintenance branch, already visible on that branch on docs.octoprint.org. The contribution guidelines have also been updated accordingly on maintenance and that will go live with the next release.

@foosel foosel closed this Nov 24, 2017
@gregersrygg gregersrygg deleted the patch-1 branch November 27, 2017 18:42
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants