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

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.

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 ||
+		if (!evhttp_response_needs_body(req)) {
-		    req->response_code == HTTP_NOTMODIFIED ||
-		    (req->response_code >= 100 && req->response_code < 200)) {
 			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) {
+	if (evhttp_find_header(req->output_headers, "Content-Length") == NULL &&
-		/* use chunked encoding for HTTP/1.1 */
+	    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));

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();