v2-0002-Add-test-for-early-backend-startup-errors.patch

From 649e2c89f62484940c55b02635e393bdd85654af Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Tue, 23 Jul 2024 20:20:37 +0300
Subject: [PATCH v2 2/3] Add test for early backend startup errors

The new test tests the libpq fallback behavior on an early error.

This adds an IS_INJECTION_POINT_ATTACHED() macro, to allow writing the
test code alongside the normal source code. In principle, the new test
could've been written by an extra test module with a callback that
sets the FrontendProtocol global variable, but I think it's more
straightforward to have the test code

Discussion: https://www.postgresql.org/message-id/CAOYmi%2Bnwvu21mJ4DYKUa98HdfM_KZJi7B1MhyXtnsyOO-PB6Ww%40mail.gmail.com
---
 doc/src/sgml/xfunc.sgml                       | 25 ++++++++++
 src/backend/tcop/backend_startup.c            | 16 ++++++
 src/backend/utils/misc/injection_point.c      | 15 ++++++
 src/include/utils/injection_point.h           |  3 ++
 src/interfaces/libpq/Makefile                 |  4 +-
 src/interfaces/libpq/meson.build              |  1 +
 .../libpq/t/005_negotiate_encryption.pl       | 50 +++++++++++++++++++
 7 files changed, 113 insertions(+), 1 deletion(-)

diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
index 7e92e89846..5b584a4f14 100644
--- a/doc/src/sgml/xfunc.sgml
+++ b/doc/src/sgml/xfunc.sgml
@@ -3672,6 +3672,31 @@ custom_injection_callback(const char *name, const void *private_data)
      logic.
     </para>
 
+    <para>
+     An alternative way to define the action to take when an injection point
+     is reached is to add the testing code alongside the normal source
+     code. This can be useful if the action e.g. depends on local variables
+     that are not accessible to loaded modules. The
+     <function>IS_INJECTION_POINT_ATTACHED</function> macro can then be used
+     to check if an injection point is attached, for example:
+<programlisting>
+#ifdef USE_INJECTION_POINTS
+if (IS_INJECTION_POINT_ATTACHED("before-foobar"))
+{
+    /* change a local variable if injection point is attached */
+    local_var = 123;
+
+    /* also execute the callback */
+    INJECTION_POINT_CACHED("before-foobar");
+}
+#endif
+</programlisting>
+     Note that the callback attached to the injection point will not be
+     executed by the <function>IS_INJECTION_POINT_ATTACHED</function>
+     macro. If you want to execute the callback, you must also call
+     <function>INJECTION_POINT_CACHED</function> like in the above example.
+    </para>
+
     <para>
      Optionally, it is possible to detach an injection point by calling:
 <programlisting>
diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.c
index a2f94b1050..b840d95e4d 100644
--- a/src/backend/tcop/backend_startup.c
+++ b/src/backend/tcop/backend_startup.c
@@ -33,6 +33,7 @@
 #include "tcop/backend_startup.h"
 #include "tcop/tcopprot.h"
 #include "utils/builtins.h"
+#include "utils/injection_point.h"
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
 #include "utils/timeout.h"
@@ -213,6 +214,21 @@ BackendInitialize(ClientSocket *client_sock, CAC_state cac)
 							remote_host)));
 	}
 
+	/* For testing client error handling */
+#ifdef USE_INJECTION_POINTS
+	INJECTION_POINT("backend-initialize");
+	if (IS_INJECTION_POINT_ATTACHED("backend-initialize-v2-error"))
+	{
+		/*
+		 * This simulates an early error from a pre-v14 server, which used the
+		 * version 2 protocol for any errors that occurred before processing
+		 * the startup packet.
+		 */
+		FrontendProtocol = PG_PROTOCOL(2, 0);
+		elog(FATAL, "protocol version 2 error triggered");
+	}
+#endif
+
 	/*
 	 * If we did a reverse lookup to name, we might as well save the results
 	 * rather than possibly repeating the lookup during authentication.
diff --git a/src/backend/utils/misc/injection_point.c b/src/backend/utils/misc/injection_point.c
index 8ad0c27bc8..ec23f1f4d5 100644
--- a/src/backend/utils/misc/injection_point.c
+++ b/src/backend/utils/misc/injection_point.c
@@ -23,6 +23,7 @@
 #include "miscadmin.h"
 #include "storage/fd.h"
 #include "storage/lwlock.h"
+#include "storage/proc.h"
 #include "storage/shmem.h"
 #include "utils/hsearch.h"
 #include "utils/injection_point.h"
@@ -570,3 +571,17 @@ InjectionPointCached(const char *name)
 	elog(ERROR, "Injection points are not supported by this build");
 #endif
 }
+
+/*
+ * Test if an injection point is defined.
+ */
+bool
+IsInjectionPointAttached(const char *name)
+{
+#ifdef USE_INJECTION_POINTS
+	return InjectionPointCacheRefresh(name) != NULL;
+#else
+	elog(ERROR, "Injection points are not supported by this build");
+	return false;				/* silence compiler */
+#endif
+}
diff --git a/src/include/utils/injection_point.h b/src/include/utils/injection_point.h
index a385e3df64..6be20b4857 100644
--- a/src/include/utils/injection_point.h
+++ b/src/include/utils/injection_point.h
@@ -18,10 +18,12 @@
 #define INJECTION_POINT_LOAD(name) InjectionPointLoad(name)
 #define INJECTION_POINT(name) InjectionPointRun(name)
 #define INJECTION_POINT_CACHED(name) InjectionPointCached(name)
