Merge branch 'http-client-fix-expect-100-continue-v6'

This patch set fixes and covers http client and "Expect: 100-continue"
functionality (plus increase coverage under some related options, to avoid
further regressions).

* http-client-fix-expect-100-continue-v6:
  http: fix leaking of response_code_line
  http: fix "Expect: 100-continue" client side
  test/http: separate coverage for EVHTTP_CON_READ_ON_WRITE_ERROR
  test/http: cover "Expect: 100-continue" client-server interaction
  test/http: *lingering tests shouldn't have "Expect: 100-continue"

http-internal.h

@@ -190,6 +190,7 @@ enum message_read_status evhttp_parse_firstline_(struct evhttp_request *, struct
 enum message_read_status evhttp_parse_headers_(struct evhttp_request *, struct evbuffer*);
 
 void evhttp_start_read_(struct evhttp_connection *);
+void evhttp_start_write_(struct evhttp_connection *);
 
 /* response sending HTML the data in the buffer */
 void evhttp_response_code_(struct evhttp_request *, int, const char *);

http.c

@@ -580,6 +580,23 @@ evhttp_make_header_response(struct evhttp_connection *evcon,
 	}
 }
 
+enum expect { NO, CONTINUE, OTHER };
+static enum expect evhttp_have_expect(struct evhttp_request *req, int input)
+{
+	const char *expect;
+	struct evkeyvalq *h = input ? req->input_headers : req->output_headers;
+
+	if (!req->kind == EVHTTP_REQUEST || !REQ_VERSION_ATLEAST(req, 1, 1))
+		return NO;
+
+	expect = evhttp_find_header(h, "Expect");
+	if (!expect)
+		return NO;
+
+	return !evutil_ascii_strcasecmp(expect, "100-continue") ? CONTINUE : OTHER;
+}
+
+
 /** Generate all headers appropriate for sending the http request in req (or
  * the response, if we're sending a response), and write them to evcon's
  * bufferevent. Also writes all data from req->output_buffer */
@@ -605,14 +622,12 @@ evhttp_make_header(struct evhttp_connection *evcon, struct evhttp_request *req)
 	}
 	evbuffer_add(output, "\r\n", 2);
 
-	if (evbuffer_get_length(req->output_buffer) > 0) {
+	if (evhttp_have_expect(req, 0) != CONTINUE &&
+		evbuffer_get_length(req->output_buffer)) {
 		/*
 		 * For a request, we add the POST data, for a reply, this
 		 * is the regular data.
 		 */
-		/* XXX We might want to support waiting (a limited amount of
-		   time) for a continue status line from the server before
-		   sending POST/PUT message bodies. */
 		evbuffer_add_buffer(output, req->output_buffer);
 	}
 }
@@ -1173,12 +1188,15 @@ evhttp_write_connectioncb(struct evhttp_connection *evcon, void *arg)
 {
 	/* This is after writing the request to the server */
 	struct evhttp_request *req = TAILQ_FIRST(&evcon->requests);
+	struct evbuffer *output = bufferevent_get_output(evcon->bufev);
 	EVUTIL_ASSERT(req != NULL);
 
 	EVUTIL_ASSERT(evcon->state == EVCON_WRITING);
 
-	/* We need to wait until we've written all of our output data before we can continue */
+	/* We need to wait until we've written all of our output data before we can
-	if (evbuffer_get_length(bufferevent_get_output(evcon->bufev)) > 0) { return; }
+	 * continue */
+	if (evbuffer_get_length(output) > 0)
+		return;
 
 	/* We are done writing our header and are now expecting the response */
 	req->kind = EVHTTP_RESPONSE;
@@ -1649,6 +1667,8 @@ evhttp_parse_response_line(struct evhttp_request *req, char *line)
 		return (-1);
 	}
 
