Merge remote branch 'niels/http_chunk'

http.c

@@ -551,8 +551,18 @@ evhttp_connection_incoming_fail(struct evhttp_request *req,
 		 * these are cases in which we probably should just
 		 * close the connection and not send a reply.  this
 		 * case may happen when a browser keeps a persistent
-		 * connection open and we timeout on the read.
+		 * connection open and we timeout on the read.  when
+		 * the request is still being used for sending, we
+		 * need to disassociated it from the connection here.
 		 */
+		if (!req->userdone) {
+			/* remove it so that it will not be freed */
+			TAILQ_REMOVE(&req->evcon->requests, req, next);
+			/* indicate that this request no longer has a
+			 * connection object
+			 */
+			req->evcon = NULL;
+		}
 		return (-1);
 	case EVCON_HTTP_INVALID_HEADER:
 	case EVCON_HTTP_BUFFER_ERROR:
@@ -609,13 +619,12 @@ evhttp_connection_fail(struct evhttp_connection *evcon,
 		cb_arg = NULL;
 	}
 
-	TAILQ_REMOVE(&evcon->requests, req, next);
-	evhttp_request_free(req);
-
 	/* do not fail all requests; the next request is going to get
 	 * send over a new connection.   when a user cancels a request,
 	 * all other pending requests should be processed as normal
 	 */
+	TAILQ_REMOVE(&evcon->requests, req, next);
+	evhttp_request_free(req);
 
 	/* reset the connection */
 	evhttp_connection_reset(evcon);
@@ -943,7 +952,11 @@ evhttp_connection_free(struct evhttp_connection *evcon)
 			(*evcon->closecb)(evcon, evcon->closecb_arg);
 	}
 
