Mercurial > dovecot > core-2.2

changeset 21425:70083e131e22

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
lib-http: client: Make sure req->conn is only not NULL when that connection holds a reference to that request. This consolidates the management of req->conn to one place, thereby preventing mishaps. It makes sure req->conn is always properly assigned, making it more reliable. This fixes a problem that emerged in the http-proxy.
author Stephan Bosch <stephan.bosch@dovecot.fi>
date Tue, 10 Jan 2017 02:12:25 +0100
parents e94ac3f36692
children e231610ca949
files src/lib-http/http-client-connection.c src/lib-http/http-client-request.c
diffstat 2 files changed, 36 insertions(+), 27 deletions(-) [+]
line wrap: on
line diff
--- a/src/lib-http/http-client-connection.c	Thu Jan 19 02:16:06 2017 +0100
+++ b/src/lib-http/http-client-connection.c	Tue Jan 10 02:12:25 2017 +0100
@@ -49,6 +49,26 @@
 static void http_client_connection_input(struct connection *_conn);
 
 static inline void
+http_client_connection_ref_request(struct http_client_connection *conn,
+	struct http_client_request *req)
+{
+	i_assert(req->conn == NULL);
+	req->conn = conn;
+	http_client_request_ref(req);
+}
+
+static inline bool
+http_client_connection_unref_request(struct http_client_connection *conn,
+	struct http_client_request **_req)
+{
+	struct http_client_request *req = *_req;
+
+	i_assert(req->conn == conn);
+	req->conn = NULL;
+	return http_client_request_unref(_req);
+}
+
+static inline void
 http_client_connection_failure(struct http_client_connection *conn,
 					 const char *reason)
 {
@@ -106,8 +126,7 @@
 	array_foreach_modifiable(&conn->request_wait_list, req_idx) {
 		req = *req_idx;
 		/* drop reference from connection */
-		req->conn = NULL;
-		if (!http_client_request_unref(req_idx))
+		if (!http_client_connection_unref_request(conn, req_idx))
 			continue;
 		/* retry the request, which may drop it */
 		if (req->state < HTTP_REQUEST_STATE_FINISHED) {
@@ -132,8 +151,7 @@
 	array_foreach_modifiable(&conn->request_wait_list, req_idx) {
 		req = *req_idx;
 		/* drop reference from connection */
-		req->conn = NULL;
-		if (!http_client_request_unref(req_idx))
+		if (!http_client_connection_unref_request(conn, req_idx))
 			continue;
 		/* resubmit the request, which may drop it */
 		if (req->state < HTTP_REQUEST_STATE_FINISHED)
@@ -160,8 +178,7 @@
 		req = *req_idx;
 		i_assert(req->submitted);
 		/* drop reference from connection */
-		req->conn = NULL;
-		if (!http_client_request_unref(req_idx))
+		if (!http_client_connection_unref_request(conn, req_idx))
 			continue;
 		/* drop request if not already aborted */
 		http_client_request_error(&req, status, error);
@@ -180,8 +197,7 @@
 			req = *req_idx;
 			i_assert(req->submitted);
 			/* drop reference from connection */
-			req->conn = NULL;
-			if (!http_client_request_unref(req_idx))
+			if (!http_client_connection_unref_request(conn, req_idx))
 				continue;
 			/* drop request if not already aborted */
 			http_client_request_error(&req,
@@ -193,8 +209,7 @@
 	if (conn->pending_request != NULL) {
 		req = conn->pending_request;
 		/* drop reference from connection */
-		req->conn = NULL;
-		if (http_client_request_unref(&conn->pending_request)) {
+		if (http_client_connection_unref_request(conn, &conn->pending_request)) {
 			/* drop request if not already aborted */
 			http_client_request_error(&req,
 				HTTP_CLIENT_REQUEST_ERROR_ABORTED,
@@ -511,8 +526,7 @@
 
 	/* add request to wait list and add a reference */
 	array_append(&conn->request_wait_list, &req, 1);
-	req->conn = conn;
-	http_client_request_ref(req);
+	http_client_connection_ref_request(conn, req);
 
 	http_client_connection_debug(conn, "Claimed request %s",
 		http_client_request_label(req));
@@ -625,8 +639,7 @@
 	net_set_nonblock(conn->conn.fd_in, TRUE);
 
 	/* drop reference from connection */
-	req->conn = NULL;
-	if (http_client_request_unref(&conn->pending_request)) {
+	if (http_client_connection_unref_request(conn, &conn->pending_request)) {
 		/* finish request if not already aborted */
 		http_client_request_finish(req);
 	}
@@ -662,7 +675,7 @@
 	i_assert(conn->pending_request == NULL);
 
 	http_client_connection_ref(conn);
-	http_client_request_ref(req);
+	http_client_connection_ref_request(conn, req);
 	req->state = HTTP_REQUEST_STATE_GOT_RESPONSE;
 
 	if (response->payload != NULL) {
@@ -688,7 +701,7 @@
 		/* the callback managed to get this connection disconnected */
 		if (!retrying)
 			http_client_request_finish(req);
-		http_client_request_unref(&req);
+		http_client_connection_unref_request(conn, &req);
 		http_client_connection_unref(&conn);
 		return FALSE;
 	}
@@ -704,7 +717,7 @@
 					       http_client_connection_input,
 					       &conn->conn);
 		}
-		http_client_request_unref(&req);
+		http_client_connection_unref_request(conn, &req);
 		return http_client_connection_unref(&conn);
 	}
 
@@ -714,7 +727,6 @@
 		response->payload = NULL;
 
 		/* maintain request reference while payload is pending */
-		req->conn = conn;
 		conn->pending_request = req;
 
 		/* request is dereferenced in payload destroy callback */
@@ -726,7 +738,7 @@
 		}
 	} else {
 		http_client_request_finish(req);
-		http_client_request_unref(&req);
+		http_client_connection_unref_request(conn, &req);
 	}
 
 	if (conn->incoming_payload == NULL && conn->conn.input != NULL) {
@@ -909,14 +921,13 @@
 
 		/* remove request from queue */
 		array_delete(&conn->request_wait_list, 0, 1);
-		req->conn = NULL;
 		aborted = (req->state == HTTP_REQUEST_STATE_ABORTED);
 		req_ref = req;
-		if (!http_client_request_unref(&req_ref)) {
+		if (!http_client_connection_unref_request(conn, &req_ref)) {
 			i_assert(aborted);
 			req = NULL;
 		}
-		
+
 		conn->close_indicated = response.connection_close;
 
 		if (!aborted) {
@@ -1157,7 +1168,6 @@
 		if (req != NULL) {
 			struct http_response response;
 
-			http_client_request_ref(req);
 			conn->tunneling = TRUE;
 
 			i_zero(&response);
@@ -1165,7 +1175,6 @@
 			response.reason = "OK";
 
 			(void)http_client_connection_return_response(conn, req, &response);
-			http_client_request_unref(&req);
 			return;
 		} 
 		
--- a/src/lib-http/http-client-request.c	Thu Jan 19 02:16:06 2017 +0100
+++ b/src/lib-http/http-client-request.c	Tue Jan 10 02:12:25 2017 +0100
@@ -1317,7 +1317,6 @@
 	req->target = p_strdup(req->pool, target);
 	
 	req->host = NULL;
-	req->conn = NULL;
 
 	origin_url = http_url_create(&req->origin_url);
 
@@ -1374,7 +1373,6 @@
 	if (req->payload_output != NULL)
 		o_stream_unref(&req->payload_output);
 
-	req->conn = NULL;
 	req->peer = NULL;
 	req->state = HTTP_REQUEST_STATE_QUEUED;
 	http_client_host_submit_request(req->host, req);
@@ -1418,7 +1416,9 @@
 void http_client_request_start_tunnel(struct http_client_request *req,
 	struct http_client_tunnel *tunnel)
 {
+	struct http_client_connection *conn = req->conn;
+
 	i_assert(req->state == HTTP_REQUEST_STATE_GOT_RESPONSE);
 
-	http_client_connection_start_tunnel(&req->conn, tunnel);
+	http_client_connection_start_tunnel(&conn, tunnel);
 }