Mercurial > dovecot > core-2.2

changeset 21438:0a2000aef1c8

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
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.
author Stephan Bosch <stephan.bosch@dovecot.fi>
date Sun, 22 Jan 2017 23:55:24 +0100
parents 7b5d7cb9100a
children 2690e7946ea2
files src/lib-http/http-client-request.c
diffstat 1 files changed, 39 insertions(+), 24 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-http/http-client-request.c	Wed Jan 18 13:46:12 2017 +0200
+++ b/src/lib-http/http-client-request.c	Sun Jan 22 23:55:24 2017 +0100
@@ -757,37 +757,52 @@
 
 	if (req->state == HTTP_REQUEST_STATE_NEW)
 		http_client_request_submit(req);
-
-	/* Wait for payload data to be written */
+	if (req->state == HTTP_REQUEST_STATE_ABORTED) {
+		/* Request already failed */
+		if (req->delayed_error != NULL) {
+			struct http_client_request *tmpreq = req;
 
-	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);
+			/* 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 */
 
-	while (req->state < HTTP_REQUEST_STATE_PAYLOAD_IN) {
-		http_client_request_debug(req, "Waiting for request to finish");
+		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");
 		
-		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:
 	case HTTP_REQUEST_STATE_FINISHED: