From 068f05431394421d3ba43669f767ddcce2967b84 Mon Sep 17 00:00:00 2001 From: Anatoly Zaretsky Date: Mon, 18 Mar 2019 03:44:54 +0200 Subject: [PATCH 1/2] Always skip body for 1xx, 204, and 304 responses Fixes: https://github.com/nodejs/http-parser/issues/251 --- http_parser.c | 48 ++++++++++++++++++++---------------------------- test.c | 8 ++------ 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/http_parser.c b/http_parser.c index 48963853..89a04ded 100644 --- a/http_parser.c +++ b/http_parser.c @@ -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, @@ -887,6 +885,12 @@ 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 == 1 || /* 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; @@ -1856,7 +1860,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); @@ -2084,30 +2088,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) { @@ -2123,7 +2103,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; } diff --git a/test.c b/test.c index 0140a18b..84f290aa 100644 --- a/test.c +++ b/test.c @@ -1848,8 +1848,7 @@ const struct message responses[] = ,.http_minor= 1 ,.status_code= 101 ,.response_status= "Switching Protocols" - ,.body= "body" - ,.upgrade= "proto" + ,.upgrade= "bodyproto" ,.num_headers= 3 ,.headers= { { "Connection", "upgrade" } @@ -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 From b25158b1e9c3cc9b5e752753ccbf9a8d7b31b1df Mon Sep 17 00:00:00 2001 From: Anatoly Zaretsky Date: Tue, 19 Mar 2019 16:21:41 +0200 Subject: [PATCH 2/2] Change the check for 1xx status code "status_code >= 100 && status_code < 200" might be faster than "status_code / 100 == 1" --- http_parser.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/http_parser.c b/http_parser.c index 89a04ded..d6950198 100644 --- a/http_parser.c +++ b/http_parser.c @@ -886,10 +886,11 @@ 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 == 1 || /* 1xx e.g. Continue */ + 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; + parser->flags |= F_SKIPBODY; } MARK(status); UPDATE_STATE(s_res_status);