Date: Sat, 25 Jul 2015 03:14:08 -0400
From: Noah Misch <noah@leadboat.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Michael Paquier <michael.paquier@gmail.com>,
 PostgreSQL mailing lists <pgsql-hackers@postgresql.org>
Subject: Re: Supporting TAP tests with MSVC and Windows
Message-ID: <20150725071408.GA1442444@tornado.leadboat.com>
References: 
 <CAB7nPqTQwphkDfZP07w7yBnbFNDhW_JBAMyCFAkarE2VWg8irQ@mail.gmail.com>
 <20150403062230.GA1153652@tornado.leadboat.com>
 <CAB7nPqSKRM0TvY3qFrGVEQpqJuaNstufG6LdD_5FMe7iSD686Q@mail.gmail.com>
 <CAB7nPqTMmanGhAZC0GME_HyFTSgOqvKYu8rF3uAP6pnL+f0QNA@mail.gmail.com>
 <CAB7nPqSHObwq89SydPYWWBafXEm=6qcxskU1-OqQOR3Zf8PrCw@mail.gmail.com>
 <CAB7nPqSmLEkGvu10ttT1bpgiT_5Hsnef6+6TNfFU_r2T1OPL4A@mail.gmail.com>
 <CAB7nPqSYzUDTj25XvYuTbzMSdCNEwo4-ofmXjXuJipqdiOVrwQ@mail.gmail.com>
 <CAB7nPqRvsJ7WvgNDQk+yFZGNUbAbMi-LRCwgcWaAcfV3R7V2nw@mail.gmail.com>
 <CAB7nPqSmCHA726iMtVuZZ4t0Rd4jkqZhDf52dTv4138iDXy7aQ@mail.gmail.com>
 <55B2758E.1010205@iki.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <55B2758E.1010205@iki.fi>
User-Agent: Mutt/1.5.23 (2014-03-12)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On Fri, Jul 24, 2015 at 08:27:42PM +0300, Heikki Linnakangas wrote:
> On 06/25/2015 07:40 AM, Michael Paquier wrote:
> >Attached is v7, rebased on 0b157a0.
> 
> Thanks! I fiddled with this a bit more, to centralize more of the
> platform-dependent stuff to RewindTest.pm. Also, Windows doesn't have "cat"
> and "touch" if you haven't installed MinGW, so I replaced those calls with
> built-in perl code.

My main priority for this patch is to not reintroduce CVE-2014-0067.  The
patch is operating in a security minefield:

> --- a/src/bin/pg_ctl/t/001_start_stop.pl
> +++ b/src/bin/pg_ctl/t/001_start_stop.pl
> @@ -15,13 +15,9 @@ command_exit_is([ 'pg_ctl', 'start', '-D', "$tempdir/nonexistent" ],
>  
>  command_ok([ 'pg_ctl', 'initdb', '-D', "$tempdir/data" ], 'pg_ctl initdb');
>  command_ok(
> -	[   "$ENV{top_builddir}/src/test/regress/pg_regress", '--config-auth',
> +	[ $ENV{PG_REGRESS}, '--config-auth',
>  		"$tempdir/data" ],
>  	'configure authentication');
> -open CONF, ">>$tempdir/data/postgresql.conf";
> -print CONF "listen_addresses = ''\n";
> -print CONF "unix_socket_directories = '$tempdir_short'\n";
> -close CONF;
>  command_ok([ 'pg_ctl', 'start', '-D', "$tempdir/data", '-w' ],
>  	'pg_ctl start -w');

Since this series of tests doesn't use standard_initdb, the deleted lines
remain necessary.

> @@ -303,7 +297,6 @@ sub run_pg_rewind
>  	}
>  	else
>  	{
> -
>  		# Cannot come here normally

Unrelated change.

>  sub standard_initdb
>  {
>  	my $pgdata = shift;
>  	system_or_bail('initdb', '-D', "$pgdata", '-A' , 'trust', '-N');
> -	system_or_bail("$ENV{top_builddir}/src/test/regress/pg_regress",
> -		'--config-auth', $pgdata);
> +	system_or_bail($ENV{PG_REGRESS}, '--config-auth', $pgdata);
> +
> +	my $tempdir_short = tempdir_short;
> +
> +	open CONF, ">>$pgdata/postgresql.conf";
> +	print CONF "\n# Added by TestLib.pm)\n";
> +	if ($Config{osname} eq "MSWin32")
> +	{
> +		print CONF "listen_addresses = '127.0.0.1'\n";
> +	}
> +	else
> +	{
> +		print CONF "unix_socket_directories = '$tempdir_short'\n";

This second branch needs listen_addresses=''; it doesn't help to secure the
socket directory if the server still listens on localhost.

> +sub configure_hba_for_replication
> +{
> +	my $pgdata = shift;
> +
> +	open HBA, ">>$pgdata/pg_hba.conf";
> +	print HBA "\n# Allow replication (set up by TestLib.pm)\n";
> +	if ($Config{osname} ne "MSWin32")
> +	{
> +		print HBA "local replication all trust\n";
> +	}
> +	else
> +	{
> +		print HBA "host replication all 127.0.0.1/32 sspi include_realm=1 map=regress\n";
> +		print HBA "host replication all ::1/128 sspi include_realm=1 map=regress\n";

The second line will make the server fail to start on non-IPv6 systems.  You
shouldn't need it if the clients always connect to 127.0.0.1, not "localhost".

> +		configure_for_replication("$tempdir/pgdata");

That function name appears nowhere else.

> +sub tapcheck
> +{
> +	InstallTemp();
> +
> +	my @args = ( "prove", "--verbose", "t/*.pl");
> +
> +	$ENV{PATH} = "$tmp_installdir/bin;$ENV{PATH}";
> +	$ENV{PERL5LIB} = "$topdir/src/test/perl;$ENV{PERL5LIB}";
> +	$ENV{PG_REGRESS} = "$topdir/$Config/pg_regress/pg_regress";
> +
> +	# Find out all the existing TAP tests by simply looking for t/
> +	# in the tree.

This target shall not run the SSL suite, for the same reason check-world shall
not run it.  We could offer a distinct vcregress.pl target for TAP suites
excluded from check-world.

nm


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers