lib-http: client: Fixed assert failure occurring when a new connection fails for...

Fixes: Panic: file http-client-queue.c: line 481 (http_client_queue_connection_failure): assertion failed: (queue->cur_peer == NULL)

diff --git a/src/lib-http/http-client-queue.c b/src/lib-http/http-client-queue.c
index 63c21852370c0c6b3a616d9943a6c243e5a0a9e1..e2e91897d1d544a77485a044c1d62f7974ebea57 100644 (file)
--- a/src/lib-http/http-client-queue.c
+++ b/src/lib-http/http-client-queue.c
@@ -478,8 +478,17 @@ http_client_queue_connection_failure(struct http_client_queue *queue,
        struct http_client_peer *failed_peer;
        struct http_client_peer *const *peer_idx;
 
-       i_assert(queue->cur_peer == NULL);
-       i_assert(array_count(&queue->pending_peers) > 0);
+       if (queue->cur_peer != NULL) {
+               /* The peer still has some working connections, which means that
+                  pending requests wait until they're picked up by those connections
+                  or the remaining connections fail as well. In the latter case,
+                  connecting to different peer can resolve the situation, but only
+                  if there is more than one IP. In any other case, the requests will
+                  eventually fail. In the future we could start connections to the next
+                  IP at this point already, but that is no small change. */
+               i_assert(array_count(&queue->pending_peers) == 0);
+               return;
+       }
 
        http_client_queue_debug(queue,
                "Failed to set up connection to %s%s: %s "

diff --git a/src/lib-http/test-http-client-errors.c b/src/lib-http/test-http-client-errors.c
index 8eb1adf05028ebbc94c5b015de213642c2c6fd4f..3df2da15eccf399256b36b4b826e3cce749fb3f3 100644 (file)
--- a/src/lib-http/test-http-client-errors.c
+++ b/src/lib-http/test-http-client-errors.c
@@ -2192,6 +2192,138 @@ static void test_dns_lookup_ttl(void)
        test_end();
 }
 
+/*
+ * Peer reuse failure
+ */
+
+/* server */
+
+static void
+test_peer_reuse_failure_input(struct server_connection *conn)
+{
+       static unsigned int seq = 0;
+       static const char *resp =
+               "HTTP/1.1 200 OK\r\n"
+               "\r\n";
+       o_stream_nsend_str(conn->conn.output, resp);
+       if (seq++ > 2) {
+               server_connection_deinit(&conn);
+               io_loop_stop(current_ioloop);
+       }
+}
+
+static void test_server_peer_reuse_failure(unsigned int index)
+{
+       test_server_input = test_peer_reuse_failure_input;
+       test_server_run(index);
+}
+
+/* client */
+
+struct _peer_reuse_failure {
+       struct timeout *to;
+       bool first:1;
+};
+
+static void
+test_client_peer_reuse_failure_response2(
+       const struct http_response *resp,
+       struct _peer_reuse_failure *ctx)
+{
+       if (debug)
+               i_debug("RESPONSE: %u %s", resp->status, resp->reason);
+
+       test_assert(resp->status == HTTP_CLIENT_REQUEST_ERROR_CONNECT_FAILED);
+       test_assert(resp->reason != NULL && *resp->reason != '\0');
+       i_free(ctx);
+       io_loop_stop(ioloop);
+}
+
+static void
+test_client_peer_reuse_failure_next(struct _peer_reuse_failure *ctx)
+{
+       struct http_client_request *hreq;
+
+       timeout_remove(&ctx->to);
+
+       hreq = http_client_request(http_client,
+               "GET", net_ip2addr(&bind_ip), "/peer-reuse-next.txt",
+               test_client_peer_reuse_failure_response2, ctx);
+       http_client_request_set_port(hreq, bind_ports[0]);
+       http_client_request_submit(hreq);
+}
+
+static void
+test_client_peer_reuse_failure_response1(
+       const struct http_response *resp,
+       struct _peer_reuse_failure *ctx)
+{
+       if (debug)
+               i_debug("RESPONSE: %u %s", resp->status, resp->reason);
+
+       if (ctx->first) {
+               test_assert(resp->status == 200);
+
+               ctx->first = FALSE;
+               ctx->to = timeout_add_short(500, test_client_peer_reuse_failure_next, ctx);
+       } else {
+               test_assert(resp->status == HTTP_CLIENT_REQUEST_ERROR_CONNECT_FAILED);
+       }
+
+       test_assert(resp->reason != NULL && *resp->reason != '\0');
+}
+
+static bool
+test_client_peer_reuse_failure(const struct http_client_settings *client_set)
+{
+       struct http_client_request *hreq;
+       struct _peer_reuse_failure *ctx;
+
+       ctx = i_new(struct _peer_reuse_failure, 1);
+       ctx->first = TRUE;
+
+       http_client = http_client_init(client_set);
+
+       hreq = http_client_request(http_client,
+               "GET", net_ip2addr(&bind_ip), "/peer-reuse.txt",
+               test_client_peer_reuse_failure_response1, 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), "/peer-reuse.txt",
+               test_client_peer_reuse_failure_response1, 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), "/peer-reuse.txt",
+               test_client_peer_reuse_failure_response1, ctx);
+       http_client_request_set_port(hreq, bind_ports[0]);
+       http_client_request_submit(hreq);
+
+       return TRUE;
+}
+
+/* test */
+
+static void test_peer_reuse_failure(void)
+{
+       struct http_client_settings http_client_set;
+
+       test_client_defaults(&http_client_set);
+       http_client_set.max_connect_attempts = 1;
+       http_client_set.max_idle_time_msecs = 500;
+
+       test_begin("peer reuse failure");
+       test_run_client_server(&http_client_set,
+               test_client_peer_reuse_failure,
+               test_server_peer_reuse_failure, 1,
+               NULL);
+       test_end();
+}
+
+
 /*
  * All tests
  */
@@ -2218,6 +2350,7 @@ static void (*test_functions[])(void) = {
        test_dns_timeout,
        test_dns_lookup_failure,
        test_dns_lookup_ttl,
+       test_peer_reuse_failure,
        NULL
 };