Skip to content

Commit

Permalink
http: add callback to allow server to decline (and thereby close) inc…
Browse files Browse the repository at this point in the history
…oming connections.

This is important, as otherwise clients can easily exhaust the file
descriptors available on a libevent HTTP server, which can cause
problems in other code which does not handle EMFILE well: for example,
see bitcoin/bitcoin#11368

Closes: libevent#578 (patch cherry picked)
  • Loading branch information
vii authored and azat committed Dec 18, 2017
1 parent 6e5c15d commit 727bcea
Show file tree
Hide file tree
Showing 4 changed files with 163 additions and 7 deletions.
2 changes: 2 additions & 0 deletions http-internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ struct evhttp {
void *gencbarg;
struct bufferevent* (*bevcb)(struct event_base *, void *);
void *bevcbarg;
int (*newreqcb)(struct evhttp_request *req, void *);
void *newreqcbarg;

struct event_base *base;
};
Expand Down
25 changes: 18 additions & 7 deletions http.c
Original file line number Diff line number Diff line change
Expand Up @@ -3929,6 +3929,14 @@ evhttp_set_bevcb(struct evhttp *http,
http->bevcbarg = cbarg;
}

void
evhttp_set_newreqcb(struct evhttp *http,
int (*cb)(struct evhttp_request *, void *), void *cbarg)
{
http->newreqcb = cb;
http->newreqcbarg = cbarg;
}

/*
* Request related functions
*/
Expand Down Expand Up @@ -4239,17 +4247,20 @@ evhttp_associate_new_request_with_connection(struct evhttp_connection *evcon)
req->evcon = evcon; /* the request ends up owning the connection */
req->flags |= EVHTTP_REQ_OWN_CONNECTION;

/* We did not present the request to the user user yet, so treat it as
* if the user was done with the request. This allows us to free the
* request on a persistent connection if the client drops it without
* sending a request.
/* We did not present the request to the user yet, so treat it
* as if the user was done with the request. This allows us
* to free the request on a persistent connection if the
* client drops it without sending a request.
*/
req->userdone = 1;

TAILQ_INSERT_TAIL(&evcon->requests, req, next);

req->kind = EVHTTP_REQUEST;

if (http->newreqcb && http->newreqcb(req, http->newreqcbarg) == -1) {
evhttp_request_free(req);
return (-1);
}

TAILQ_INSERT_TAIL(&evcon->requests, req, next);

evhttp_start_read_(evcon);

Expand Down
18 changes: 18 additions & 0 deletions include/event2/http.h
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,24 @@ EVENT2_EXPORT_SYMBOL
void evhttp_set_bevcb(struct evhttp *http,
struct bufferevent *(*cb)(struct event_base *, void *), void *arg);


/**
Set a callback which allows the user to note or throttle incoming requests.
The requests are not populated with HTTP level information. They
are just associated to a connection.
If the callback returns -1, the associated connection is terminated
and the request is closed.
@param http the evhttp server object for which to set the callback
@param cb the callback to invoke for incoming connections
@param arg an context argument for the callback
*/
EVENT2_EXPORT_SYMBOL
void evhttp_set_newreqcb(struct evhttp *http,
int (*cb)(struct evhttp_request*, void *), void *arg);

/**
Adds a virtual host to the http server.
Expand Down
125 changes: 125 additions & 0 deletions test/regress_http.c
Original file line number Diff line number Diff line change
Expand Up @@ -4604,6 +4604,129 @@ http_request_extra_body_test(void *arg)
evbuffer_free(body);
}

struct http_newreqcb_test_state
{
int connections_started;
int connections_noticed;
int connections_throttled;
int connections_good;
int connections_error;
int connections_done;
};

static void
http_newreqcb_test_state_check(struct http_newreqcb_test_state* state)
{
tt_int_op(state->connections_started, >=, 0);
tt_int_op(state->connections_started, >=, state->connections_noticed);
tt_int_op(state->connections_throttled, >=, state->connections_error);

tt_int_op(state->connections_done, <=, state->connections_started);
if (state->connections_good + state->connections_error == state->connections_started) {
tt_int_op(state->connections_throttled, ==, state->connections_error);
tt_int_op(state->connections_good + state->connections_error, ==, state->connections_done);
event_base_loopexit(exit_base, NULL);
}

return;
end:
tt_fail();
exit(17);
}

static void
http_request_done_newreqcb(struct evhttp_request *req, void *arg)
{
struct http_newreqcb_test_state* state = arg;
if (req && evhttp_request_get_response_code(req) == HTTP_OK) {
++state->connections_good;
evhttp_request_set_error_cb(req, NULL);
}
++state->connections_done;

http_newreqcb_test_state_check(state);
}

static void
http_request_error_newreqcb(enum evhttp_request_error err, void *arg)
{
struct http_newreqcb_test_state* state = arg;
++state->connections_error;

http_newreqcb_test_state_check(state);
}

static int
http_newreqcb(struct evhttp_request* req, void *arg)
{
struct http_newreqcb_test_state* state = arg;
++state->connections_noticed;
http_newreqcb_test_state_check(state);
if (1 == state->connections_noticed % 7) {
state->connections_throttled++;
return -1;
}
return 0;
}


static void
http_newreqcb_test(void *arg)
{
struct basic_test_data *data = arg;
ev_uint16_t port = 0;
struct evhttp *http = http_setup(&port, data->base, 0);
struct evhttp_connection *evcons[100];
struct http_newreqcb_test_state newreqcb_test_state;
unsigned n;

exit_base = data->base;
test_ok = 0;

memset(&newreqcb_test_state, 0, sizeof(newreqcb_test_state));
memset(evcons, 0, sizeof(evcons));

evhttp_set_newreqcb(http, http_newreqcb, &newreqcb_test_state);

for (n = 0; n < sizeof(evcons)/sizeof(evcons[0]); ++n) {
struct evhttp_connection* evcon = NULL;
struct evhttp_request *req = NULL;
evcons[n] = evhttp_connection_base_new(data->base, NULL, "127.0.0.1", port);
evcon = evcons[n];
evhttp_connection_set_retries(evcon, 0);

tt_assert(evcon);

req = evhttp_request_new(http_request_done_newreqcb, &newreqcb_test_state);
evhttp_add_header(evhttp_request_get_output_headers(req), "Connection", "close");
evhttp_request_set_error_cb(req, http_request_error_newreqcb);

/* We give ownership of the request to the connection */
if (evhttp_make_request(evcon, req, EVHTTP_REQ_GET, "/test") == -1) {
tt_abort_msg("Couldn't make request");
}

++newreqcb_test_state.connections_started;
http_newreqcb_test_state_check(&newreqcb_test_state);
}

event_base_dispatch(data->base);

http_newreqcb_test_state_check(&newreqcb_test_state);
tt_int_op(newreqcb_test_state.connections_throttled, >, 0);

end:
evhttp_free(http);

for (n = 0; n < sizeof(evcons)/sizeof(evcons[0]); ++n) {
if (evcons[n])
evhttp_connection_free(evcons[n]);

}

}


#define HTTP_LEGACY(name) \
{ #name, run_legacy_test_fn, TT_ISOLATED|TT_LEGACY, &legacy_setup, \
http_##name##_test }
Expand Down Expand Up @@ -4725,6 +4848,8 @@ struct testcase_t http_testcases[] = {

HTTP(request_extra_body),

HTTP(newreqcb),

#ifdef EVENT__HAVE_OPENSSL
HTTPS(basic),
HTTPS(filter_basic),
Expand Down

0 comments on commit 727bcea

Please sign in to comment.