Feedback-ID: i1ff147bf:Fastmail
From: =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Nathan Bossart <nathandbossart@gmail.com>,  Kyotaro Horiguchi
 <horikyota.ntt@gmail.com>,  michael@paquier.xyz,  gurjeet@singh.im,
  pgsql-hackers@postgresql.org
Subject: Re: Unprivileged user can induce crash by using an SUSET param in
 PGOPTIONS
References: 
 <CABwTF4WXCbjEpVRq+G+MZeJEXU=2evYMb_DVdzXpWap5cczkcQ@mail.gmail.com>
	<Ytn2ssEKdzPFM3Vf@paquier.xyz>
	<20220722.151600.1094104349120879204.horikyota.ntt@gmail.com>
	<729330.1658516182@sss.pgh.pa.us>
	<20220722223359.GB3998906@nathanxps13>
	<763512.1658529844@sss.pgh.pa.us>
	<20220722225615.GC3998906@nathanxps13>
	<954936.1658597004@sss.pgh.pa.us>
	<20220725034040.GA4085281@nathanxps13>
	<1834276.1658720963@sss.pgh.pa.us>
	<20220725043044.GC4085281@nathanxps13>
	<2015981.1658759562@sss.pgh.pa.us>
Date: Mon, 25 Jul 2022 16:26:32 +0100
In-Reply-To: <2015981.1658759562@sss.pgh.pa.us> (Tom Lane's message of "Mon,
	25 Jul 2022 10:32:42 -0400")
Message-ID: <87ilnli57b.fsf@wibble.ilmari.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Archived-At: 
 <https://www.postgresql.org/message-id/87ilnli57b.fsf%40wibble.ilmari.org>
Precedence: bulk

--=-=-=
Content-Type: text/plain

Tom Lane <tgl@sss.pgh.pa.us> writes:

> I wonder if it'd be a good idea to convert
> auto_explain's TAP test to load auto_explain via session_preload_libraries
> instead of shared_preload_libraries, and then pass in the settings for
> each test via PGOPTIONS instead of constantly rewriting postgresql.conf.

That whole config-file rewriting did feel a bit icky when I added more
tests recently, but I completely forgot about PGOPTIONS and -c.
Something like the attached is indeed much nicer.

- ilmari


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=0001-Use-PGOPTIONS-and-session_preload_libraries-in-auto_.patch

From c9d6850e89ff95df1034c13ca9c10aaf1cf0f02f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Mon, 25 Jul 2022 16:23:09 +0100
Subject: [PATCH] Use PGOPTIONS and session_preload_libraries in auto_explain
 tests

Instead of constantly rewriting postgresql.conf.  Also change the from
shared_preload_libraries to session_preload_libraries to givesome
coverage of extensions providing custom GUCs.
---
 contrib/auto_explain/t/001_auto_explain.pl | 24 +++-------------------
 1 file changed, 3 insertions(+), 21 deletions(-)

diff --git a/contrib/auto_explain/t/001_auto_explain.pl b/contrib/auto_explain/t/001_auto_explain.pl
index 1d952fb54d..b86761919b 100644
--- a/contrib/auto_explain/t/001_auto_explain.pl
+++ b/contrib/auto_explain/t/001_auto_explain.pl
@@ -16,38 +16,20 @@ sub query_log
 	my ($node, $sql, $params) = @_;
 	$params ||= {};
 
-	if (keys %$params)
-	{
-		for my $key (keys %$params)
-		{
-			$node->append_conf('postgresql.conf', "$key = $params->{$key}\n");
-		}
-		$node->reload;
-	}
+	local $ENV{PGOPTIONS} = join " ", map { "-c $_=$params->{$_}" } keys %$params;
 
 	my $log    = $node->logfile();
 	my $offset = -s $log;
 
 	$node->safe_psql("postgres", $sql);
 
-	my $log_contents = slurp_file($log, $offset);
-
-	if (keys %$params)
-	{
-		for my $key (keys %$params)
-		{
-			$node->adjust_conf('postgresql.conf', $key, undef);
-		}
-		$node->reload;
-	}
-
-	return $log_contents;
+	return slurp_file($log, $offset);
 }
 
 my $node = PostgreSQL::Test::Cluster->new('main');
 $node->init;
 $node->append_conf('postgresql.conf',
-	"shared_preload_libraries = 'auto_explain'");
+	"session_preload_libraries = 'auto_explain'");
 $node->append_conf('postgresql.conf', "auto_explain.log_min_duration = 0");
 $node->append_conf('postgresql.conf', "auto_explain.log_analyze = on");
 $node->start;
-- 
2.30.2


--=-=-=--