From 649e2c89f62484940c55b02635e393bdd85654af Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas 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. + + 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 + IS_INJECTION_POINT_ATTACHED macro can then be used + to check if an injection point is attached, for example: + +#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 + + Note that the callback attached to the injection point will not be + executed by the IS_INJECTION_POINT_ATTACHED + macro. If you want to execute the callback, you must also call + INJECTION_POINT_CACHED like in the above example. + + Optionally, it is possible to detach an injection point by calling: 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