http-parser icon indicating copy to clipboard operation
http-parser copied to clipboard

Need to consider the noBody situation for HEAD method

Open tianchao-haohan opened this issue 6 years ago • 3 comments

Here is the diff code:

+++ b/src/3rd_party/http_parser/http_parser.c
@@ -1783,8 +1783,9 @@ reexecute:

         hasBody = parser->flags & F_CHUNKED ||
           (parser->content_length > 0 && parser->content_length != ULLONG_MAX);
-        if (parser->upgrade && (parser->method == HTTP_CONNECT ||
-                                (parser->flags & F_SKIPBODY) || !hasBody)) {
+        if ((parser->upgrade && (parser->method == HTTP_CONNECT ||
+                                (parser->flags & F_SKIPBODY) || !hasBody))
+                                || parser->method == HTTP_HEAD) {
           /* Exit, the rest of the message is in a different protocol. */
           UPDATE_STATE(NEW_MESSAGE());
           CALLBACK_NOTIFY(message_complete);

tianchao-haohan avatar Dec 18 '17 13:12 tianchao-haohan

Can you go in more detail? The diff doesn't look Obviously Correct to me.

bnoordhuis avatar Dec 18 '17 13:12 bnoordhuis

@bnoordhuis There is no http request body for the http request with HEAD method. And I found that the parser can't be finished normally (on_message_complete callbacks is triggered) in this situation.

tianchao-haohan avatar Aug 14 '18 09:08 tianchao-haohan

This change matches a HEAD request (which had Upgrade headers or for which the user returned 2 from the on_headers_complete callback). A HEAD response may have a Content-Length but does not include the body. A HEAD request does not have the same rule. It really should not have a body, but if it has a non-zero Content-Length, the body must be present (and should be ignored by the server).

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

A payload within a HEAD request message has no defined semantics;
sending a payload body on a HEAD request might cause some existing
implementations to reject the request.

ploxiln avatar Aug 14 '18 17:08 ploxiln