Merge branch 'more-graceful-http-v10'/lingering close

In short: now `evhttp_set_max_body_size()` is browser-friendly.

This patch set implements lingering close (something like nginx have), this
will make web-server more graceful for currently existing web browsers, so:
- without EVHTTP_CON_LINGERING_CLOSE/EVHTTP_SERVER_LINGERING_CLOSE
  before: it will read min(max_body_size, Content-Length)
- with EVHTTP_CON_LINGERING_CLOSE/EVHTTP_SERVER_LINGERING_CLOSE:
  it will read max(max_body_size, Content-Length), and web browsers will show
  "413 Request Entity Too Large".

Also it fixes a bug on client-side with non-lingering close web-servers
(EVHTTP_CON_READ_ON_WRITE_ERROR), found during implementing web-server
lingering close.

* more-graceful-http-v10:
  test: http/lingering_close: cover EVHTTP_SERVER_LINGERING_CLOSE
  test: http/non_lingering_close: cover ~EVHTTP_SERVER_LINGERING_CLOSE
  test: http/*: update expected HTTP codes for body exceeds `max_body_size`
  http: take EVHTTP_CON_LINGERING_CLOSE into account for "Expect: 100-Continue"
  test: http/data_length_constrains: set EVHTTP_CON_READ_ON_WRITE_ERROR
  http: lingering close (like nginx have) for entity-too-large
  http: read server response even after server closed the connection
  test: increase buffer size for http/data_length_constraints to trigger EPIPE

Fixes: #321
Covered-by: http/non_lingering_close
Covered-by: http/lingering_close

http-internal.h

@@ -154,6 +154,7 @@ struct evhttp {
 
 	size_t default_max_headers_size;
 	ev_uint64_t default_max_body_size;
+	int flags;
 	const char *default_content_type;
 
 	/* Bitmask of all HTTP methods that we accept and pass to user

http.c

@@ -640,6 +640,14 @@ static int
 evhttp_connection_incoming_fail(struct evhttp_request *req,
     enum evhttp_request_error error)
 {
+	switch (error) {
+		case EVREQ_HTTP_DATA_TOO_LONG:
+			req->response_code = HTTP_ENTITYTOOLARGE;
+			break;
+		default:
+			req->response_code = HTTP_BADREQUEST;
+	}
+
 	switch (error) {
 	case EVREQ_HTTP_TIMEOUT:
 	case EVREQ_HTTP_EOF:
@@ -984,6 +992,35 @@ evhttp_read_trailer(struct evhttp_connection *evcon, struct evhttp_request *req)
 	}
 }
 
+static void
+evhttp_lingering_close(struct evhttp_connection *evcon,
+	struct evhttp_request *req)
+{
+	struct evbuffer *buf = bufferevent_get_input(evcon->bufev);
+
+	size_t n = evbuffer_get_length(buf);
+	if (n > (size_t) req->ntoread)
+		n = (size_t) req->ntoread;
+	req->ntoread -= n;
+	req->body_size += n;
+
+	event_debug(("Request body is too long, left " EV_SIZE_FMT,
+		EV_SIZE_ARG(req->ntoread)));
+
+	evbuffer_drain(buf, n);
+	if (!req->ntoread)
+		evhttp_connection_fail_(evcon, EVREQ_HTTP_DATA_TOO_LONG);
+}
+static void
+evhttp_lingering_fail(struct evhttp_connection *evcon,
+	struct evhttp_request *req)
+{
+	if (evcon->flags & EVHTTP_CON_LINGERING_CLOSE)
+		evhttp_lingering_close(evcon, req);
+	else
+		evhttp_connection_fail_(evcon, EVREQ_HTTP_DATA_TOO_LONG);
+}
+
 static void
 evhttp_read_body(struct evhttp_connection *evcon, struct evhttp_request *req)
 {
@@ -1037,9 +1074,8 @@ evhttp_read_body(struct evhttp_connection *evcon, struct evhttp_request *req)
 		(size_t)req->ntoread > req->evcon->max_body_size)) {
 		/* XXX: The above casted comparison must checked for overflow */
 		/* failed body length test */
