From bf45537f0263bdc02dea119ef2ab79599bf1a5c8 Mon Sep 17 00:00:00 2001
From: Stephan Bosch <stephan.bosch@dovecot.fi>
Date: Sun, 22 Jan 2017 23:55:24 +0100
Subject: [PATCH] lib-http: client: Fixed handling of errors occurring for
 unsubmitted requests during http_client_request_send_payload().

When http_client_request_send_payload() is executed for the first time, the request is submitted.
Errors occurring during submission don't trigger a callback immediately.
Instead, these are queued in the client and will trigger a callback when an ioloop is run with the client.
However, in http_client_request_send_payload() the ioloop is never executed when the request fails that way, meaning that
the callback was never called. Since for example SOLR assumes the callback is always called for an error in
http_client_request_send_payload(), this causes all kinds of problems.

Fixed by manually handling the delayed request errors in http_client_request_send_payload() explicitly.
---
 src/lib-http/http-client-request.c | 63 ++++++++++++++++++------------
 1 file changed, 39 insertions(+), 24 deletions(-)

diff --git a/src/lib-http/http-client-request.c b/src/lib-http/http-client-request.c
index 650dd99776..0cff311f3e 100644
--- a/src/lib-http/http-client-request.c
+++ b/src/lib-http/http-client-request.c
@@ -754,36 +754,51 @@ http_client_request_continue_payload(struct http_client_request **_req,
 
 	if (req->state == HTTP_REQUEST_STATE_NEW)
 		http_client_request_submit(req);
+	if (req->state == HTTP_REQUEST_STATE_ABORTED) {
+		/* Request already failed */
+		if (req->delayed_error != NULL) {
+			struct http_client_request *tmpreq = req;
+
+			/* Handle delayed error outside ioloop; the caller expects callbacks
+			   occurring, so there is no need for delay. Also, it is very
+			   important that any error triggers a callback before
+			   http_client_request_send_payload() finishes, since its return
+			   value is not always checked.
+			 */
+			http_client_remove_request_error(req->client, req);
+			http_client_request_error_delayed(&tmpreq);
+		}
+	} else {
+		/* Wait for payload data to be written */
 
-	/* Wait for payload data to be written */
-
-	i_assert(client->ioloop == NULL);
-	client->ioloop = io_loop_create();
-	http_client_switch_ioloop(client);
-	if (client->set.dns_client != NULL)
-		dns_client_switch_ioloop(client->set.dns_client);
+		i_assert(client->ioloop == NULL);
+		client->ioloop = io_loop_create();
+		http_client_switch_ioloop(client);
+		if (client->set.dns_client != NULL)
+			dns_client_switch_ioloop(client->set.dns_client);
 
-	while (req->state < HTTP_REQUEST_STATE_PAYLOAD_IN) {
-		http_client_request_debug(req, "Waiting for request to finish");
+		while (req->state < HTTP_REQUEST_STATE_PAYLOAD_IN) {
+			http_client_request_debug(req, "Waiting for request to finish");
 		
-		if (req->state == HTTP_REQUEST_STATE_PAYLOAD_OUT)
-			o_stream_set_flush_pending(req->payload_output, TRUE);
-		io_loop_run(client->ioloop);
+			if (req->state == HTTP_REQUEST_STATE_PAYLOAD_OUT)
+				o_stream_set_flush_pending(req->payload_output, TRUE);
+			io_loop_run(client->ioloop);
 
-		if (req->state == HTTP_REQUEST_STATE_PAYLOAD_OUT &&
-			req->payload_input->eof) {
-			i_stream_unref(&req->payload_input);
-			req->payload_input = NULL;
-			break;
+			if (req->state == HTTP_REQUEST_STATE_PAYLOAD_OUT &&
+				req->payload_input->eof) {
+				i_stream_unref(&req->payload_input);
+				req->payload_input = NULL;
+				break;
+			}
 		}
-	}
 
-	io_loop_set_current(prev_ioloop);
-	http_client_switch_ioloop(client);
-	if (client->set.dns_client != NULL)
-		dns_client_switch_ioloop(client->set.dns_client);
-	io_loop_set_current(client->ioloop);
-	io_loop_destroy(&client->ioloop);
+		io_loop_set_current(prev_ioloop);
+		http_client_switch_ioloop(client);
+		if (client->set.dns_client != NULL)
+			dns_client_switch_ioloop(client->set.dns_client);
+		io_loop_set_current(client->ioloop);
+		io_loop_destroy(&client->ioloop);
+	}
 
 	switch (req->state) {
 	case HTTP_REQUEST_STATE_PAYLOAD_IN: