do not fail while sending on http connections the client closed.

when sending chunked requests via multiple calls to evhttp_send_reply_chunk,
the client may close the connection before the server is done sending. this
used to cause a crash.

we introduce a new function evhttp_request_get_connection() that allows the
server to determine if the request is still associated with a connection.
If it's not, evhttp_request_free() needs to be called explicitly or the user
can call evhttp_send_reply_end() which just frees the request, too.
This commit is contained in:
Niels Provos 2010-02-03 14:34:56 -08:00
parent 20d706d044
commit 5c8b446eba
3 changed files with 168 additions and 9 deletions

View File

@ -221,7 +221,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 */
@ -252,6 +253,9 @@ void evhttp_request_set_chunked_cb(struct evhttp_request *,
/** Frees the request object and removes associated events. */
void evhttp_request_free(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);
/**
* A connection object that can be used to for making HTTP requests. The
* connection object tries to establish the connection when it is given an

55
http.c
View File

@ -606,8 +606,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:
default: /* xxx: probably should just error on default */
@ -654,11 +664,13 @@ evhttp_connection_fail(struct evhttp_connection *evcon,
cb = req->cb;
cb_arg = req->cb_arg;
/* 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);
/* xxx: maybe we should fail all requests??? */
/* reset the connection */
evhttp_connection_reset(evcon);
@ -991,7 +1003,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);
@ -1921,6 +1937,9 @@ evhttp_send(struct evhttp_request *req, struct evbuffer *databuf)
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);
@ -1958,15 +1977,20 @@ evhttp_send_reply_start(struct evhttp_request *req, int code,
void
evhttp_send_reply_chunk(struct evhttp_request *req, struct evbuffer *databuf)
{
struct evhttp_connection *evcon = req->evcon;
if (evcon == NULL)
return;
if (req->chunked) {
evbuffer_add_printf(req->evcon->output_buffer, "%x\r\n",
evbuffer_add_printf(evcon->output_buffer, "%x\r\n",
(unsigned)EVBUFFER_LENGTH(databuf));
}
evbuffer_add_buffer(req->evcon->output_buffer, databuf);
evbuffer_add_buffer(evcon->output_buffer, databuf);
if (req->chunked) {
evbuffer_add(req->evcon->output_buffer, "\r\n", 2);
evbuffer_add(evcon->output_buffer, "\r\n", 2);
}
evhttp_write_buffer(req->evcon, NULL, NULL);
evhttp_write_buffer(evcon, NULL, NULL);
}
void
@ -1974,6 +1998,14 @@ evhttp_send_reply_end(struct evhttp_request *req)
{
struct evhttp_connection *evcon = req->evcon;
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(req->evcon->output_buffer, "0\r\n\r\n", 5);
evhttp_write_buffer(req->evcon, evhttp_send_done, NULL);
@ -2513,6 +2545,13 @@ evhttp_request_free(struct evhttp_request *req)
free(req);
}
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 *))

View File

@ -1604,6 +1604,121 @@ http_negative_content_length_test(void)
fprintf(stdout, "OK\n");
}
/*
* 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(int 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);
return;
}
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;
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(bev->output, -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;
fprintf(stdout, "Testing Terminated Chunked Connection: ");
http = http_setup(&port, NULL);
evhttp_del_cb(http, "/test");
evhttp_set_cb(http, "/test", terminate_chunked_cb, &terminate_state);
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();
if (test_ok != 1) {
fprintf(stdout, "FAILED\n");
exit(1);
}
fprintf(stdout, "OK\n");
if (fd >= 0)
EVUTIL_CLOSESOCKET(fd);
if (http)
evhttp_free(http);
}
void
http_suite(void)
{
@ -1625,4 +1740,5 @@ http_suite(void)
http_negative_content_length_test();
http_chunked_test();
http_terminate_chunked_test();
}