From 248fbc3ab459d987a40eae8ed2bce7cc13e58372 Mon Sep 17 00:00:00 2001 From: Peter Griess Date: Fri, 6 Jan 2012 17:33:26 -0600 Subject: [PATCH 1/2] Get HTTP/1.1 message length logic working for HTTP/1.0 - Port message length logic from #72 to HTTP/1.0. - Add a bunch of unit tests for handling 0-length messages. --- http_parser.c | 35 +++++++++-------- test.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 120 insertions(+), 22 deletions(-) diff --git a/http_parser.c b/http_parser.c index 5ab2be2..52992f9 100644 --- a/http_parser.c +++ b/http_parser.c @@ -1712,27 +1712,30 @@ http_should_keep_alive (http_parser *parser) if (parser->flags & F_CONNECTION_CLOSE) { return 0; } - if (parser->type == HTTP_RESPONSE) { - /* 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 1; - } - if (!(parser->flags & F_CHUNKED) && parser->content_length == -1) { - return 0; - } - } - return 1; } else { /* HTTP/1.0 or earlier */ - if (parser->flags & F_CONNECTION_KEEP_ALIVE) { - return 1; - } else { + if (!(parser->flags & F_CONNECTION_KEEP_ALIVE)) { return 0; } } + + if (parser->type == HTTP_REQUEST) { + return 1; + } + + /* 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 1; + } + + if (parser->flags & F_CHUNKED || parser->content_length >= 0) { + return 1; + } + + return 0; } diff --git a/test.c b/test.c index 807921f..146d7c4 100644 --- a/test.c +++ b/test.c @@ -1141,23 +1141,118 @@ const struct message responses[] = ,.body= "hello world" } -#define NO_HEADERS_NO_BODY_204 13 -, {.name= "204 no headers no body" +#define NO_BODY_HTTP10_KA_200 13 +, {.name= "HTTP/1.0 with keep-alive and EOF-terminated 200 status" ,.type= HTTP_RESPONSE - ,.raw= "HTTP/1.1 204 No Content\r\n\r\n" + ,.raw= "HTTP/1.0 200 OK\r\n" + "Connection: keep-alive\r\n" + "\r\n" + ,.should_keep_alive= FALSE + ,.message_complete_on_eof= TRUE + ,.http_major= 1 + ,.http_minor= 0 + ,.status_code= 200 + ,.num_headers= 1 + ,.headers= + { { "Connection", "keep-alive" } + } + ,.body_size= 0 + ,.body= "" + } + +#define NO_BODY_HTTP10_KA_204 14 +, {.name= "HTTP/1.0 with keep-alive and a 204 status" + ,.type= HTTP_RESPONSE + ,.raw= "HTTP/1.0 204 No content\r\n" + "Connection: keep-alive\r\n" + "\r\n" + ,.should_keep_alive= TRUE + ,.message_complete_on_eof= FALSE + ,.http_major= 1 + ,.http_minor= 0 + ,.status_code= 204 + ,.num_headers= 1 + ,.headers= + { { "Connection", "keep-alive" } + } + ,.body_size= 0 + ,.body= "" + } + +#define NO_BODY_HTTP11_KA_200 15 +, {.name= "HTTP/1.1 with an EOF-terminated 200 status" + ,.type= HTTP_RESPONSE + ,.raw= "HTTP/1.1 200 OK\r\n" + "\r\n" + ,.should_keep_alive= FALSE + ,.message_complete_on_eof= TRUE + ,.http_major= 1 + ,.http_minor= 1 + ,.status_code= 200 + ,.num_headers= 0 + ,.headers={} + ,.body_size= 0 + ,.body= "" + } + +#define NO_BODY_HTTP11_KA_204 16 +, {.name= "HTTP/1.1 with a 204 status" + ,.type= HTTP_RESPONSE + ,.raw= "HTTP/1.1 204 No content\r\n" + "\r\n" ,.should_keep_alive= TRUE ,.message_complete_on_eof= FALSE ,.http_major= 1 ,.http_minor= 1 ,.status_code= 204 ,.num_headers= 0 - ,.headers= {} + ,.headers={} + ,.body_size= 0 + ,.body= "" + } + +#define NO_BODY_HTTP11_NOKA_204 17 +, {.name= "HTTP/1.1 with a 204 status and keep-alive disabled" + ,.type= HTTP_RESPONSE + ,.raw= "HTTP/1.1 204 No content\r\n" + "Connection: close\r\n" + "\r\n" + ,.should_keep_alive= FALSE + ,.message_complete_on_eof= TRUE + ,.http_major= 1 + ,.http_minor= 1 + ,.status_code= 204 + ,.num_headers= 1 + ,.headers= + { { "Connection", "close" } + } + ,.body_size= 0 + ,.body= "" + } + +#define NO_BODY_HTTP11_KA_CHUNKED_200 18 +, {.name= "HTTP/1.1 with chunked endocing and a 200 response" + ,.type= HTTP_RESPONSE + ,.raw= "HTTP/1.1 200 OK\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "0\r\n" + "\r\n" + ,.should_keep_alive= TRUE + ,.message_complete_on_eof= FALSE + ,.http_major= 1 + ,.http_minor= 1 + ,.status_code= 200 + ,.num_headers= 1 + ,.headers= + { { "Transfer-Encoding", "chunked" } + } ,.body_size= 0 ,.body= "" } #if !HTTP_PARSER_STRICT -#define SPACE_IN_FIELD_RES 14 +#define SPACE_IN_FIELD_RES 19 /* Should handle spaces in header fields */ , {.name= "field space" ,.type= HTTP_RESPONSE @@ -2056,7 +2151,7 @@ main (void) printf("response scan 1/2 "); test_scan( &responses[TRAILING_SPACE_ON_CHUNKED_BODY] - , &responses[NO_HEADERS_NO_BODY_204] + , &responses[NO_BODY_HTTP10_KA_204] , &responses[NO_REASON_PHRASE] ); From b115d110a3e82b8dceb6941534c3ed1c4378458f Mon Sep 17 00:00:00 2001 From: Peter Griess Date: Sat, 7 Jan 2012 17:37:05 -0600 Subject: [PATCH 2/2] Don't wait for EOF on 0-length KA messages. - Break EOF handling out of http_should_keep_alive() into http_message_needs_eof(), which we now use when determining what to do with a message of unknown length. This prevents us from falling into the s_body_identity_eof state in the cases where we actually *do* know the length of the message (e.g. because the response status was 204). --- http_parser.c | 47 +++++++++++++++++++++++++++++------------------ test.c | 2 +- 2 files changed, 30 insertions(+), 19 deletions(-) diff --git a/http_parser.c b/http_parser.c index 52992f9..9150c05 100644 --- a/http_parser.c +++ b/http_parser.c @@ -358,6 +358,8 @@ static struct { }; #undef HTTP_STRERROR_GEN +int http_message_needs_eof(http_parser *parser); + /* Our URL parser. * * This is designed to be shared by http_parser_execute() for URL validation, @@ -1541,7 +1543,8 @@ size_t http_parser_execute (http_parser *parser, /* Content-Length header given and non-zero */ state = s_body_identity; } else { - if (parser->type == HTTP_REQUEST || http_should_keep_alive(parser)) { + if (parser->type == HTTP_REQUEST || + !http_message_needs_eof(parser)) { /* Assume content-length 0 - read the next */ CALLBACK2(message_complete); state = NEW_MESSAGE(); @@ -1704,6 +1707,30 @@ error: } +/* Does the parser need to see an EOF to find the end of the message? */ +int +http_message_needs_eof (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 >= 0) { + return 0; + } + + return 1; +} + + int http_should_keep_alive (http_parser *parser) { @@ -1719,23 +1746,7 @@ http_should_keep_alive (http_parser *parser) } } - if (parser->type == HTTP_REQUEST) { - return 1; - } - - /* 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 1; - } - - if (parser->flags & F_CHUNKED || parser->content_length >= 0) { - return 1; - } - - return 0; + return !http_message_needs_eof(parser); } diff --git a/test.c b/test.c index 146d7c4..fd41d97 100644 --- a/test.c +++ b/test.c @@ -1218,7 +1218,7 @@ const struct message responses[] = "Connection: close\r\n" "\r\n" ,.should_keep_alive= FALSE - ,.message_complete_on_eof= TRUE + ,.message_complete_on_eof= FALSE ,.http_major= 1 ,.http_minor= 1 ,.status_code= 204