-	/* remove all requests that might be queued on this connection */
+	/* remove all requests that might be queued on this
+	 * connection.  for server connections, this should be empty.
+	 * because it gets dequeued either in evhttp_connection_done or
+	 * evhttp_connection_fail.
+	 */
 	while ((req = TAILQ_FIRST(&evcon->requests)) != NULL) {
 		TAILQ_REMOVE(&evcon->requests, req, next);
 		evhttp_request_free(req);
@@ -2055,6 +2068,9 @@ evhttp_send(struct evhttp_request *req, struct evbuffer *databuf)
 
 	EVUTIL_ASSERT(TAILQ_FIRST(&evcon->requests) == req);
 
+	/* we expect no more calls form the user on this request */
+	req->userdone = 1;
+
 	/* xxx: not sure if we really should expose the data buffer this way */
 	if (databuf != NULL)
 		evbuffer_add_buffer(req->output_buffer, databuf);
@@ -2098,7 +2114,14 @@ evhttp_send_reply_start(struct evhttp_request *req, int code,
 void
 evhttp_send_reply_chunk(struct evhttp_request *req, struct evbuffer *databuf)
 {
-	struct evbuffer *output = bufferevent_get_output(req->evcon->bufev);
+	struct evhttp_connection *evcon = req->evcon;
+	struct evbuffer *output;
+
+	if (evcon == NULL)
+		return;
+
+	output = bufferevent_get_output(evcon->bufev);
+
 	if (evbuffer_get_length(databuf) == 0)
 		return;
 	if (!evhttp_response_needs_body(req))
@@ -2111,7 +2134,7 @@ evhttp_send_reply_chunk(struct evhttp_request *req, struct evbuffer *databuf)
 	if (req->chunked) {
 		evbuffer_add(output, "\r\n", 2);
 	}
-	evhttp_write_buffer(req->evcon, NULL, NULL);
+	evhttp_write_buffer(evcon, NULL, NULL);
 }
 
 void
@@ -2120,6 +2143,14 @@ evhttp_send_reply_end(struct evhttp_request *req)
 	struct evhttp_connection *evcon = req->evcon;
 	struct evbuffer *output = bufferevent_get_output(evcon->bufev);
 
+	if (evcon == NULL) {
+		evhttp_request_free(req);
+		return;
+	}
+
+	/* we expect no more calls form the user on this request */
+	req->userdone = 1;
+
 	if (req->chunked) {
 		evbuffer_add(output, "0\r\n\r\n", 5);
 		evhttp_write_buffer(req->evcon, evhttp_send_done, NULL);
@@ -2840,6 +2871,13 @@ evhttp_request_is_owned(struct evhttp_request *req)
 	return (req->flags & EVHTTP_USER_OWNED) != 0;
 }
 
+struct evhttp_connection *
+evhttp_request_get_connection(struct evhttp_request *req)
+{
+	return req->evcon;
+}
+
+
 void
 evhttp_request_set_chunked_cb(struct evhttp_request *req,
     void (*cb)(struct evhttp_request *, void *))

include/event2/http.h

@@ -374,6 +374,9 @@ void evhttp_request_own(struct evhttp_request *req);
 /** Returns 1 if the request is owned by the user */
 int evhttp_request_is_owned(struct evhttp_request *req);
 
+/** Returns the connection object associated with the request or NULL */
+struct evhttp_connection *evhttp_request_get_connection(struct evhttp_request *req);
+
 void evhttp_connection_set_max_headers_size(struct evhttp_connection *evcon,
     ev_ssize_t new_max_headers_size);
 

include/event2/http_struct.h

@@ -101,7 +101,8 @@ struct {
 
 	struct evbuffer *input_buffer;	/* read data */
 	ev_int64_t ntoread;
-	int chunked;
+	int chunked:1,			/* a chunked request */
+	    userdone:1;			/* the user has sent all data */
 
 	struct evbuffer *output_buffer;	/* outgoing post or data */
 

test/regress_http.c

@@ -2515,6 +2515,117 @@ http_data_length_constraints_test(void)
 		evhttp_free(http);
 }
 
+/*
+ * Testing client reset of server chunked connections
+ */
+
+struct terminate_state {
+	struct evhttp_request *req;
+	struct bufferevent *bev;
+	int fd;
+} terminate_state;
+
+static void
+terminate_chunked_trickle_cb(evutil_socket_t fd, short events, void *arg)
+{
+	struct terminate_state *state = arg;
+	struct evbuffer *evb = evbuffer_new();
+	struct timeval tv;
+
+	if (evhttp_request_get_connection(state->req) == NULL) {
+		test_ok = 1;
+		evhttp_request_free(state->req);
+		event_loopexit(NULL);
+	}
+
+	evbuffer_add_printf(evb, "%p", evb);
+	evhttp_send_reply_chunk(state->req, evb);
+	evbuffer_free(evb);
+
+	tv.tv_sec = 0;
+	tv.tv_usec = 3000;
+	event_once(-1, EV_TIMEOUT, terminate_chunked_trickle_cb, arg, &tv);
+}
+
+static void
+terminate_chunked_cb(struct evhttp_request *req, void *arg)
+{
+	struct terminate_state *state = arg;
+	struct timeval tv;
+
+	/* we need to hold on to this */
+	evhttp_request_own(req);
+
+	state->req = req;
+
+	evhttp_send_reply_start(req, HTTP_OK, "OK");
+
+	tv.tv_sec = 0;
+	tv.tv_usec = 3000;
+	event_once(-1, EV_TIMEOUT, terminate_chunked_trickle_cb, arg, &tv);
+}
+
+static void
+terminate_chunked_client(int fd, short event, void *arg)
+{
+	struct terminate_state *state = arg;
+	bufferevent_free(state->bev);
+	EVUTIL_CLOSESOCKET(state->fd);
+}
+
+static void
+terminate_readcb(struct bufferevent *bev, void *arg)
+{
+	/* just drop the data */
+	evbuffer_drain(bufferevent_get_input(bev), -1);
+}
+
+
+static void
+http_terminate_chunked_test(void)
+{
+	struct bufferevent *bev = NULL;
+	struct timeval tv;
+	const char *http_request;
+	short port = -1;
+	int fd = -1;
+
+	test_ok = 0;
+
+	http = http_setup(&port, NULL);
+	evhttp_del_cb(http, "/test");
+	tt_assert(evhttp_set_cb(http, "/test",
+		terminate_chunked_cb, &terminate_state) == 0);
+
+	fd = http_connect("127.0.0.1", port);
+
+	/* Stupid thing to send a request */
+	bev = bufferevent_new(fd, terminate_readcb, http_writecb,
+	    http_errorcb, NULL);
+
+	terminate_state.fd = fd;
+	terminate_state.bev = bev;
+
+	/* first half of the http request */
+	http_request =
+	    "GET /test HTTP/1.1\r\n"
+	    "Host: some\r\n\r\n";
+
+	bufferevent_write(bev, http_request, strlen(http_request));
+	evutil_timerclear(&tv);
+	tv.tv_usec = 10000;
+	event_once(-1, EV_TIMEOUT, terminate_chunked_client, &terminate_state,
+	    &tv);
+
+	event_dispatch();
+
+ end:
+	if (fd >= 0)
+		EVUTIL_CLOSESOCKET(fd);
+	if (http)
+		evhttp_free(http);
+}
+
 #define HTTP_LEGACY(name)						\
 	{ #name, run_legacy_test_fn, TT_ISOLATED|TT_LEGACY, &legacy_setup, \
                     http_##name##_test }
@@ -2539,6 +2650,9 @@ struct testcase_t http_testcases[] = {
 	HTTP_LEGACY(bad_request),
 	HTTP_LEGACY(incomplete),
 	HTTP_LEGACY(incomplete_timeout),
+	{ "terminate_chunked", run_legacy_test_fn,
+	  TT_ISOLATED|TT_LEGACY, &legacy_setup,
+	  http_terminate_chunked_test },
 
 	HTTP_LEGACY(highport),
 	HTTP_LEGACY(dispatcher),