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 92301ed0b80307380e385388635f1314dd8f8269..a79e70c717213822b233054b257618063a5978d1 100644 (file)
--- 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 572b0bd3e7c420b689710103d2a9c83cf6d51c7c..e4a6f0d3ed08f1079010c9c950a6ce59313edae6 100644 (file)
--- 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();