From 45d6789e78dc2997292a528e2538fa496485b85f Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 4 Aug 2019 13:07:12 -0400 Subject: [PATCH] Avoid picking already-bound TCP ports in kerberos and ldap test suites. src/test/kerberos and src/test/ldap need to run a private authentication server of the relevant type, for which they need a free TCP port. They were just picking a random port number in 48K-64K, which works except when something's already using the particular port. Notably, the probability of failure rises dramatically if one simply runs those tests in a tight loop, because each test cycle leaves behind a bunch of high ports that are transiently in TIME_WAIT state. To fix, split out the code that PostgresNode.pm already had for identifying a free TCP port number, so that it can be invoked to choose a port for the KDC or LDAP server. This isn't 100% bulletproof, since conceivably something else on the machine could grab the port between the time we check and the time we actually start the server. But that's a pretty short window, so in practice this should be good enough. Back-patch to v11 where these test suites were added. Patch by me, reviewed by Andrew Dunstan. Discussion: https://postgr.es/m/3397.1564872168@sss.pgh.pa.us --- src/test/kerberos/t/001_auth.pl | 2 +- src/test/ldap/t/001_auth.pl | 2 +- src/test/perl/PostgresNode.pm | 134 ++++++++++++++++++++------------ 3 files changed, 88 insertions(+), 50 deletions(-) diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl index 1be89aef4f4..d0fc45e5869 100644 --- a/src/test/kerberos/t/001_auth.pl +++ b/src/test/kerberos/t/001_auth.pl @@ -56,7 +56,7 @@ my $krb5_conf = "${TestLib::tmp_check}/krb5.conf"; my $kdc_conf = "${TestLib::tmp_check}/kdc.conf"; my $krb5_log = "${TestLib::tmp_check}/krb5libs.log"; my $kdc_log = "${TestLib::tmp_check}/krb5kdc.log"; -my $kdc_port = int(rand() * 16384) + 49152; +my $kdc_port = get_free_port(); my $kdc_datadir = "${TestLib::tmp_check}/krb5kdc"; my $kdc_pidfile = "${TestLib::tmp_check}/krb5kdc.pid"; my $keytab = "${TestLib::tmp_check}/krb5.keytab"; diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl index f601c2b04a7..3b071753fd0 100644 --- a/src/test/ldap/t/001_auth.pl +++ b/src/test/ldap/t/001_auth.pl @@ -48,7 +48,7 @@ my $slapd_pidfile = "${TestLib::tmp_check}/slapd.pid"; my $slapd_logfile = "${TestLib::tmp_check}/slapd.log"; my $ldap_conf = "${TestLib::tmp_check}/ldap.conf"; my $ldap_server = 'localhost'; -my $ldap_port = int(rand() * 16384) + 49152; +my $ldap_port = get_free_port(); my $ldaps_port = $ldap_port + 1; my $ldap_url = "ldap://$ldap_server:$ldap_port"; my $ldaps_url = "ldaps://$ldap_server:$ldaps_port"; diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm index c222b4fc915..32825414f8e 100644 --- a/src/test/perl/PostgresNode.pm +++ b/src/test/perl/PostgresNode.pm @@ -63,6 +63,9 @@ PostgresNode - class representing PostgreSQL server instance # Stop the server $node->stop('fast'); + # Find a free, unprivileged TCP port to bind some other service to + my $port = get_free_port(); + =head1 DESCRIPTION PostgresNode contains a set of routines able to work on a PostgreSQL node, @@ -102,6 +105,7 @@ use Scalar::Util qw(blessed); our @EXPORT = qw( get_new_node + get_free_port ); our ($use_tcp, $test_localhost, $test_pghost, $last_host_assigned, @@ -1022,54 +1026,21 @@ sub get_new_node my $class = 'PostgresNode'; $class = shift if scalar(@_) % 2 != 1; my ($name, %params) = @_; - my $port_is_forced = defined $params{port}; - my $found = $port_is_forced; - my $port = $port_is_forced ? $params{port} : $last_port_assigned; - while ($found == 0) + # Select a port. + my $port; + if (defined $params{port}) { - - # advance $port, wrapping correctly around range end - $port = 49152 if ++$port >= 65536; - print "# Checking port $port\n"; - - # Check first that candidate port number is not included in - # the list of already-registered nodes. - $found = 1; - foreach my $node (@all_nodes) - { - $found = 0 if ($node->port == $port); - } - - # Check to see if anything else is listening on this TCP port. This - # is *necessary* on $use_tcp (Windows) configurations. Seek a port - # available for all possible listen_addresses values, for own_host - # nodes and so the caller can harness this port for the widest range - # of purposes. The 0.0.0.0 test achieves that for post-2006 Cygwin, - # which automatically sets SO_EXCLUSIVEADDRUSE. The same holds for - # MSYS (a Cygwin fork). Testing 0.0.0.0 is insufficient for Windows - # native Perl (https://stackoverflow.com/a/14388707), so we also test - # individual addresses. - # - # This seems like a good idea on Unixen as well, even though we don't - # ask the postmaster to open a TCP port on Unix. On Non-Linux, - # non-Windows kernels, binding to 127.0.0.1/24 addresses other than - # 127.0.0.1 might fail with EADDRNOTAVAIL. Binding to 0.0.0.0 is - # unnecessary on non-Windows systems. - # - # XXX A port available now may become unavailable by the time we start - # the postmaster. - if ($found == 1) - { - foreach my $addr (qw(127.0.0.1), - $use_tcp ? qw(127.0.0.2 127.0.0.3 0.0.0.0) : ()) - { - can_bind($addr, $port) or $found = 0; - } - } + $port = $params{port}; + } + else + { + # When selecting a port, we look for an unassigned TCP port number, + # even if we intend to use only Unix-domain sockets. This is clearly + # necessary on $use_tcp (Windows) configurations, and it seems like a + # good idea on Unixen as well. + $port = get_free_port(); } - - print "# Found port $port\n"; # Select a host. my $host = $test_pghost; @@ -1094,12 +1065,79 @@ sub get_new_node # Add node to list of nodes push(@all_nodes, $node); - # And update port for next time - $port_is_forced or $last_port_assigned = $port; - return $node; } +=pod + +=item get_free_port() + +Locate an unprivileged (high) TCP port that's not currently bound to +anything. This is used by get_new_node, and is also exported for use +by test cases that need to start other, non-Postgres servers. + +Ports assigned to existing PostgresNode objects are automatically +excluded, even if those servers are not currently running. + +XXX A port available now may become unavailable by the time we start +the desired service. + +=cut + +sub get_free_port +{ + my $found = 0; + my $port = $last_port_assigned; + + while ($found == 0) + { + + # advance $port, wrapping correctly around range end + $port = 49152 if ++$port >= 65536; + print "# Checking port $port\n"; + + # Check first that candidate port number is not included in + # the list of already-registered nodes. + $found = 1; + foreach my $node (@all_nodes) + { + $found = 0 if ($node->port == $port); + } + + # Check to see if anything else is listening on this TCP port. + # Seek a port available for all possible listen_addresses values, + # so callers can harness this port for the widest range of purposes. + # The 0.0.0.0 test achieves that for post-2006 Cygwin, which + # automatically sets SO_EXCLUSIVEADDRUSE. The same holds for MSYS (a + # Cygwin fork). Testing 0.0.0.0 is insufficient for Windows native + # Perl (https://stackoverflow.com/a/14388707), so we also test + # individual addresses. + # + # On non-Linux, non-Windows kernels, binding to 127.0.0/24 addresses + # other than 127.0.0.1 might fail with EADDRNOTAVAIL. Binding to + # 0.0.0.0 is unnecessary on non-Windows systems. + if ($found == 1) + { + foreach my $addr (qw(127.0.0.1), + $use_tcp ? qw(127.0.0.2 127.0.0.3 0.0.0.0) : ()) + { + if (!can_bind($addr, $port)) + { + $found = 0; + last; + } + } + } + } + + print "# Found port $port\n"; + + # Update port for next time + $last_port_assigned = $port; + + return $port; +} + # Internal routine to check whether a host:port is available to bind sub can_bind {