lib-http: client: Fixed assert failure occurring when a new connectio…

…n fails for a peer that has active connections.

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

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 "

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
 };