From b291fa30b08ee96e67a47a49ef5850c0a839a63c Mon Sep 17 00:00:00 2001 From: Stephan Bosch Date: Fri, 25 Mar 2016 02:48:55 +0900 Subject: [PATCH] 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. --- src/lib-http/http-client-connection.c | 31 ++++++++++++++++----------- src/lib-http/http-client-private.h | 4 ++-- src/lib-http/http-client-request.c | 21 +++++++++++------- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/lib-http/http-client-connection.c b/src/lib-http/http-client-connection.c index 3f17ac0654..f725e5452b 100644 --- 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 acdb53ab26..f34a4ceb1d 100644 --- 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 492a22887e..ce1a00d45a 100644 --- 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; -- 2.47.3