lib-http: client: Fixed request timeout handling during pipelining.

The timeout was not managed correctly. If an earlier request finished, it
would not restart the timeout for the next pending request.

Also, filling the pipeline caused the timout to be reset inappropriately,
postponing its expiry.

diff --git a/src/lib-http/http-client-connection.c b/src/lib-http/http-client-connection.c
index 3f17ac06543a202c5e160bf28c082964f49a6118..f725e5452b0212830da0cf6ab24440147787e3ff 100644 (file)
--- a/src/lib-http/http-client-connection.c
+++ b/src/lib-http/http-client-connection.c
@@ -353,10 +353,13 @@ void http_client_connection_stop_request_timeout(
 static void
 http_client_connection_continue_timeout(struct http_client_connection *conn)
 {
-       struct http_client_request *const *req_idx;
+       struct http_client_request *const *wait_reqs;
        struct http_client_request *req;
+       unsigned int wait_count;
        const char *error;
 
+       i_assert(conn->pending_request == NULL);
+
        if (conn->to_response != NULL)
                timeout_remove(&conn->to_response);
        conn->peer->no_payload_sync = TRUE;
@@ -364,13 +367,12 @@ http_client_connection_continue_timeout(struct http_client_connection *conn)
        http_client_connection_debug(conn, 
                "Expected 100-continue response timed out; sending payload anyway");
 
-       i_assert(array_count(&conn->request_wait_list) > 0);
-       req_idx = array_idx(&conn->request_wait_list,
-               array_count(&conn->request_wait_list)-1);
-       req = req_idx[0];
+       wait_reqs = array_get(&conn->request_wait_list, &wait_count);
+       i_assert(wait_count == 1);
+       req = wait_reqs[wait_count-1];
 
        req->payload_sync_continue = TRUE;
-       if (http_client_request_send_more(req, &error) < 0) {
+       if (http_client_request_send_more(req, FALSE, &error) < 0) {
                http_client_connection_abort_temp_error(&conn,
                        HTTP_CLIENT_REQUEST_ERROR_CONNECTION_LOST,
                        t_strdup_printf("Failed to send request: %s", error));
@@ -381,7 +383,7 @@ int http_client_connection_next_request(struct http_client_connection *conn)
 {
        struct http_client_request *req = NULL;
        const char *error;
-       bool have_pending_requests;
+       bool pipelined;
 
        if (!http_client_connection_is_ready(conn)) {
                http_client_connection_debug(conn, "Not ready for next request");
@@ -389,9 +391,9 @@ int http_client_connection_next_request(struct http_client_connection *conn)
        }
 
        /* claim request, but no urgent request can be second in line */
-       have_pending_requests = array_count(&conn->request_wait_list) > 0 ||
+       pipelined = array_count(&conn->request_wait_list) > 0 ||
                conn->pending_request != NULL;
-       req = http_client_peer_claim_request(conn->peer, have_pending_requests);
+       req = http_client_peer_claim_request(conn->peer, pipelined);
        if (req == NULL)
                return 0;       
 
@@ -411,7 +413,7 @@ int http_client_connection_next_request(struct http_client_connection *conn)
        http_client_connection_debug(conn, "Claimed request %s",
                http_client_request_label(req));
 
-       if (http_client_request_send(req, &error) < 0) {
+       if (http_client_request_send(req, pipelined, &error) < 0) {
                http_client_connection_abort_temp_error(&conn,
                        HTTP_CLIENT_REQUEST_ERROR_CONNECTION_LOST,
                        t_strdup_printf("Failed to send request: %s", error));
@@ -431,6 +433,7 @@ int http_client_connection_next_request(struct http_client_connection *conn)
                    indefinite period before sending the message body.
         */
        if (req->payload_sync && !conn->peer->seen_100_response) {
+               i_assert(!pipelined);
                i_assert(req->payload_chunked || req->payload_size > 0);
                i_assert(conn->to_response == NULL);
                conn->to_response =     timeout_add(HTTP_CLIENT_CONTINUE_TIMEOUT_MSECS,
@@ -485,6 +488,8 @@ static void http_client_payload_finished(struct http_client_connection *conn)
        timeout_remove(&conn->to_input);
        conn->conn.io = io_add_istream(conn->conn.input,
                                       http_client_connection_input, &conn->conn);
+       if (array_count(&conn->request_wait_list) > 0)
+               http_client_connection_start_request_timeout(conn);
 }
 
 static void
@@ -762,7 +767,7 @@ static void http_client_connection_input(struct connection *_conn)
                                return;
                        }
 
-                       if (http_client_request_send_more(req, &error) < 0) {
+                       if (http_client_request_send_more(req, FALSE, &error) < 0) {
                                http_client_connection_abort_temp_error(&conn,
                                        HTTP_CLIENT_REQUEST_ERROR_CONNECTION_LOST,
                                        t_strdup_printf("Failed to send request: %s", error));
@@ -889,7 +894,6 @@ static void http_client_connection_input(struct connection *_conn)
                        payload_type = http_client_request_get_payload_type(req);
                } else {
                        /* no more requests waiting for the connection */
-                       http_client_connection_stop_request_timeout(conn);
                        req = NULL;
                        payload_type = HTTP_RESPONSE_PAYLOAD_TYPE_ALLOWED;
                }
@@ -964,6 +968,7 @@ int http_client_connection_output(struct http_client_connection *conn)
        reqs = array_get(&conn->request_wait_list, &count);
        if (count > 0 && conn->output_locked) {
                struct http_client_request *req = reqs[count-1];
+               bool pipelined = (count > 1 || conn->pending_request != NULL);
 
                if (req->state == HTTP_REQUEST_STATE_ABORTED) {
                        http_client_connection_debug(conn,
@@ -978,7 +983,7 @@ int http_client_connection_output(struct http_client_connection *conn)
                }
 
                if (!req->payload_sync || req->payload_sync_continue) {
-                       if (http_client_request_send_more(req, &error) < 0) {
+                       if (http_client_request_send_more(req, pipelined, &error) < 0) {
                                http_client_connection_abort_temp_error(&conn,
                                        HTTP_CLIENT_REQUEST_ERROR_CONNECTION_LOST,
                                        t_strdup_printf("Connection lost: %s", error));

diff --git a/src/lib-http/http-client-private.h b/src/lib-http/http-client-private.h
index acdb53ab26edafd86dc15fb4b56d268cdae6cab1..f34a4ceb1d86f46db2dfc186457ccc79fdb12d58 100644 (file)
--- a/src/lib-http/http-client-private.h
+++ b/src/lib-http/http-client-private.h
@@ -282,9 +282,9 @@ void http_client_request_get_peer_addr(const struct http_client_request *req,
 enum http_response_payload_type
 http_client_request_get_payload_type(struct http_client_request *req);
 int http_client_request_send(struct http_client_request *req,
-                           const char **error_r);
+                           bool pipelined, const char **error_r);
 int http_client_request_send_more(struct http_client_request *req,
-                                 const char **error_r);
+                                 bool pipelined, const char **error_r);
 bool http_client_request_callback(struct http_client_request *req,
        struct http_response *response);
 void http_client_request_connect_callback(struct http_client_request *req,

diff --git a/src/lib-http/http-client-request.c b/src/lib-http/http-client-request.c
index 492a22887eb91ee54f7947dac96f110998a3e1e7..ce1a00d45ac6e16e2649133c6e683ae60df824f6 100644 (file)
--- a/src/lib-http/http-client-request.c
+++ b/src/lib-http/http-client-request.c
@@ -748,7 +748,7 @@ static void http_client_request_payload_input(struct http_client_request *req)
 }
 
 int http_client_request_send_more(struct http_client_request *req,
-                                 const char **error_r)
+                                 bool pipelined, const char **error_r)
 {
        struct http_client_connection *conn = req->conn;
        struct ostream *output = req->payload_output;
@@ -804,6 +804,7 @@ int http_client_request_send_more(struct http_client_request *req,
                if (req->payload_wait) {
                        /* this chunk of input is finished
                           (client needs to act; disable timeout) */
+                       i_assert(!pipelined);
                        conn->output_locked = TRUE;
                        http_client_connection_stop_request_timeout(conn);
                        if (req->client->ioloop != NULL)
@@ -815,13 +816,15 @@ int http_client_request_send_more(struct http_client_request *req,
        } else if (i_stream_get_data_size(req->payload_input) > 0) {
                /* output is blocking (server needs to act; enable timeout) */
                conn->output_locked = TRUE;
-               http_client_connection_start_request_timeout(conn);
+               if (!pipelined)
+                       http_client_connection_start_request_timeout(conn);
                o_stream_set_flush_pending(output, TRUE);
                http_client_request_debug(req, "Partially sent payload");
        } else {
                /* input is blocking (client needs to act; disable timeout) */
                conn->output_locked = TRUE;     
-               http_client_connection_stop_request_timeout(conn);
+               if (!pipelined)
+                       http_client_connection_stop_request_timeout(conn);
                conn->io_req_payload = io_add_istream(req->payload_input,
                        http_client_request_payload_input, req);
        }
@@ -829,7 +832,7 @@ int http_client_request_send_more(struct http_client_request *req,
 }
 
 static int http_client_request_send_real(struct http_client_request *req,
-                                        const char **error_r)
+                                        bool pipelined, const char **error_r)
 {
        const struct http_client_settings *set = &req->client->set;
        struct http_client_connection *conn = req->conn;
@@ -946,7 +949,8 @@ static int http_client_request_send_real(struct http_client_request *req,
 
                if (req->payload_output != NULL) {
                        if (!req->payload_sync) {
-                               if (http_client_request_send_more(req, error_r) < 0)
+                               if (http_client_request_send_more
+                                       (req, pipelined, error_r) < 0)
                                        ret = -1;
                        } else {
                                http_client_request_debug(req, "Waiting for 100-continue");
@@ -954,7 +958,8 @@ static int http_client_request_send_real(struct http_client_request *req,
                        }
                } else {
                        req->state = HTTP_REQUEST_STATE_WAITING;
-                       http_client_connection_start_request_timeout(req->conn);
+                       if (!pipelined)
+                               http_client_connection_start_request_timeout(req->conn);
                        conn->output_locked = FALSE;
                }
                if (ret >= 0 && o_stream_flush(output) < 0) {
@@ -969,13 +974,13 @@ static int http_client_request_send_real(struct http_client_request *req,
 }
 
 int http_client_request_send(struct http_client_request *req,
-                            const char **error_r)
+                            bool pipelined, const char **error_r)
 {
        char *errstr = NULL;
        int ret;
 
        T_BEGIN {
-               ret = http_client_request_send_real(req, error_r);
+               ret = http_client_request_send_real(req, pipelined, error_r);
                if (ret < 0)
                        errstr = i_strdup(*error_r);
        } T_END;