From 272823f8b0fb3c5f752ea5ff79b9b223b4d1b453 Mon Sep 17 00:00:00 2001 From: Tomash Brechko Date: Wed, 23 Mar 2011 12:05:33 +0300 Subject: [PATCH 1/3] Reset outgoing connection when read data in idle state. Imagine server side is buggy and miscalculates Content-Length: in the reply. Data arriving in idle state shouldn't make us crash, instead we can just reset the connection. --- http.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/http.c b/http.c index 3a24df2f..1429462e 100644 --- a/http.c +++ b/http.c @@ -1021,9 +1021,22 @@ evhttp_read_cb(struct bufferevent *bufev, void *arg) case EVCON_READING_TRAILER: evhttp_read_trailer(evcon, req); break; + case EVCON_IDLE: + { + struct evbuffer *input; + size_t total_len; + + input = bufferevent_get_input(evcon->bufev); + total_len = evbuffer_get_length(input); + event_debug(("%s: read %d bytes in EVCON_IDLE state," + " resetting connection", + __func__, (int)total_len)); + + evhttp_connection_reset(evcon); + } + break; case EVCON_DISCONNECTED: case EVCON_CONNECTING: - case EVCON_IDLE: case EVCON_WRITING: default: event_errx(1, "%s: illegal connection state %d", From 218cf19743e3671924ec01edc9dc26be8b6f1ae8 Mon Sep 17 00:00:00 2001 From: Tomash Brechko Date: Thu, 24 Mar 2011 15:52:34 +0300 Subject: [PATCH 2/3] Fix subtle recursion in evhttp_connection_cb_cleanup(). --- http.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/http.c b/http.c index 1429462e..fa2b9cc3 100644 --- a/http.c +++ b/http.c @@ -1230,6 +1230,8 @@ evhttp_connection_retry(evutil_socket_t fd, short what, void *arg) static void evhttp_connection_cb_cleanup(struct evhttp_connection *evcon) { + struct evcon_requestq requests; + if (evcon->retry_max < 0 || evcon->retry_cnt < evcon->retry_max) { evtimer_assign(&evcon->retry_ev, evcon->base, evhttp_connection_retry, evcon); /* XXXX handle failure from evhttp_add_event */ @@ -1241,10 +1243,23 @@ evhttp_connection_cb_cleanup(struct evhttp_connection *evcon) } evhttp_connection_reset(evcon); - /* for now, we just signal all requests by executing their callbacks */ + /* + * User callback can do evhttp_make_request() on the same + * evcon so new request will be added to evcon->requests. To + * avoid freeing it prematurely we iterate over the copy of + * the queue. + */ + TAILQ_INIT(&requests); while (TAILQ_FIRST(&evcon->requests) != NULL) { struct evhttp_request *request = TAILQ_FIRST(&evcon->requests); TAILQ_REMOVE(&evcon->requests, request, next); + TAILQ_INSERT_TAIL(&requests, request, next); + } + + /* for now, we just signal all requests by executing their callbacks */ + while (TAILQ_FIRST(&requests) != NULL) { + struct evhttp_request *request = TAILQ_FIRST(&requests); + TAILQ_REMOVE(&requests, request, next); request->evcon = NULL; /* we might want to set an error here */ From 0d6622e26a83c1efac4a26a5b98e86ca61dc0a7f Mon Sep 17 00:00:00 2001 From: Tomash Brechko Date: Thu, 31 Mar 2011 19:11:10 +0400 Subject: [PATCH 3/3] Fix the case when failed evhttp_make_request() leaved request in the queue. --- http.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/http.c b/http.c index fa2b9cc3..0bdf2fa8 100644 --- a/http.c +++ b/http.c @@ -2191,11 +2191,20 @@ evhttp_make_request(struct evhttp_connection *evcon, req->evcon = evcon; EVUTIL_ASSERT(!(req->flags & EVHTTP_REQ_OWN_CONNECTION)); - TAILQ_INSERT_TAIL(&evcon->requests, req, next); - /* If the connection object is not connected; make it so */ - if (!evhttp_connected(evcon)) - return (evhttp_connection_connect(evcon)); + if (!evhttp_connected(evcon)) { + int res = evhttp_connection_connect(evcon); + /* + * Enqueue the request only if we aren't going to + * return failure from evhttp_make_request(). + */ + if (res == 0) + TAILQ_INSERT_TAIL(&evcon->requests, req, next); + + return res; + } + + TAILQ_INSERT_TAIL(&evcon->requests, req, next); /* * If it's connected already and we are the first in the queue,