From: Niels Provos Date: Wed, 2 Jul 2008 06:08:16 +0000 (+0000) Subject: From Scott Lamb: X-Git-Tag: release-2.0.1-alpha~241 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=df97fca9ca112c4ed24a0b81b5115b53c3f2fc47;p=libevent From Scott Lamb: * Allow the user to set the Content-Length: then stream a reply. This is useful for large requests of a known size. Added unit test. * Don't send a response body on HEAD requests, 1xx status codes, 204 status codes, or 304 status codes, as described in RFC 2616 section 4.3. (Doing otherwise causes problems - in particular, if a 304 has a chunked body (even an empty one), Safari 3.1.1 issues and then fails the next request on the connection with the non-sequitur error message "Too many HTTP redirects"!) * Specify a default Content-Type: when a response body is required, not when we have data in the response buffer by the time we make the header. (I.e., do this on evhttp_send_reply_start() for consistency.) * Don't expect a body in response to HEAD requests. svn:r898 --- diff --git a/ChangeLog b/ChangeLog index f19cbd22..4fa55b3c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -115,6 +115,7 @@ Changes in current version: o Support multi-line HTTP headers; based on a patch from Moshe Litvin o Reject negative Content-Length headers; anonymous bug report o Detect CLOCK_MONOTONIC at runtime for evdns; anonymous bug report + o Various HTTP correctness fixes from Scott Lamb Changes in 1.4.0: o allow \r or \n individually to separate HTTP headers instead of the standard "\r\n"; from Charles Kerr. diff --git a/http.c b/http.c index b5eac236..b283685d 100644 --- a/http.c +++ b/http.c @@ -326,6 +326,21 @@ evhttp_method(enum evhttp_cmd_type type) return (method); } +/** + * Determines if a response should have a body. + * Follows the rules in RFC 2616 section 4.3. + * @return 1 if the response MUST have a body; + * 0 if the response MUST NOT have a body. + */ +static int +evhttp_response_needs_body(struct evhttp_request *req) +{ + return (req->response_code != HTTP_NOCONTENT && + req->response_code != HTTP_NOTMODIFIED && + (req->response_code < 100 || req->response_code >= 200) && + req->type != EVHTTP_REQ_HEAD); +} + static void evhttp_add_event(struct event *ev, int timeout, int default_timeout) { @@ -482,7 +497,8 @@ evhttp_make_header_response(struct evhttp_connection *evcon, evhttp_add_header(req->output_headers, "Connection", "keep-alive"); - if (req->minor == 1 || is_keepalive) { + if ((req->minor == 1 || is_keepalive) && + evhttp_response_needs_body(req)) { /* * we need to add the content length if the * user did not give it, this is required for @@ -495,7 +511,7 @@ evhttp_make_header_response(struct evhttp_connection *evcon, } /* Potentially add headers for unidentified content. */ - if (EVBUFFER_LENGTH(req->output_buffer)) { + if (evhttp_response_needs_body(req)) { if (evhttp_find_header(req->output_headers, "Content-Type") == NULL) { evhttp_add_header(req->output_headers, @@ -1601,9 +1617,7 @@ evhttp_read_header(struct evhttp_connection *evcon, break; case EVHTTP_RESPONSE: - if (req->response_code == HTTP_NOCONTENT || - req->response_code == HTTP_NOTMODIFIED || - (req->response_code >= 100 && req->response_code < 200)) { + if (!evhttp_response_needs_body(req)) { event_debug(("%s: skipping body for code %d\n", __func__, req->response_code)); evhttp_connection_done(evcon); @@ -1957,8 +1971,14 @@ evhttp_send_reply_start(struct evhttp_request *req, int code, /* set up to watch for client close */ evhttp_connection_start_detectclose(req->evcon); evhttp_response_code(req, code, reason); - if (req->major == 1 && req->minor == 1) { - /* use chunked encoding for HTTP/1.1 */ + if (evhttp_find_header(req->output_headers, "Content-Length") == NULL && + req->major == 1 && req->minor == 1 && + evhttp_response_needs_body(req)) { + /* + * prefer HTTP/1.1 chunked encoding to closing the connection; + * note RFC 2616 section 4.4 forbids it with Content-Length: + * and it's not necessary then anyway. + */ evhttp_add_header(req->output_headers, "Transfer-Encoding", "chunked"); req->chunked = 1; @@ -1971,6 +1991,10 @@ void evhttp_send_reply_chunk(struct evhttp_request *req, struct evbuffer *databuf) { struct evbuffer *output = bufferevent_get_output(req->evcon->bufev); + if (EVBUFFER_LENGTH(databuf) == 0) + return; + if (!evhttp_response_needs_body(req)) + return; if (req->chunked) { evbuffer_add_printf(output, "%x\r\n", (unsigned)EVBUFFER_LENGTH(databuf)); diff --git a/test/regress_http.c b/test/regress_http.c index bd4469bf..207814ad 100644 --- a/test/regress_http.c +++ b/test/regress_http.c @@ -64,6 +64,8 @@ static struct evhttp *http; /* set if a test needs to call loopexit on a base */ static struct event_base *base; +static char const BASIC_REQUEST_BODY[] = "This is funny"; + void http_suite(void); static void http_basic_cb(struct evhttp_request *req, void *arg); @@ -96,6 +98,7 @@ http_setup(short *pport, struct event_base *base) /* Register a callback for certain types of requests */ evhttp_set_cb(myhttp, "/test", http_basic_cb, NULL); evhttp_set_cb(myhttp, "/chunked", http_chunked_cb, NULL); + evhttp_set_cb(myhttp, "/streamed", http_chunked_cb, NULL); evhttp_set_cb(myhttp, "/postit", http_post_cb, NULL); evhttp_set_cb(myhttp, "/putit", http_put_cb, NULL); evhttp_set_cb(myhttp, "/deleteit", http_delete_cb, NULL); @@ -174,7 +177,7 @@ http_connect(const char *address, u_short port) static void http_readcb(struct bufferevent *bev, void *arg) { - const char *what = "This is funny"; + const char *what = BASIC_REQUEST_BODY; event_debug(("%s: %s\n", __func__, EVBUFFER_DATA(EVBUFFER_INPUT(bev)))); @@ -230,7 +233,7 @@ http_basic_cb(struct evhttp_request *req, void *arg) struct evbuffer *evb = evbuffer_new(); int empty = evhttp_find_header(req->input_headers, "Empty") != NULL; event_debug(("%s: called\n", __func__)); - evbuffer_add_printf(evb, "This is funny"); + evbuffer_add_printf(evb, BASIC_REQUEST_BODY); /* For multi-line headers test */ { @@ -297,7 +300,11 @@ http_chunked_cb(struct evhttp_request *req, void *arg) memset(state, 0, sizeof(struct chunk_req_state)); state->req = req; - /* generate a chunked reply */ + if (strcmp(evhttp_request_uri(req), "/streamed") == 0) { + evhttp_add_header(req->output_headers, "Content-Length", "39"); + } + + /* generate a chunked/streamed reply */ evhttp_send_reply_start(req, HTTP_OK, "Everything is fine"); /* but trickle it across several iterations to ensure we're not @@ -416,7 +423,7 @@ http_delete_cb(struct evhttp_request *req, void *arg) } event_debug(("%s: called\n", __func__)); - evbuffer_add_printf(evb, "This is funny"); + evbuffer_add_printf(evb, BASIC_REQUEST_BODY); /* allow sending of an empty reply */ evhttp_send_reply(req, HTTP_OK, "Everything is fine", @@ -494,7 +501,7 @@ http_connection_test(int persistent) * server using our make request method. */ - req = evhttp_request_new(http_request_done, NULL); + req = evhttp_request_new(http_request_done, (void*) BASIC_REQUEST_BODY); /* Add the information that we care about */ evhttp_add_header(req->output_headers, "Host", "somehost"); @@ -515,7 +522,7 @@ http_connection_test(int persistent) /* try to make another request over the same connection */ test_ok = 0; - req = evhttp_request_new(http_request_done, NULL); + req = evhttp_request_new(http_request_done, (void*) BASIC_REQUEST_BODY); /* Add the information that we care about */ evhttp_add_header(req->output_headers, "Host", "somehost"); @@ -636,7 +643,7 @@ http_cancel_test(void) /* try to make another request over the same connection */ test_ok = 0; - req = evhttp_request_new(http_request_done, NULL); + req = evhttp_request_new(http_request_done, (void*) BASIC_REQUEST_BODY); /* Add the information that we care about */ evhttp_add_header(req->output_headers, "Host", "somehost"); @@ -679,7 +686,7 @@ http_cancel_test(void) static void http_request_done(struct evhttp_request *req, void *arg) { - const char *what = "This is funny"; + const char *what = arg; if (req->response_code != HTTP_OK) { fprintf(stderr, "FAILED\n"); @@ -776,7 +783,7 @@ http_virtual_host_test(void) test_ok = 0; /* make a request with the right host and expect a response */ - req = evhttp_request_new(http_request_done, NULL); + req = evhttp_request_new(http_request_done, (void*) BASIC_REQUEST_BODY); /* Add the information that we care about */ evhttp_add_header(req->output_headers, "Host", "foo.com"); @@ -798,7 +805,7 @@ http_virtual_host_test(void) test_ok = 0; /* make a request with the right host and expect a response */ - req = evhttp_request_new(http_request_done, NULL); + req = evhttp_request_new(http_request_done, (void*) BASIC_REQUEST_BODY); /* Add the information that we care about */ evhttp_add_header(req->output_headers, "Host", "bar.magic.foo.com"); @@ -1046,7 +1053,7 @@ http_post_cb(struct evhttp_request *req, void *arg) } evb = evbuffer_new(); - evbuffer_add_printf(evb, "This is funny"); + evbuffer_add_printf(evb, BASIC_REQUEST_BODY); evhttp_send_reply(req, HTTP_OK, "Everything is fine", evb); @@ -1056,7 +1063,7 @@ http_post_cb(struct evhttp_request *req, void *arg) void http_postrequest_done(struct evhttp_request *req, void *arg) { - const char *what = "This is funny"; + const char *what = BASIC_REQUEST_BODY; if (req == NULL) { fprintf(stderr, "FAILED (timeout)\n"); @@ -1719,7 +1726,7 @@ http_chunked_request_done(struct evhttp_request *req, void *arg) } static void -http_chunked_test(void) +http_chunk_out_test(void) { struct bufferevent *bev; int fd; @@ -1804,6 +1811,55 @@ http_chunked_test(void) fprintf(stdout, "OK\n"); } +static void +http_stream_out_test(void) +{ + short port = -1; + struct evhttp_connection *evcon = NULL; + struct evhttp_request *req = NULL; + + test_ok = 0; + printf("Tests streaming responses out: "); + + http = http_setup(&port, NULL); + + evcon = evhttp_connection_new("127.0.0.1", port); + if (evcon == NULL) { + fprintf(stdout, "FAILED\n"); + exit(1); + } + + /* + * At this point, we want to schedule a request to the HTTP + * server using our make request method. + */ + + req = evhttp_request_new(http_request_done, + (void *)"This is funnybut not hilarious.bwv 1052"); + + /* Add the information that we care about */ + evhttp_add_header(req->output_headers, "Host", "somehost"); + + /* We give ownership of the request to the connection */ + if (evhttp_make_request(evcon, req, EVHTTP_REQ_GET, "/streamed") + == -1) { + fprintf(stdout, "FAILED\n"); + exit(1); + } + + event_dispatch(); + + if (test_ok != 1) { + fprintf(stdout, "FAILED\n"); + exit(1); + } + + evhttp_connection_free(evcon); + evhttp_free(http); + + fprintf(stdout, "OK\n"); +} + static void http_stream_in_chunk(struct evhttp_request *req, void *arg) { @@ -1885,7 +1941,8 @@ http_stream_in_test(void) "This is funnybut not hilarious.bwv 1052"); printf("Testing streamed reading of non-chunked response: "); - _http_stream_in_test("/test", 13, "This is funny"); + _http_stream_in_test("/test", strlen(BASIC_REQUEST_BODY), + BASIC_REQUEST_BODY); } static void @@ -2273,7 +2330,8 @@ http_suite(void) http_incomplete_test(0 /* use_timeout */); http_incomplete_test(1 /* use_timeout */); - http_chunked_test(); + http_chunk_out_test(); + http_stream_out_test(); http_stream_in_test(); http_stream_in_cancel_test();