+	if (req->response_code_line != NULL)
+		mm_free(req->response_code_line);
 	if ((req->response_code_line = mm_strdup(readable)) == NULL) {
 		event_warn("%s: strdup", __func__);
 		return (-1);
@@ -2165,8 +2185,7 @@ evhttp_get_body(struct evhttp_connection *evcon, struct evhttp_request *req)
 		req->ntoread = -1;
 	} else {
 		if (evhttp_get_body_length(req) == -1) {
-			evhttp_connection_fail_(evcon,
+			evhttp_connection_fail_(evcon, EVREQ_HTTP_INVALID_HEADER);
-			    EVREQ_HTTP_INVALID_HEADER);
 			return;
 		}
 		if (req->kind == EVHTTP_REQUEST && req->ntoread < 1) {
@@ -2178,12 +2197,8 @@ evhttp_get_body(struct evhttp_connection *evcon, struct evhttp_request *req)
 	}
 
 	/* Should we send a 100 Continue status line? */
-	if (req->kind == EVHTTP_REQUEST && REQ_VERSION_ATLEAST(req, 1, 1)) {
+	switch (evhttp_have_expect(req, 1)) {
-		const char *expect;
+		case CONTINUE:
-
-		expect = evhttp_find_header(req->input_headers, "Expect");
-		if (expect) {
-			if (!evutil_ascii_strcasecmp(expect, "100-continue")) {
 				/* XXX It would be nice to do some sanity
 				   checking here. Does the resource exist?
 				   Should the resource accept post requests? If
@@ -2192,19 +2207,19 @@ evhttp_get_body(struct evhttp_connection *evcon, struct evhttp_request *req)
 				   send their message body. */
 				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) {
+					if ((req->evcon->max_body_size <= EV_INT64_MAX) &&
+						(ev_uint64_t)req->ntoread > req->evcon->max_body_size) {
 						evhttp_lingering_fail(evcon, req);
 						return;
 					}
 				}
 				if (!evbuffer_get_length(bufferevent_get_input(evcon->bufev)))
 					evhttp_send_continue(evcon, req);
-			} else {
+			break;
-				evhttp_send_error(req, HTTP_EXPECTATIONFAILED,
+		case OTHER:
-					NULL);
+			evhttp_send_error(req, HTTP_EXPECTATIONFAILED, NULL);
 			return;
-			}
+		case NO: break;
-		}
 	}
 
 	evhttp_read_body(evcon, req);
@@ -2272,7 +2287,9 @@ evhttp_read_header(struct evhttp_connection *evcon,
 	case EVHTTP_RESPONSE:
 		/* Start over if we got a 100 Continue response. */
 		if (req->response_code == 100) {
-			evhttp_start_read_(evcon);
+			struct evbuffer *output = bufferevent_get_output(evcon->bufev);
+			evbuffer_add_buffer(output, req->output_buffer);
+			evhttp_start_write_(evcon);
 			return;
 		}
 		if (!evhttp_response_needs_body(req)) {
@@ -2685,6 +2702,16 @@ evhttp_start_read_(struct evhttp_connection *evcon)
 	}
 }
 
+void
+evhttp_start_write_(struct evhttp_connection *evcon)
+{
+	bufferevent_disable(evcon->bufev, EV_WRITE);
+	bufferevent_enable(evcon->bufev, EV_READ);
+
+	evcon->state = EVCON_WRITING;
+	evhttp_write_buffer(evcon, evhttp_write_connectioncb, NULL);
+}
+
 static void
 evhttp_send_done(struct evhttp_connection *evcon, void *arg)
 {

test/regress_http.c

@@ -3768,7 +3768,6 @@ http_data_length_constraints_test_done(struct evhttp_request *req, void *arg)
 end:
 	event_base_loopexit(arg, NULL);
 }
-
 static void
 http_large_entity_test_done(struct evhttp_request *req, void *arg)
 {
@@ -3777,23 +3776,47 @@ http_large_entity_test_done(struct evhttp_request *req, void *arg)
 end:
 	event_base_loopexit(arg, NULL);
 }
+static void
+http_failed_request_done(struct evhttp_request *req, void *arg)
+{
+	tt_assert(!req);
+end:
+	event_base_loopexit(arg, NULL);
+}
+static void
+http_expectation_failed_done(struct evhttp_request *req, void *arg)
+{
+	tt_assert(req);
+	tt_int_op(evhttp_request_get_response_code(req), ==, HTTP_EXPECTATIONFAILED);
+end:
+	event_base_loopexit(arg, NULL);
+}
 
 static void
-http_data_length_constraints_test(void *arg)
+http_data_length_constraints_test_impl(void *arg, int read_on_write_error)
 {
 	struct basic_test_data *data = arg;
 	ev_uint16_t port = 0;
 	struct evhttp_connection *evcon = NULL;
 	struct evhttp_request *req = NULL;
 	char *long_str = NULL;
-	size_t size = (1<<20) * 3;
+	const size_t continue_size = 1<<20;
+	const size_t size = (1<<20) * 3;
+	void (*cb)(struct evhttp_request *, void *);
 
 	test_ok = 0;
+	cb = http_failed_request_done;
+	if (read_on_write_error)
+		cb = http_data_length_constraints_test_done;
 
 	http = http_setup(&port, data->base, 0);
 
+	tt_assert(continue_size < size);
+
 	evcon = evhttp_connection_base_new(data->base, NULL, "127.0.0.1", port);
 	tt_assert(evcon);
+
+	if (read_on_write_error)
 		tt_assert(!evhttp_connection_set_flags(evcon, EVHTTP_CON_READ_ON_WRITE_ERROR));
 
 	/* also bind to local host */
@@ -3830,8 +3853,10 @@ http_data_length_constraints_test(void *arg)
 	}
 	event_base_dispatch(data->base);
 
+	if (read_on_write_error)
+		cb = http_large_entity_test_done;
 	evhttp_set_max_body_size(http, size - 2);
-	req = evhttp_request_new(http_large_entity_test_done, data->base);
+	req = evhttp_request_new(cb, 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) {
@@ -3848,6 +3873,27 @@ http_data_length_constraints_test(void *arg)
 	}
 	event_base_dispatch(data->base);
 
+	req = evhttp_request_new(http_dispatcher_test_done, data->base);
+	evhttp_add_header(evhttp_request_get_output_headers(req), "Host", "somehost");
+	evhttp_add_header(evhttp_request_get_output_headers(req), "Expect", "100-continue");
+	long_str[continue_size] = '\0';
+	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);
+
+	if (read_on_write_error)
+		cb = http_expectation_failed_done;
+	req = evhttp_request_new(cb, data->base);
+	evhttp_add_header(evhttp_request_get_output_headers(req), "Host", "somehost");
+	evhttp_add_header(evhttp_request_get_output_headers(req), "Expect", "101-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)
@@ -3857,6 +3903,10 @@ http_data_length_constraints_test(void *arg)
 	if (long_str)
 		free(long_str);
 }
+static void http_data_length_constraints_test(void *arg)
+{ http_data_length_constraints_test_impl(arg, 0); }
+static void http_read_on_write_error_test(void *arg)
+{ http_data_length_constraints_test_impl(arg, 1); }
 
 static void
 http_large_entity_non_lingering_test_done(struct evhttp_request *req, void *arg)
@@ -3903,7 +3953,6 @@ http_lingering_close_test_impl(void *arg, int lingering)
 	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");
@@ -4367,6 +4416,7 @@ struct testcase_t http_testcases[] = {
 	  TT_ISOLATED|TT_OFF_BY_DEFAULT, &basic_setup, NULL },
 
 	HTTP(data_length_constraints),
+	HTTP(read_on_write_error),
 	HTTP(non_lingering_close),
 	HTTP(lingering_close),