Skip to content
This repository was archived by the owner on Nov 6, 2022. It is now read-only.

Always skip body for 1xx, 204, and 304 responses #464

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Always skip body for 1xx, 204, and 304 responses #464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
068f054
Always skip body for 1xx, 204, and 304 responses
azaretsky Mar 18, 2019
b25158b
Change the check for 1xx status code
azaretsky Mar 19, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 21 additions & 28 deletions http_parser.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,6 @@ static struct {
};
#undef HTTP_STRERROR_GEN

int http_message_needs_eof(const http_parser *parser);

/* Our URL parser.
*
* This is designed to be shared by http_parser_execute() for URL validation,
Expand Down Expand Up @@ -887,6 +885,13 @@ size_t http_parser_execute (http_parser *parser,

case s_res_status_start:
{
/* See RFC 7230 section 3.3.3, step 1 */
if ((parser->status_code >= 100 &&
parser->status_code < 200) || /* 1xx e.g. Continue */
parser->status_code == 204 || /* No Content */
parser->status_code == 304) { /* Not Modified */
parser->flags |= F_SKIPBODY;
}
MARK(status);
UPDATE_STATE(s_res_status);
parser->index = 0;
Expand Down Expand Up @@ -1856,7 +1861,7 @@ size_t http_parser_execute (http_parser *parser,
/* Content-Length header given and non-zero */
UPDATE_STATE(s_body_identity);
} else {
if (!http_message_needs_eof(parser)) {
if (parser->type == HTTP_REQUEST) {
/* Assume content-length 0 - read the next */
UPDATE_STATE(NEW_MESSAGE());
CALLBACK_NOTIFY(message_complete);
Expand Down Expand Up @@ -2084,30 +2089,6 @@ size_t http_parser_execute (http_parser *parser,
}


/* Does the parser need to see an EOF to find the end of the message? */
int
http_message_needs_eof (const http_parser *parser)
{
if (parser->type == HTTP_REQUEST) {
return 0;
}

/* See RFC 2616 section 4.4 */
if (parser->status_code / 100 == 1 || /* 1xx e.g. Continue */
parser->status_code == 204 || /* No Content */
parser->status_code == 304 || /* Not Modified */
parser->flags & F_SKIPBODY) { /* response to a HEAD request */
return 0;
}

if ((parser->flags & F_CHUNKED) || parser->content_length != ULLONG_MAX) {
return 0;
}

return 1;
}


int
http_should_keep_alive (const http_parser *parser)
{
Expand All @@ -2123,7 +2104,19 @@ http_should_keep_alive (const http_parser *parser)
}
}

return !http_message_needs_eof(parser);
/* RFC 7230 section 3.3.3, step 7:
* ... this is a response message without a declared message body length, so
* the message body length is determined by the number of octets received
* prior to the server closing the connection.
*/
if (parser->type == HTTP_RESPONSE &&
!(parser->flags & F_SKIPBODY) &&
!(parser->flags & F_CHUNKED) &&
parser->content_length == ULLONG_MAX) {
return 0;
}

return 1;
}


Expand Down
8 changes: 2 additions & 6 deletions test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1848,8 +1848,7 @@ const struct message responses[] =
,.http_minor= 1
,.status_code= 101
,.response_status= "Switching Protocols"
,.body= "body"
,.upgrade= "proto"
,.upgrade= "bodyproto"
Copy link
Member

Choose a reason for hiding this comment

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

See #363, it's possible someone might be relying on the current behavior.

Copy link
Author

Choose a reason for hiding this comment

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

This PR only changes the tests for 101 responses. The HTTP_200_RESPONSE_WITH_UPGRADE_HEADER* tests are unaffected (and they all pass).

RFC 7230 explicitly forbids using Content-Length or Transfer-Encoding in 1xx responses ("A server MUST NOT send" etc.) and RFC 2616 also had similarly strict requirements ("All 1xx ... responses MUST NOT include a message-body"), so I think it would probably be ok.

,.num_headers= 3
,.headers=
{ { "Connection", "upgrade" }
Expand Down Expand Up @@ -1879,16 +1878,13 @@ const struct message responses[] =
,.http_minor= 1
,.status_code= 101
,.response_status= "Switching Protocols"
,.body= "body"
,.upgrade= "proto"
,.upgrade= "2\r\nbo\r\n2\r\ndy\r\n0\r\n\r\nproto"
,.num_headers= 3
,.headers=
{ { "Connection", "upgrade" }
, { "Upgrade", "h2c" }
, { "Transfer-Encoding", "chunked" }
}
,.num_chunks_complete= 3
,.chunk_lengths= { 2, 2 }
}

#define HTTP_200_RESPONSE_WITH_UPGRADE_HEADER 25
Expand Down