Do not send an HTTP error when we've already closed or responded.

Previously, we'd issue an HTTP/1.1 400 Bad Request" response on every
connection close, event if sever sent response already.

This patch changes the behavior, so we only issue the response on
close when the connection state is not DISCONNECTED, and so we set
the state to DISCONNECTED when the connection closes.

Includes a regression test; fixes sourceforge bug 2909909.
This commit is contained in:
Pavel Plesov 2010-01-11 18:52:54 -05:00 committed by Nick Mathewson
parent 8771d5b646
commit 4fd2dd9d83
2 changed files with 163 additions and 3 deletions

12
http.c
View File

@ -751,7 +751,7 @@ evhttp_connection_done(struct evhttp_connection *evcon)
*/
evhttp_connection_start_detectclose(evcon);
}
} else {
} else if (evcon->state != EVCON_DISCONNECTED) {
/*
* incoming connection - we need to leave the request on the
* connection so that we can reply to it.
@ -933,6 +933,7 @@ evhttp_read(int fd, short what, void *arg)
return;
} else if (n == 0) {
/* Connection closed */
evcon->state = EVCON_DISCONNECTED;
evhttp_connection_done(evcon);
return;
}
@ -2190,8 +2191,15 @@ evhttp_handle_request(struct evhttp_request *req, void *arg)
struct evhttp *http = arg;
struct evhttp_cb *cb = NULL;
event_debug(("%s: req->uri=%s", __func__, req->uri));
if (req->uri == NULL) {
evhttp_send_error(req, HTTP_BADREQUEST, "Bad Request");
event_debug(("%s: bad request", __func__));
if (req->evcon->state == EVCON_DISCONNECTED) {
evhttp_connection_fail(req->evcon, EVCON_HTTP_EOF);
} else {
event_debug(("%s: sending error", __func__));
evhttp_send_error(req, HTTP_BADREQUEST, "Bad Request");
}
return;
}

View File

@ -71,6 +71,7 @@ static void http_chunked_cb(struct evhttp_request *req, void *arg);
void http_post_cb(struct evhttp_request *req, void *arg);
void http_dispatcher_cb(struct evhttp_request *req, void *arg);
static void http_large_delay_cb(struct evhttp_request *req, void *arg);
static void http_badreq_cb(struct evhttp_request *req, void *arg);
static struct evhttp *
http_setup(short *pport, struct event_base *base)
@ -96,6 +97,7 @@ http_setup(short *pport, struct event_base *base)
evhttp_set_cb(myhttp, "/chunked", http_chunked_cb, NULL);
evhttp_set_cb(myhttp, "/postit", http_post_cb, NULL);
evhttp_set_cb(myhttp, "/largedelay", http_large_delay_cb, NULL);
evhttp_set_cb(myhttp, "/badrequest", http_badreq_cb, NULL);
evhttp_set_cb(myhttp, "/", http_dispatcher_cb, NULL);
*pport = port;
@ -377,6 +379,155 @@ http_basic_test(void)
fprintf(stdout, "OK\n");
}
static void
http_badreq_cb(struct evhttp_request *req, void *arg)
{
struct evbuffer *buf = evbuffer_new();
evhttp_add_header(req->output_headers, "Content-Type", "text/xml; charset=UTF-8");
evbuffer_add_printf(buf, "Hello, %s!", "127.0.0.1");
evhttp_send_reply(req, HTTP_OK, "OK", buf);
evbuffer_free(buf);
}
static void
http_badreq_errorcb(struct bufferevent *bev, short what, void *arg)
{
event_debug(("%s: called (what=%04x, arg=%p)", __func__, what, arg));
/* ignore */
}
static void
http_badreq_readcb(struct bufferevent *bev, void *arg)
{
const char *what = "Hello, 127.0.0.1";
const char *bad_request = "400 Bad Request";
event_debug(("%s: %s\n", __func__, EVBUFFER_DATA(bev->input)));
if (evbuffer_find(bev->input,
(const unsigned char *) bad_request, strlen(bad_request)) != NULL) {
event_debug(("%s: bad request detected", __func__));
test_ok = -10;
bufferevent_disable(bev, EV_READ);
event_loopexit(NULL);
return;
}
if (evbuffer_find(bev->input,
(const unsigned char*) what, strlen(what)) != NULL) {
struct evhttp_request *req = evhttp_request_new(NULL, NULL);
enum message_read_status done;
req->kind = EVHTTP_RESPONSE;
done = evhttp_parse_firstline(req, bev->input);
if (done != ALL_DATA_READ)
goto out;
done = evhttp_parse_headers(req, bev->input);
if (done != ALL_DATA_READ)
goto out;
if (done == 1 &&
evhttp_find_header(req->input_headers,
"Content-Type") != NULL)
test_ok++;
out:
evhttp_request_free(req);
evbuffer_drain(bev->input, EVBUFFER_LENGTH(bev->input));
}
shutdown(bev->ev_read.ev_fd, SHUT_WR);
}
static void
http_badreq_successcb(int fd, short what, void *arg)
{
event_debug(("%s: called (what=%04x, arg=%p)", __func__, what, arg));
event_loopexit(NULL);
}
static void
http_bad_request(void)
{
struct timeval tv;
struct bufferevent *bev;
int fd;
const char *http_request;
short port = -1;
test_ok = 0;
fprintf(stdout, "Testing \"Bad Request\" on connection close: ");
http = http_setup(&port, NULL);
/* bind to a second socket */
if (evhttp_bind_socket(http, "127.0.0.1", port + 1) == -1) {
fprintf(stdout, "FAILED (bind)\n");
exit(1);
}
/* NULL request test */
fd = http_connect("127.0.0.1", port);
/* Stupid thing to send a request */
bev = bufferevent_new(fd, http_badreq_readcb, http_writecb,
http_badreq_errorcb, NULL);
bufferevent_enable(bev, EV_READ);
/* real NULL request */
http_request = "";
shutdown(fd, SHUT_WR);
timerclear(&tv);
tv.tv_usec = 10000;
event_once(-1, EV_TIMEOUT, http_badreq_successcb, bev, &tv);
event_dispatch();
bufferevent_free(bev);
EVUTIL_CLOSESOCKET(fd);
if (test_ok != 0) {
fprintf(stdout, "FAILED\n");
exit(1);
}
/* Second answer (BAD REQUEST) on connection close */
/* connect to the second port */
fd = http_connect("127.0.0.1", port + 1);
/* Stupid thing to send a request */
bev = bufferevent_new(fd, http_badreq_readcb, http_writecb,
http_badreq_errorcb, NULL);
bufferevent_enable(bev, EV_READ);
/* first half of the http request */
http_request =
"GET /badrequest HTTP/1.0\r\n" \
"Connection: Keep-Alive\r\n" \
"\r\n";
bufferevent_write(bev, http_request, strlen(http_request));
timerclear(&tv);
tv.tv_usec = 10000;
event_once(-1, EV_TIMEOUT, http_badreq_successcb, bev, &tv);
event_dispatch();
evhttp_free(http);
if (test_ok != 2) {
fprintf(stdout, "FAILED\n");
exit(1);
}
fprintf(stdout, "OK\n");
}
static struct evhttp_connection *delayed_client;
static void
@ -1462,8 +1613,9 @@ http_suite(void)
http_basic_test();
http_connection_test(0 /* not-persistent */);
http_connection_test(1 /* persistent */);
http_close_detection(0 /* with delay */);
http_close_detection(0 /* without delay */);
http_close_detection(1 /* with delay */);
http_bad_request();
http_post_test();
http_failure_test();
http_highport_test();