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

Commit

Permalink
Fix response body is not read
Browse files Browse the repository at this point in the history
With HTTP/1.1, if neither Content-Length nor Transfer-Encoding is present,
section 4.4 of RFC 2616 suggests http-parser needs to read a response body
until the connection is closed (except the response must not include a body)

See also nodejs/node-v0.x-archive#2457.
Fixes #72
  • Loading branch information
koichik authored and ry committed Jan 6, 2012
1 parent 3cf68f9 commit b47c44d
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 8 deletions.
15 changes: 13 additions & 2 deletions http_parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -1733,9 +1733,20 @@ http_should_keep_alive (http_parser *parser)
/* HTTP/1.1 */
if (parser->flags & F_CONNECTION_CLOSE) {
return 0;
} else {
return 1;
}
if (parser->type == HTTP_RESPONSE) {
/* See RFC 2616 section 4.4 */
if (parser->status_code / 100 == 1 || /* 1xx e.g. Continue */

This comment has been minimized.

Copy link
@pgriess

pgriess Jan 6, 2012

Contributor

I believe we need to be doing this status code check for HTTP/1.0 as well.

This comment has been minimized.

Copy link
@koichik

koichik Jan 7, 2012

Author Contributor

@pgriess - With HTTP/1.0 or earlier, Keep-Alive is disabled by default. Therefore, I think that we do not need it. Keep-Alive is used if and only if the response has Connection: Keep-Alive (or legacy Keep-Alive:, but http-parser does not seem to support it).

[EDIT] I may understand what you mean. HTTP/1.0 server sends Connection: Keep-Alive, but the body length may be unable to be determined.

This comment has been minimized.

Copy link
@pgriess

pgriess Jan 7, 2012

Contributor

Yeah, if the client and server both agree on keep-alive (i.e. by both sending "Connection: keep-alive" headers), we're dealing with the same situation as HTTP/1.1: we have a bytestream that contains a response with no body followed by another response. The case we're dealing with here is not that we can't determine the response length -- it's that we know that the length is 0.

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) {
Expand Down
50 changes: 44 additions & 6 deletions test.c
Original file line number Diff line number Diff line change
Expand Up @@ -780,8 +780,8 @@ const struct message responses[] =
, {.name= "404 no headers no body"
,.type= HTTP_RESPONSE
,.raw= "HTTP/1.1 404 Not Found\r\n\r\n"
,.should_keep_alive= TRUE
,.message_complete_on_eof= FALSE
,.should_keep_alive= FALSE
,.message_complete_on_eof= TRUE
,.http_major= 1
,.http_minor= 1
,.status_code= 404
Expand All @@ -795,8 +795,8 @@ const struct message responses[] =
, {.name= "301 no response phrase"
,.type= HTTP_RESPONSE
,.raw= "HTTP/1.1 301\r\n\r\n"
,.should_keep_alive = TRUE
,.message_complete_on_eof= FALSE
,.should_keep_alive = FALSE
,.message_complete_on_eof= TRUE
,.http_major= 1
,.http_minor= 1
,.status_code= 301
Expand Down Expand Up @@ -1057,8 +1057,46 @@ const struct message responses[] =
{}
,.body= ""
}
, {.name= NULL } /* sentinel */

#define NO_CONTENT_LENGTH_NO_TRANSFER_ENCODING_RESPONSE 13
/* The client should wait for the server's EOF. That is, when neither
* content-length nor transfer-encoding is specified, the end of body
* is specified by the EOF.
*/
, {.name= "neither content-length nor transfer-encoding response"
,.type= HTTP_RESPONSE
,.raw= "HTTP/1.1 200 OK\r\n"
"Content-Type: text/plain\r\n"
"\r\n"
"hello world"
,.should_keep_alive= FALSE
,.message_complete_on_eof= TRUE
,.http_major= 1
,.http_minor= 1
,.status_code= 200
,.num_headers= 1
,.headers=
{ { "Content-Type", "text/plain" }
}
,.body= "hello world"
}

#define NO_HEADERS_NO_BODY_204 14
, {.name= "204 no headers no body"
,.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= {}
,.body_size= 0
,.body= ""
}

, {.name= NULL } /* sentinel */
};

int
Expand Down Expand Up @@ -1888,7 +1926,7 @@ main (void)

printf("response scan 1/2 ");
test_scan( &responses[TRAILING_SPACE_ON_CHUNKED_BODY]
, &responses[NO_HEADERS_NO_BODY_404]
, &responses[NO_HEADERS_NO_BODY_204]
, &responses[NO_REASON_PHRASE]
);

Expand Down

0 comments on commit b47c44d

Please sign in to comment.