From 4fd2dd9d83a000b6fa931130d7b7159130251282 Mon Sep 17 00:00:00 2001 From: Pavel Plesov Date: Mon, 11 Jan 2010 18:52:54 -0500 Subject: [PATCH] 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. --- http.c | 12 +++- test/regress_http.c | 154 +++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 163 insertions(+), 3 deletions(-) diff --git a/http.c b/http.c index 6d32f5dc..c7fc51ef 100644 --- a/http.c +++ b/http.c @@ -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; } diff --git a/test/regress_http.c b/test/regress_http.c index 1e2a1eb0..276f12e0 100644 --- a/test/regress_http.c +++ b/test/regress_http.c @@ -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();