-		event_debug(("Request body is too long"));
+
-		evhttp_connection_fail_(evcon,
+		evhttp_lingering_fail(evcon, req);
-				       EVREQ_HTTP_DATA_TOO_LONG);
 		return;
 	}
 
@@ -1055,7 +1091,7 @@ evhttp_read_body(struct evhttp_connection *evcon, struct evhttp_request *req)
 		}
 	}
 
-	if (req->ntoread == 0) {
+	if (!req->ntoread) {
 		bufferevent_disable(evcon->bufev, EV_READ);
 		/* Completed content length */
 		evhttp_connection_done(evcon);
@@ -1372,6 +1408,28 @@ evhttp_connection_cb_cleanup(struct evhttp_connection *evcon)
 	}
 }
 
+static void
+evhttp_connection_read_on_write_error(struct evhttp_connection *evcon,
+    struct evhttp_request *req)
+{
+	struct evbuffer *buf;
+
+	evcon->state = EVCON_READING_FIRSTLINE;
+	req->kind = EVHTTP_RESPONSE;
+
+	buf = bufferevent_get_output(evcon->bufev);
+	evbuffer_unfreeze(buf, 1);
+	evbuffer_drain(buf, evbuffer_get_length(buf));
+	evbuffer_freeze(buf, 1);
+
+	bufferevent_setcb(evcon->bufev,
+		evhttp_read_cb,
+		NULL, /* evhttp_write_cb */
+		evhttp_error_cb,
+		evcon);
+	evhttp_connection_start_detectclose(evcon);
+}
+
 static void
 evhttp_error_cb(struct bufferevent *bufev, short what, void *arg)
 {
@@ -1441,6 +1499,12 @@ evhttp_error_cb(struct bufferevent *bufev, short what, void *arg)
 	if (what & BEV_EVENT_TIMEOUT) {
 		evhttp_connection_fail_(evcon, EVREQ_HTTP_TIMEOUT);
 	} else if (what & (BEV_EVENT_EOF|BEV_EVENT_ERROR)) {
+		if (what & BEV_EVENT_WRITING &&
+			evcon->flags & EVHTTP_CON_READ_ON_WRITE_ERROR) {
+			evhttp_connection_read_on_write_error(evcon, req);
+			return;
+		}
+
 		evhttp_connection_fail_(evcon, EVREQ_HTTP_EOF);
 	} else if (what == BEV_EVENT_CONNECTED) {
 	} else {
@@ -2129,7 +2193,7 @@ evhttp_get_body(struct evhttp_connection *evcon, struct evhttp_request *req)
 				if (req->ntoread > 0) {
 					/* ntoread is ev_int64_t, max_body_size is ev_uint64_t */ 
 					if ((req->evcon->max_body_size <= EV_INT64_MAX) && (ev_uint64_t)req->ntoread > req->evcon->max_body_size) {
-						evhttp_send_error(req, HTTP_ENTITYTOOLARGE, NULL);
+						evhttp_lingering_fail(evcon, req);
 						return;
 					}
 				}
@@ -2344,6 +2408,7 @@ int evhttp_connection_set_flags(struct evhttp_connection *evcon,
 {
 	int avail_flags = 0;
 	avail_flags |= EVHTTP_CON_REUSE_CONNECTED_ADDR;
+	avail_flags |= EVHTTP_CON_READ_ON_WRITE_ERROR;
 
 	if (flags & ~avail_flags || flags > EVHTTP_CON_PUBLIC_FLAGS_END)
 		return 1;
@@ -3301,7 +3366,7 @@ evhttp_handle_request(struct evhttp_request *req, void *arg)
 	req->userdone = 0;
 
 	if (req->type == 0 || req->uri == NULL) {
-		evhttp_send_error(req, HTTP_BADREQUEST, NULL);
+		evhttp_send_error(req, req->response_code, NULL);
 		return;
 	}
 
@@ -3690,6 +3755,20 @@ evhttp_set_timeout_tv(struct evhttp* http, const struct timeval* tv)
 	}
 }
 
+int evhttp_set_flags(struct evhttp *http, int flags)
+{
+	int avail_flags = 0;
+	avail_flags |= EVHTTP_SERVER_LINGERING_CLOSE;
+
+	if (flags & ~avail_flags)
+		return 1;
+	http->flags &= ~avail_flags;
+
+	http->flags |= flags;
+
+	return 0;
+}
+
 void
 evhttp_set_max_headers_size(struct evhttp* http, ev_ssize_t max_headers_size)
 {
@@ -4062,6 +4141,8 @@ evhttp_get_request_connection(
 
 	evcon->max_headers_size = http->default_max_headers_size;
 	evcon->max_body_size = http->default_max_body_size;
+	if (http->flags & EVHTTP_SERVER_LINGERING_CLOSE)
+		evcon->flags |= EVHTTP_CON_LINGERING_CLOSE;
 
 	evcon->flags |= EVHTTP_CON_INCOMING;
 	evcon->state = EVCON_READING_FIRSTLINE;

include/event2/http.h

@@ -373,6 +373,19 @@ void evhttp_set_timeout(struct evhttp *http, int timeout_in_secs);
 EVENT2_EXPORT_SYMBOL
 void evhttp_set_timeout_tv(struct evhttp *http, const struct timeval* tv);
 
+/* Read all the clients body, and only after this respond with an error if the
+ * clients body exceed max_body_size */
+#define EVHTTP_SERVER_LINGERING_CLOSE	0x0001
+/**
+ * Set connection flags for HTTP server.
+ *
+ * @see EVHTTP_SERVER_*
+ * @return 0 on success, otherwise non zero (for example if flag doesn't
+ * supported).
+ */
+EVENT2_EXPORT_SYMBOL
+int evhttp_set_flags(struct evhttp *http, int flags);
+
 /* Request/Response functionality */
 
 /**
@@ -639,6 +652,12 @@ void evhttp_connection_set_family(struct evhttp_connection *evcon,
 
 /* reuse connection address on retry */
 #define EVHTTP_CON_REUSE_CONNECTED_ADDR	0x0008
+/* Try to read error, since server may already send and close
+ * connection, but if at that time we have some data to send then we
+ * can send get EPIPE and fail, while we can read that HTTP error. */
+#define EVHTTP_CON_READ_ON_WRITE_ERROR	0x0010
+/* @see EVHTTP_SERVER_LINGERING_CLOSE */
+#define EVHTTP_CON_LINGERING_CLOSE	0x0020
 /* Padding for public flags, @see EVHTTP_CON_* in http-internal.h */
 #define EVHTTP_CON_PUBLIC_FLAGS_END	0x100000
 /**

test/regress_http.c

@@ -3793,7 +3793,7 @@ http_data_length_constraints_test(void *arg)
 	ev_uint16_t port = 0;
 	struct evhttp_connection *evcon = NULL;
 	struct evhttp_request *req = NULL;
-	char long_str[8192];
+	char long_str[(1<<20) * 3];
 
 	test_ok = 0;
 
@@ -3801,6 +3801,7 @@ http_data_length_constraints_test(void *arg)
 
 	evcon = evhttp_connection_base_new(data->base, NULL, "127.0.0.1", port);
 	tt_assert(evcon);
+	tt_assert(!evhttp_connection_set_flags(evcon, EVHTTP_CON_READ_ON_WRITE_ERROR));
 
 	/* also bind to local host */
 	evhttp_connection_set_local_address(evcon, "127.0.0.1");
@@ -3813,10 +3814,10 @@ http_data_length_constraints_test(void *arg)
 	req = evhttp_request_new(http_data_length_constraints_test_done, data->base);
 	tt_assert(req);
 
-	memset(long_str, 'a', 8192);
+	memset(long_str, 'a', sizeof(long_str));
-	long_str[8191] = '\0';
+	long_str[sizeof(long_str)-1] = '\0';
 	/* Add the information that we care about */
-	evhttp_set_max_headers_size(http, 8191);
+	evhttp_set_max_headers_size(http, sizeof(long_str)-1);
 	evhttp_add_header(evhttp_request_get_output_headers(req), "Host", "somehost");
 	evhttp_add_header(evhttp_request_get_output_headers(req), "Longheader", long_str);
 
@@ -3835,8 +3836,8 @@ http_data_length_constraints_test(void *arg)
 	}
 	event_base_dispatch(data->base);
 
-	evhttp_set_max_body_size(http, 8190);
+	evhttp_set_max_body_size(http, sizeof(long_str)-2);
-	req = evhttp_request_new(http_data_length_constraints_test_done, data->base);
+	req = evhttp_request_new(http_large_entity_test_done, data->base);
 	evhttp_add_header(evhttp_request_get_output_headers(req), "Host", "somehost");
 	evbuffer_add_printf(evhttp_request_get_output_buffer(req), "%s", long_str);
 	if (evhttp_make_request(evcon, req, EVHTTP_REQ_POST, "/") == -1) {
@@ -3861,6 +3862,68 @@ http_data_length_constraints_test(void *arg)
 		evhttp_free(http);
 }
 
+static void
+http_large_entity_non_lingering_test_done(struct evhttp_request *req, void *arg)
+{
+	tt_assert(!req);
+end:
+	event_base_loopexit(arg, NULL);
+}
+static void
+http_lingering_close_test_impl(void *arg, int lingering)
+{
+	struct basic_test_data *data = arg;
+	ev_uint16_t port = 0;
+	struct evhttp_connection *evcon = NULL;
+	struct evhttp_request *req = NULL;
+	char long_str[(1<<20) * 3];
+	void (*cb)(struct evhttp_request *, void *);
+
+	test_ok = 0;
+
+	http = http_setup(&port, data->base, 0);
+	if (lingering)
+		tt_assert(!evhttp_set_flags(http, EVHTTP_SERVER_LINGERING_CLOSE));
+	evhttp_set_max_body_size(http, sizeof(long_str)/2);
+
+	evcon = evhttp_connection_base_new(data->base, NULL, "127.0.0.1", port);
+	tt_assert(evcon);
+	evhttp_connection_set_local_address(evcon, "127.0.0.1");
+
+	/*
+	 * At this point, we want to schedule an HTTP GET request
+	 * server using our make request method.
+	 */
+
+	memset(long_str, 'a', sizeof(long_str));
+	long_str[sizeof(long_str)-1] = '\0';
+
+	if (lingering)
+		cb = http_large_entity_test_done;
+	else
+		cb = http_large_entity_non_lingering_test_done;
+	req = evhttp_request_new(cb, data->base);
+	tt_assert(req);
+	evhttp_add_header(evhttp_request_get_output_headers(req), "Host", "somehost");
+	evhttp_add_header(evhttp_request_get_output_headers(req), "Expect", "100-continue");
+	evbuffer_add_printf(evhttp_request_get_output_buffer(req), "%s", long_str);
+	if (evhttp_make_request(evcon, req, EVHTTP_REQ_POST, "/") == -1) {
+		tt_abort_msg("Couldn't make request");
+	}
+	event_base_dispatch(data->base);
+
+	test_ok = 1;
+ end:
+	if (evcon)
+		evhttp_connection_free(evcon);
+	if (http)
+		evhttp_free(http);
+}
+static void http_non_lingering_close_test(void *arg)
+{ http_lingering_close_test_impl(arg, 0); }
+static void http_lingering_close_test(void *arg)
+{ http_lingering_close_test_impl(arg, 1); }
+
 /*
  * Testing client reset of server chunked connections
  */
@@ -4304,6 +4367,8 @@ struct testcase_t http_testcases[] = {
 	  TT_ISOLATED|TT_OFF_BY_DEFAULT, &basic_setup, NULL },
 
 	HTTP(data_length_constraints),
+	HTTP(non_lingering_close),
+	HTTP(lingering_close),
 
 	HTTP(ipv6_for_domain),
 	HTTP(get_addr),