From: Stephan Bosch Date: Thu, 30 Mar 2017 20:29:13 +0000 (+0200) Subject: lib-http: client: Fixed request-specific attempt timeout. X-Git-Tag: 2.3.0.rc1~1825 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=210256b5d443a13a9ea662cd3a807b80bdeff331;p=thirdparty%2Fdovecot%2Fcore.git lib-http: client: Fixed request-specific attempt timeout. This is the timeout applied to a single request attempt. Using http_client_request_set_attempt_timeout_msecs() this can be set for a specific request. However, this was mostly ignored for requests that weren't in the process of handling response payload. Instead, the global request_timeout_msecs client setting was used. Also amended the (currently manual) test suite with tests that demonstated the problem and now verify the fix. --- diff --git a/src/lib-http/http-client-connection.c b/src/lib-http/http-client-connection.c index a09fa481fd..7431529c91 100644 --- a/src/lib-http/http-client-connection.c +++ b/src/lib-http/http-client-connection.c @@ -437,10 +437,18 @@ void http_client_connection_start_request_timeout( struct http_client_connection *conn) { unsigned int timeout_msecs = - conn->pending_request != NULL ? - conn->pending_request->attempt_timeout_msecs : conn->client->set.request_timeout_msecs; + if (conn->pending_request != NULL) + return; + + i_assert(array_is_created(&conn->request_wait_list)); + if (array_count(&conn->request_wait_list) > 0) { + struct http_client_request *const *requestp; + requestp = array_idx(&conn->request_wait_list, 0); + timeout_msecs = (*requestp)->attempt_timeout_msecs; + } + if (timeout_msecs == 0) ; else if (conn->to_requests != NULL) diff --git a/src/lib-http/test-http-client-errors.c b/src/lib-http/test-http-client-errors.c index 245192fdac..0f29bcd5e9 100644 --- a/src/lib-http/test-http-client-errors.c +++ b/src/lib-http/test-http-client-errors.c @@ -1618,14 +1618,14 @@ static void test_server_request_timed_out(unsigned int index) /* client */ -struct _request_timed_out_ctx { +struct _request_timed_out1_ctx { unsigned int count; }; static void -test_client_request_timed_out_response( +test_client_request_timed_out1_response( const struct http_response *resp, - struct _request_timed_out_ctx *ctx) + struct _request_timed_out1_ctx *ctx) { if (debug) i_debug("RESPONSE: %u %s", resp->status, resp->reason); @@ -1640,31 +1640,102 @@ test_client_request_timed_out_response( } static bool -test_client_request_timed_out(const struct http_client_settings *client_set) +test_client_request_timed_out1(const struct http_client_settings *client_set) { struct http_client_request *hreq; - struct _request_timed_out_ctx *ctx; + struct _request_timed_out1_ctx *ctx; - ctx = i_new(struct _request_timed_out_ctx, 1); + ctx = i_new(struct _request_timed_out1_ctx, 1); ctx->count = 2; http_client = http_client_init(client_set); hreq = http_client_request(http_client, - "GET", net_ip2addr(&bind_ip), "/request-timed-out.txt", - test_client_request_timed_out_response, ctx); + "GET", net_ip2addr(&bind_ip), "/request-timed-out1-1.txt", + test_client_request_timed_out1_response, ctx); http_client_request_set_port(hreq, bind_ports[0]); http_client_request_submit(hreq); hreq = http_client_request(http_client, - "GET", net_ip2addr(&bind_ip), "/request-timed-out2.txt", - test_client_request_timed_out_response, ctx); + "GET", net_ip2addr(&bind_ip), "/request-timed-out1-2.txt", + test_client_request_timed_out1_response, ctx); http_client_request_set_port(hreq, bind_ports[0]); http_client_request_submit(hreq); return TRUE; } +struct _request_timed_out2_ctx { + struct timeout *to; + unsigned int count; + unsigned int max_parallel_connections; +}; + +static void +test_client_request_timed_out2_timeout( + struct _request_timed_out2_ctx *ctx) +{ + if (ctx->to != NULL) + timeout_remove(&ctx->to); + i_debug("TIMEOUT"); +} + +static void +test_client_request_timed_out2_response( + const struct http_response *resp, + struct _request_timed_out2_ctx *ctx) +{ + if (debug) + i_debug("RESPONSE: %u %s", resp->status, resp->reason); + + test_assert(resp->status == HTTP_CLIENT_REQUEST_ERROR_TIMED_OUT); + test_assert(resp->reason != NULL && *resp->reason != '\0'); + test_assert(ctx->to != NULL); + + if (--ctx->count > 0) { + if (ctx->to != NULL && ctx->max_parallel_connections <= 1) + timeout_reset(ctx->to); + } else { + if (ctx->to != NULL) + timeout_remove(&ctx->to); + i_free(ctx); + io_loop_stop(ioloop); + } +} + +static bool +test_client_request_timed_out2(const struct http_client_settings *client_set) +{ + struct http_client_request *hreq; + struct _request_timed_out2_ctx *ctx; + + ctx = i_new(struct _request_timed_out2_ctx, 1); + ctx->count = 2; + ctx->max_parallel_connections = + client_set->max_parallel_connections; + + ctx->to = timeout_add(2000, + test_client_request_timed_out2_timeout, ctx); + + http_client = http_client_init(client_set); + + hreq = http_client_request(http_client, + "GET", net_ip2addr(&bind_ip), "/request-timed-out2-1.txt", + test_client_request_timed_out2_response, ctx); + http_client_request_set_port(hreq, bind_ports[0]); + http_client_request_set_attempt_timeout_msecs(hreq, 1000); + http_client_request_submit(hreq); + + hreq = http_client_request(http_client, + "GET", net_ip2addr(&bind_ip), "/request-timed-out2-2.txt", + test_client_request_timed_out2_response, ctx); + http_client_request_set_port(hreq, bind_ports[0]); + http_client_request_set_attempt_timeout_msecs(hreq, 1000); + http_client_request_submit(hreq); + + return TRUE; +} + /* test */ static void test_request_timed_out(void) @@ -1677,7 +1748,7 @@ static void test_request_timed_out(void) http_client_set.request_timeout_msecs = 1000; http_client_set.max_attempts = 1; test_run_client_server(&http_client_set, - test_client_request_timed_out, + test_client_request_timed_out1, test_server_request_timed_out, 1, NULL); test_end(); @@ -1686,7 +1757,7 @@ static void test_request_timed_out(void) http_client_set.request_timeout_msecs = 1000; http_client_set.max_attempts = 1; test_run_client_server(&http_client_set, - test_client_request_timed_out, + test_client_request_timed_out1, test_server_request_timed_out, 1, NULL); test_end(); @@ -1696,7 +1767,7 @@ static void test_request_timed_out(void) http_client_set.request_absolute_timeout_msecs = 2000; http_client_set.max_attempts = 3; test_run_client_server(&http_client_set, - test_client_request_timed_out, + test_client_request_timed_out1, test_server_request_timed_out, 1, NULL); test_end(); @@ -1706,7 +1777,29 @@ static void test_request_timed_out(void) http_client_set.request_absolute_timeout_msecs = 2000; http_client_set.max_attempts = 3; test_run_client_server(&http_client_set, - test_client_request_timed_out, + test_client_request_timed_out1, + test_server_request_timed_out, 1, + NULL); + test_end(); + + test_begin("request timed out: specific timeout"); + http_client_set.request_timeout_msecs = 3000; + http_client_set.request_absolute_timeout_msecs = 0; + http_client_set.max_attempts = 1; + http_client_set.max_parallel_connections = 1; + test_run_client_server(&http_client_set, + test_client_request_timed_out2, + test_server_request_timed_out, 1, + NULL); + test_end(); + + test_begin("request timed out: specific timeout (parallel)"); + http_client_set.request_timeout_msecs = 3000; + http_client_set.request_absolute_timeout_msecs = 0; + http_client_set.max_attempts = 1; + http_client_set.max_parallel_connections = 4; + test_run_client_server(&http_client_set, + test_client_request_timed_out2, test_server_request_timed_out, 1, NULL); test_end();