Skip to content

Commit

Permalink
lib-http: client: Make sure req->conn is only not NULL when that conn…
Browse files Browse the repository at this point in the history 
…ection 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.
  • Loading branch information
@stephanbosch
stephanbosch committed Jan 19, 2017
1 parent 7945012 commit 9465a05
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 27 deletions.
57 changes: 33 additions & 24 deletions src/lib-http/http-client-connection.c
View file
Expand Up @@ -48,6 +48,26 @@ http_client_connection_debug(struct http_client_connection *conn,
static void http_client_connection_ready(struct http_client_connection *conn);
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)
Expand Down Expand Up @@ -106,8 +126,7 @@ http_client_connection_retry_requests(struct http_client_connection *conn,
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) {
Expand All @@ -132,8 +151,7 @@ http_client_connection_server_close(struct http_client_connection **_conn)
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)
Expand All @@ -160,8 +178,7 @@ http_client_connection_abort_error(struct http_client_connection **_conn,
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);
Expand All @@ -180,8 +197,7 @@ http_client_connection_abort_any_requests(struct http_client_connection *conn)
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,
Expand All @@ -193,8 +209,7 @@ http_client_connection_abort_any_requests(struct http_client_connection *conn)
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,
Expand Down Expand Up @@ -511,8 +526,7 @@ int http_client_connection_next_request(struct http_client_connection *conn)

/* 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));
Expand Down Expand Up @@ -625,8 +639,7 @@ static void http_client_payload_destroyed(struct http_client_request *req)
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);
}
Expand Down Expand Up @@ -662,7 +675,7 @@ http_client_connection_return_response(
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) {
Expand All @@ -688,7 +701,7 @@ http_client_connection_return_response(
/* 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;
}
Expand All @@ -704,7 +717,7 @@ http_client_connection_return_response(
http_client_connection_input,
&conn->conn);
}
http_client_request_unref(&req);
http_client_connection_unref_request(conn, &req);
return http_client_connection_unref(&conn);
}

Expand All @@ -714,7 +727,6 @@ http_client_connection_return_response(
response->payload = NULL;

/* maintain request reference while payload is pending */
req->conn = conn;
conn->pending_request = req;

/* request is dereferenced in payload destroy callback */
Expand All @@ -726,7 +738,7 @@ http_client_connection_return_response(
}
} 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) {
Expand Down Expand Up @@ -909,14 +921,13 @@ static void http_client_connection_input(struct connection *_conn)

/* 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) {
Expand Down Expand Up @@ -1157,15 +1168,13 @@ http_client_connection_ready(struct http_client_connection *conn)
if (req != NULL) {
struct http_response response;

http_client_request_ref(req);
conn->tunneling = TRUE;

i_zero(&response);
response.status = 200;
response.reason = "OK";

(void)http_client_connection_return_response(conn, req, &response);
http_client_request_unref(&req);
return;
}

Expand Down
6 changes: 3 additions & 3 deletions src/lib-http/http-client-request.c
View file
Expand Up @@ -1312,7 +1312,6 @@ void http_client_request_redirect(struct http_client_request *req,
req->target = p_strdup(req->pool, target);

req->host = NULL;
req->conn = NULL;

origin_url = http_url_create(&req->origin_url);

Expand Down Expand Up @@ -1369,7 +1368,6 @@ void http_client_request_resubmit(struct http_client_request *req)
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);
Expand Down Expand Up @@ -1413,7 +1411,9 @@ void http_client_request_set_destroy_callback(struct http_client_request *req,
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);
}

0 comments on commit 9465a05

Please sign in to comment.