+#define IS_INJECTION_POINT_ATTACHED(name) IsInjectionPointAttached(name)
 #else
 #define INJECTION_POINT_LOAD(name) ((void) name)
 #define INJECTION_POINT(name) ((void) name)
 #define INJECTION_POINT_CACHED(name) ((void) name)
+#define IS_INJECTION_POINT_ATTACHED(name) (false)
 #endif
 
 /*
@@ -41,6 +43,7 @@ extern void InjectionPointAttach(const char *name,
 extern void InjectionPointLoad(const char *name);
 extern void InjectionPointRun(const char *name);
 extern void InjectionPointCached(const char *name);
+extern bool IsInjectionPointAttached(const char *name);
 extern bool InjectionPointDetach(const char *name);
 
 #endif							/* INJECTION_POINT_H */
diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index b36a765764..27f8499d8a 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -9,11 +9,13 @@
 #
 #-------------------------------------------------------------------------
 
+EXTRA_INSTALL=src/test/modules/injection_points
+
 subdir = src/interfaces/libpq
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
-export with_ssl with_gssapi with_krb_srvnam
+export with_ssl with_gssapi with_krb_srvnam enable_injection_points
 
 PGFILEDESC = "PostgreSQL Access Library"
 
diff --git a/src/interfaces/libpq/meson.build b/src/interfaces/libpq/meson.build
index ed2a4048d1..7623aeadab 100644
--- a/src/interfaces/libpq/meson.build
+++ b/src/interfaces/libpq/meson.build
@@ -121,6 +121,7 @@ tests += {
       't/005_negotiate_encryption.pl',
     ],
     'env': {
+      'enable_injection_points': get_option('injection_points') ? 'yes' : 'no',
       'with_ssl': ssl_library,
       'with_gssapi': gssapi.found() ? 'yes' : 'no',
       'with_krb_srvnam': 'postgres',
diff --git a/src/interfaces/libpq/t/005_negotiate_encryption.pl b/src/interfaces/libpq/t/005_negotiate_encryption.pl
index 251c405926..e21c883ab4 100644
--- a/src/interfaces/libpq/t/005_negotiate_encryption.pl
+++ b/src/interfaces/libpq/t/005_negotiate_encryption.pl
@@ -90,6 +90,8 @@ my $kerberos_enabled =
   $ENV{PG_TEST_EXTRA} && $ENV{PG_TEST_EXTRA} =~ /\bkerberos\b/;
 my $ssl_supported = $ENV{with_ssl} eq 'openssl';
 
+my $injection_points_supported = $ENV{enable_injection_points} eq 'yes';
+
 ###
 ### Prepare test server for GSSAPI and SSL authentication, with a few
 ### different test users and helper functions. We don't actually
@@ -155,6 +157,10 @@ $node->safe_psql('postgres', 'CREATE USER ssluser;');
 $node->safe_psql('postgres', 'CREATE USER nossluser;');
 $node->safe_psql('postgres', 'CREATE USER gssuser;');
 $node->safe_psql('postgres', 'CREATE USER nogssuser;');
+if ($injection_points_supported != 0)
+{
+	$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;')
+}
 
 my $unixdir = $node->safe_psql('postgres', 'SHOW unix_socket_directories;');
 chomp($unixdir);
@@ -312,6 +318,27 @@ nossluser   .            disable      postgres       connect, authok
 		['disable'], \@all_sslmodes, \@all_sslnegotiations,
 		parse_table($test_table));
 
+	if ($injection_points_supported != 0)
+	{
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser sslmode=prefer",
+			'connect, backenderror -> fail');
+		$node->restart;
+
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize-v2-error', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser sslmode=prefer",
+			'connect, v2error -> fail');
+		$node->restart;
+	}
+
 	# Disable SSL again
 	$node->adjust_conf('postgresql.conf', 'ssl', 'off');
 	$node->reload;
@@ -393,6 +420,27 @@ nogssuser   disable      disable      postgres       connect, authok
 	test_matrix($node, [ 'testuser', 'gssuser', 'nogssuser' ],
 		\@all_gssencmodes, $sslmodes, $sslnegotiations,
 		parse_table($test_table));
+
+	if ($injection_points_supported != 0)
+	{
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser gssencmode=prefer sslmode=disable",
+			'backenderror, backenderror -> fail');
+		$node->restart;
+
+		$node->safe_psql('postgres',
+						 "SELECT injection_points_attach('backend-initialize-v2-error', 'error');",
+						 connstr => "user=localuser host=$unixdir");
+		connect_test(
+			$node,
+			"user=testuser gssencmode=prefer sslmode=disable",
+			'v2error -> fail');
+		$node->restart;
+	}
 }
 
 ###
@@ -738,6 +786,8 @@ sub parse_log_events
 		push @events, "gssreject" if $line =~ /GSSENCRequest rejected/;
 		push @events, "authfail" if $line =~ /no pg_hba.conf entry/;
 		push @events, "authok" if $line =~ /connection authenticated/;
+		push @events, "backenderror" if $line =~ /error triggered for injection point backend-/;
+		push @events, "v2error" if $line =~ /protocol version 2 error triggered/;
 	}
 
 	# No events at all is represented by "-"
-- 
2.39.2