From: Stephan Bosch Date: Thu, 25 Oct 2018 08:31:07 +0000 (+0200) Subject: lib-http: client: Fix handling of servers with several alternative IP addresses. X-Git-Tag: 2.3.6~106 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=df14938d0e9fc57c60141312087ac37df30cfe01;p=thirdparty%2Fdovecot%2Fcore.git lib-http: client: Fix handling of servers with several alternative IP addresses. This also addresses an assert failure occurring with parallel clients: Panic: file http-client-queue.c: line 518 (http_client_queue_connection_failure): assertion failed: (queue->cur_peer == peer) --- diff --git a/src/lib-http/http-client-queue.c b/src/lib-http/http-client-queue.c index 0f92eaa550..683f6dbf85 100644 --- a/src/lib-http/http-client-queue.c +++ b/src/lib-http/http-client-queue.c @@ -514,8 +514,11 @@ http_client_queue_connection_failure(struct http_client_queue *queue, t_strdup_printf(" (SSL=%s)", https_name)), reason, array_count(&queue->pending_peers), num_requests); + http_client_peer_unlink_queue(peer, queue); + if (array_count(&queue->pending_peers) == 0) { - i_assert(queue->cur_peer == peer); + i_assert(queue->cur_peer == NULL || queue->cur_peer == peer); + queue->cur_peer = NULL; } else { bool found = FALSE; @@ -537,7 +540,6 @@ http_client_queue_connection_failure(struct http_client_queue *queue, if (array_count(&queue->pending_peers) > 0) { e_debug(queue->event, "Waiting for remaining pending peers."); - http_client_peer_unlink_queue(peer, queue); return; } @@ -547,7 +549,6 @@ http_client_queue_connection_failure(struct http_client_queue *queue, timeout_remove(&queue->to_connect); if (queue->addr.type == HTTP_CLIENT_PEER_ADDR_UNIX) { - http_client_peer_unlink_queue(peer, queue); http_client_queue_fail(queue, HTTP_CLIENT_REQUEST_ERROR_CONNECT_FAILED, reason); return; @@ -555,13 +556,19 @@ http_client_queue_connection_failure(struct http_client_queue *queue, } if (http_client_queue_is_last_connect_ip(queue)) { - /* all IPs failed, but retry all of them again if we have more - connect attempts left or on the next request. */ + if (array_count(&queue->pending_peers) > 0) { + /* Other connection attempts still pending */ + return; + } + + /* All IPs failed up until here and we allow no more connect + attempts, but try the next ones on the next request. */ queue->ips_connect_idx = queue->ips_connect_start_idx = (queue->ips_connect_idx + 1) % ips_count; - if (queue->cur_peer == NULL && (set->max_connect_attempts == 0 || - queue->connect_attempts >= set->max_connect_attempts)) { + if (set->max_connect_attempts == 0 || + queue->connect_attempts >= set->max_connect_attempts) { + e_debug(queue->event, "Failed to set up any connection; failing all queued requests"); if (queue->connect_attempts > 1) { @@ -572,7 +579,6 @@ http_client_queue_connection_failure(struct http_client_queue *queue, total_msecs/1000, total_msecs%1000); } queue->connect_attempts = 0; - http_client_peer_unlink_queue(peer, queue); http_client_queue_fail(queue, HTTP_CLIENT_REQUEST_ERROR_CONNECT_FAILED, reason); return; @@ -581,10 +587,8 @@ http_client_queue_connection_failure(struct http_client_queue *queue, queue->ips_connect_idx = (queue->ips_connect_idx + 1) % ips_count; } - if (http_client_queue_connection_attempt(queue) != peer) { + if (http_client_queue_connection_attempt(queue) != peer) http_client_peer_unlink_queue(peer, queue); - queue->cur_peer = NULL; - } return; } diff --git a/src/lib-http/test-http-client-errors.c b/src/lib-http/test-http-client-errors.c index 0e580071fd..6bff4a8aea 100644 --- a/src/lib-http/test-http-client-errors.c +++ b/src/lib-http/test-http-client-errors.c @@ -2930,6 +2930,182 @@ static void test_reconnect_failure(void) test_end(); } +/* + * Multi IP attempts + */ + +/* dns */ + +static void +test_multi_ip_attempts_input(struct server_connection *conn) +{ + unsigned int count = 0; + const char *line; + + if (!conn->version_sent) { + conn->version_sent = TRUE; + o_stream_nsend_str(conn->conn.output, "VERSION\tdns\t1\t0\n"); + } + + while ((line=i_stream_read_next_line(conn->conn.input)) != NULL) { + if (str_begins(line, "VERSION")) + continue; + if (debug) + i_debug("DNS REQUEST %u: %s", count, line); + + if (strcmp(line, "IP\ttest1.local") == 0) { + o_stream_nsend_str(conn->conn.output, + "0\t127.0.0.4\t127.0.0.3\t" + "127.0.0.2\t127.0.0.1\n"); + continue; + } + + o_stream_nsend_str(conn->conn.output, + "0\t10.255.255.1\t192.168.0.0\t" + "192.168.255.255\t127.0.0.1\n"); + } +} + +static void test_dns_multi_ip_attempts(void) +{ + test_server_input = test_multi_ip_attempts_input; + test_server_run(0); +} + +/* server */ + +static void +test_server_multi_ip_attempts_input(struct server_connection *conn) +{ + string_t *resp; + + resp = t_str_new(512); + str_printfa(resp, + "HTTP/1.1 200 OK\r\n" + "Connection: close\r\n" + "\r\n"); + o_stream_nsend(conn->conn.output, + str_data(resp), str_len(resp)); + server_connection_deinit(&conn); +} + +static void test_server_multi_ip_attempts(unsigned int index) +{ + test_server_input = test_server_multi_ip_attempts_input; + test_server_run(index); +} + +/* client */ + +struct _multi_ip_attempts { + unsigned int count; +}; + +static void +test_client_multi_ip_attempts_response( + const struct http_response *resp, + struct _multi_ip_attempts *ctx) +{ + if (debug) + i_debug("RESPONSE: %u %s", resp->status, resp->reason); + + test_assert(resp->status == 200); + test_assert(resp->reason != NULL && *resp->reason != '\0'); + + if (--ctx->count == 0) { + i_free(ctx); + io_loop_stop(ioloop); + } +} + +static bool +test_client_multi_ip_attempts1(const struct http_client_settings *client_set) +{ + struct http_client_request *hreq; + struct _multi_ip_attempts *ctx; + + ctx = i_new(struct _multi_ip_attempts, 1); + ctx->count = 2; + + http_client = http_client_init(client_set); + + hreq = http_client_request(http_client, + "GET", "test1.local", "/multi-ip-attempts.txt", + test_client_multi_ip_attempts_response, ctx); + http_client_request_set_port(hreq, bind_ports[0]); + http_client_request_submit(hreq); + + hreq = http_client_request(http_client, + "GET", "test1.local", "/multi-ip-attempts2.txt", + test_client_multi_ip_attempts_response, ctx); + http_client_request_set_port(hreq, bind_ports[0]); + http_client_request_submit(hreq); + + return TRUE; +} + +static bool +test_client_multi_ip_attempts2(const struct http_client_settings *client_set) +{ + struct http_client_request *hreq; + struct _multi_ip_attempts *ctx; + + ctx = i_new(struct _multi_ip_attempts, 1); + ctx->count = 2; + + http_client = http_client_init(client_set); + + hreq = http_client_request(http_client, + "GET", "test2.local", "/multi-ip-attempts.txt", + test_client_multi_ip_attempts_response, ctx); + http_client_request_set_port(hreq, bind_ports[0]); + http_client_request_submit(hreq); + + hreq = http_client_request(http_client, + "GET", "test2.local", "/multi-ip-attempts2.txt", + test_client_multi_ip_attempts_response, ctx); + http_client_request_set_port(hreq, bind_ports[0]); + http_client_request_submit(hreq); + + return TRUE; +} + +/* test */ + +static void test_multi_ip_attempts(void) +{ + struct http_client_settings http_client_set; + + test_client_defaults(&http_client_set); + http_client_set.connect_timeout_msecs = 1000; + http_client_set.request_timeout_msecs = 1000; + http_client_set.dns_client_socket_path = "./dns-test"; + http_client_set.max_connect_attempts = 4; + + test_begin("multi IP attempts (connection refused)"); + test_run_client_server(&http_client_set, + test_client_multi_ip_attempts1, + test_server_multi_ip_attempts, 1, + test_dns_multi_ip_attempts); + test_end(); + + test_begin("multi IP attempts (connect timeout)"); + test_run_client_server(&http_client_set, + test_client_multi_ip_attempts2, + test_server_multi_ip_attempts, 1, + test_dns_multi_ip_attempts); + test_end(); + + http_client_set.soft_connect_timeout_msecs = 100; + + test_begin("multi IP attempts (soft connect timeout)"); + test_run_client_server(&http_client_set, + test_client_multi_ip_attempts2, + test_server_multi_ip_attempts, 1, + test_dns_multi_ip_attempts); + test_end(); +} + /* * All tests */ @@ -2962,6 +3138,7 @@ static void (*const test_functions[])(void) = { test_dns_lookup_ttl, test_peer_reuse_failure, test_reconnect_failure, + test_multi_ip_attempts, NULL };