-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
Disable body reading on head requests #95
Conversation
3156364
to
9794e6f
Compare
// The HEAD method is identical to GET except that the server | ||
// MUST NOT return a message-body in the response. | ||
// https://www.w3.org/Protocols/rfc2616/rfc2616-sec9.html section 9.4 | ||
if (verb.toLowerCase == "head") f(new ByteArrayInputStream(Array())) |
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 HTTP verbs are case-sensitive. Probably you need to check for HEAD
because head
or Head
are considered extension-methods and don't share the HEAD
semantics.
if (verb.toLowerCase == "head") f(new ByteArrayInputStream(Array())) | |
if (verb == "HEAD") f(new ByteArrayInputStream(Array())) |
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.
@lolgab updated as per your suggestion
Just out of curiosity, why you needed to introduce the |
@lihaoyi Oh, now it makes much more sense! |
@lolgab yeah waiting for your review on that one |
@@ -1,11 +1,11 @@ | |||
package requests | |||
|
|||
import requests.Requester.methodField |
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.
Is this a spurious change?
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.
yeah, will remove
Fixes #37
Review by @lolgab