On Wed, Jul 09, 2025 at 04:31:31PM +0900, Michael Paquier wrote: > Attached is a rebased version of the rest, with the recent stanza > related to fef6da9e9c87 taken into account. 0002 still has a change > that should be in 0001: I have not really touched the structure of the > two remaining patches yet.
+ /* If service was found successfully, set servicefile option if not already set */ + if (*group_found && result == 0) + { + for (i = 0; options[i].keyword; i++) + { + if (strcmp(options[i].keyword, "servicefile") != 0) + continue; + + if (options[i].val != NULL) + break; + + options[i].val = strdup(serviceFile); + if (options[i].val == NULL) + { + libpq_append_error(errorMessage, "out of memory"); + return 3; + } + break; There was a bug here: if the new value cannot be strdup'd, we would miss the fclose() of the exit path, so this cannot return directly. It is possible to set the status to a new value instead, then break. After that, I have applied a few cosmetic tweaks here and there, and attached is what I have staged for commit, minus proper commit messages. The new TAP tests have some WIN32-specific things, and I won't be able to look at the buildfarm if I were to apply things today, so this will have to wait until the beginning of next week. The CI is happy with it, so at least we are one checkbox down. -- Michael
From 6f916278ca1d637851485820a8edbc79ff469656 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Thu, 10 Jul 2025 14:12:53 +0900 Subject: [PATCH v13 1/2] libpq: Add "servicefile" connection option --- src/interfaces/libpq/fe-connect.c | 55 +++++++++++++++++-- src/interfaces/libpq/libpq-int.h | 2 + src/interfaces/libpq/t/006_service.pl | 79 ++++++++++++++++++++++++++- doc/src/sgml/libpq.sgml | 15 ++++- 4 files changed, 144 insertions(+), 7 deletions(-) diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index 09eb79812ac6..cd2e99a3213e 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -201,6 +201,10 @@ static const internalPQconninfoOption PQconninfoOptions[] = { "Database-Service", "", 20, offsetof(struct pg_conn, pgservice)}, + {"servicefile", "PGSERVICEFILE", NULL, NULL, + "Database-Service-File", "", 64, + offsetof(struct pg_conn, pgservicefile)}, + {"user", "PGUSER", NULL, NULL, "Database-User", "", 20, offsetof(struct pg_conn, pguser)}, @@ -5062,6 +5066,7 @@ freePGconn(PGconn *conn) free(conn->dbName); free(conn->replication); free(conn->pgservice); + free(conn->pgservicefile); free(conn->pguser); if (conn->pgpass) { @@ -5914,6 +5919,7 @@ static int parseServiceInfo(PQconninfoOption *options, PQExpBuffer errorMessage) { const char *service = conninfo_getval(options, "service"); + const char *service_fname = conninfo_getval(options, "servicefile"); char serviceFile[MAXPGPATH]; char *env; bool group_found = false; @@ -5933,10 +5939,13 @@ parseServiceInfo(PQconninfoOption *options, PQExpBuffer errorMessage) return 0; /* - * Try PGSERVICEFILE if specified, else try ~/.pg_service.conf (if that - * exists). + * First, try the "servicefile" option in connection string. Then, try + * the PGSERVICEFILE environment variable. Finally, check + * ~/.pg_service.conf (if that exists). */ - if ((env = getenv("PGSERVICEFILE")) != NULL) + if (service_fname != NULL) + strlcpy(serviceFile, service_fname, sizeof(serviceFile)); + else if ((env = getenv("PGSERVICEFILE")) != NULL) strlcpy(serviceFile, env, sizeof(serviceFile)); else { @@ -6092,7 +6101,17 @@ parseServiceFile(const char *serviceFile, if (strcmp(key, "service") == 0) { libpq_append_error(errorMessage, - "nested service specifications not supported in service file \"%s\", line %d", + "nested \"service\" specifications not supported in service file \"%s\", line %d", + serviceFile, + linenr); + result = 3; + goto exit; + } + + if (strcmp(key, "servicefile") == 0) + { + libpq_append_error(errorMessage, + "nested \"servicefile\" specifications not supported in service file \"%s\", line %d", serviceFile, linenr); result = 3; @@ -6135,6 +6154,34 @@ parseServiceFile(const char *serviceFile, } exit: + + /* + * If a service has been successfully found, set the "servicefile" option + * if not already set. This matters for the cases where we use a default + * service file or a service file set with PGSERVICEFILE, where we want to + * be able track the value used. + */ + if (*group_found && result == 0) + { + for (i = 0; options[i].keyword; i++) + { + if (strcmp(options[i].keyword, "servicefile") != 0) + continue; + + /* If value is already set, nothing to do */ + if (options[i].val != NULL) + break; + + options[i].val = strdup(serviceFile); + if (options[i].val == NULL) + { + libpq_append_error(errorMessage, "out of memory"); + result = 3; + } + break; + } + } + fclose(f); return result; diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index a6cfd7f5c9d8..70c28f2ffca0 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -389,6 +389,8 @@ struct pg_conn char *dbName; /* database name */ char *replication; /* connect as the replication standby? */ char *pgservice; /* Postgres service, if any */ + char *pgservicefile; /* path to a service file containing + * service(s) */ char *pguser; /* Postgres username and password, if any */ char *pgpass; char *pgpassfile; /* path to a file containing password(s) */ diff --git a/src/interfaces/libpq/t/006_service.pl b/src/interfaces/libpq/t/006_service.pl index d896558a6cc2..797e6232b8fc 100644 --- a/src/interfaces/libpq/t/006_service.pl +++ b/src/interfaces/libpq/t/006_service.pl @@ -53,6 +53,13 @@ copy($srvfile_valid, $srvfile_nested) or die "Could not copy $srvfile_valid to $srvfile_nested: $!"; append_to_file($srvfile_nested, 'service=invalid_srv' . $newline); +# Service file with nested "servicefile" defined. +my $srvfile_nested_2 = "$td/pg_service_nested_2.conf"; +copy($srvfile_valid, $srvfile_nested_2) + or die "Could not copy $srvfile_valid to $srvfile_nested_2: $!"; +append_to_file($srvfile_nested_2, + 'servicefile=' . $srvfile_default . $newline); + # Set the fallback directory lookup of the service file to the temporary # directory of this test. PGSYSCONFDIR is used if the service file # defined in PGSERVICEFILE cannot be found, or when a service file is @@ -158,9 +165,77 @@ local $ENV{PGSERVICEFILE} = "$srvfile_empty"; $dummy_node->connect_fails( 'service=my_srv', - 'connection with nested service file', + 'connection with "service" in nested service file', expected_stderr => - qr/nested service specifications not supported in service file/); + qr/nested "service" specifications not supported in service file/); + + local $ENV{PGSERVICEFILE} = $srvfile_nested_2; + + $dummy_node->connect_fails( + 'service=my_srv', + 'connection with "servicefile" in nested service file', + expected_stderr => + qr/nested "servicefile" specifications not supported in service file/ + ); +} + +# Properly escape backslashes in the path, to ensure the generation of +# correct connection strings. +my $srvfile_win_cared = $srvfile_valid; +$srvfile_win_cared =~ s/\\/\\\\/g; + +# Checks that the "servicefile" option works as expected +{ + $dummy_node->connect_ok( + q{service=my_srv servicefile='} . $srvfile_win_cared . q{'}, + 'connection with valid servicefile in connection string', + sql => "SELECT 'connect3_1'", + expected_stdout => qr/connect3_1/); + + # Encode slashes and backslash + my $encoded_srvfile = $srvfile_valid =~ s{([\\/])}{ + $1 eq '/' ? '%2F' : '%5C' + }ger; + + # Additionally encode a colon in servicefile path of Windows + $encoded_srvfile =~ s/:/%3A/g; + + $dummy_node->connect_ok( + 'postgresql:///?service=my_srv&servicefile=' . $encoded_srvfile, + 'connection with valid servicefile in URI', + sql => "SELECT 'connect3_2'", + expected_stdout => qr/connect3_2/); + + local $ENV{PGSERVICE} = 'my_srv'; + $dummy_node->connect_ok( + q{servicefile='} . $srvfile_win_cared . q{'}, + 'connection with PGSERVICE and servicefile in connection string', + sql => "SELECT 'connect3_3'", + expected_stdout => qr/connect3_3/); + + $dummy_node->connect_ok( + 'postgresql://?servicefile=' . $encoded_srvfile, + 'connection with PGSERVICE and servicefile in URI', + sql => "SELECT 'connect3_4'", + expected_stdout => qr/connect3_4/); +} + +# Check that the "servicefile" option takes priority over the PGSERVICEFILE +# environment variable. +{ + local $ENV{PGSERVICEFILE} = 'non-existent-file.conf'; + + $dummy_node->connect_fails( + 'service=my_srv', + 'connection with invalid PGSERVICEFILE', + expected_stderr => + qr/service file "non-existent-file\.conf" not found/); + + $dummy_node->connect_ok( + q{service=my_srv servicefile='} . $srvfile_win_cared . q{'}, + 'connection with both servicefile and PGSERVICEFILE', + sql => "SELECT 'connect4_1'", + expected_stdout => qr/connect4_1/); } $node->teardown_node; diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index b2c2cf9eac83..c4c4bb35ac29 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -2320,6 +2320,16 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname </listitem> </varlistentry> + <varlistentry id="libpq-connect-servicefile" xreflabel="servicefile"> + <term><literal>servicefile</literal></term> + <listitem> + <para> + This option specifies the location of connection service file + (see <xref linkend="libpq-pgservice"/>). + </para> + </listitem> + </varlistentry> + <varlistentry id="libpq-connect-target-session-attrs" xreflabel="target_session_attrs"> <term><literal>target_session_attrs</literal></term> <listitem> @@ -9146,6 +9156,8 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) Defaults to <filename>~/.pg_service.conf</filename>, or <filename>%APPDATA%\postgresql\.pg_service.conf</filename> on Microsoft Windows. + <envar>This environment variable</envar> behaves the same as the <xref + linkend="libpq-connect-servicefile"/> connection parameter. </para> </listitem> @@ -9576,7 +9588,8 @@ myEventProc(PGEventId evtId, void *evtInfo, void *passThrough) On Microsoft Windows, it is named <filename>%APPDATA%\postgresql\.pg_service.conf</filename> (where <filename>%APPDATA%</filename> refers to the Application Data subdirectory - in the user's profile). A different file name can be specified by + in the user's profile). A different file name can be specified using the + <literal>servicefile</literal> key word in a libpq connection string or by setting the environment variable <envar>PGSERVICEFILE</envar>. The system-wide file is named <filename>pg_service.conf</filename>. By default it is sought in the <filename>etc</filename> directory -- 2.50.0
From f4a589919986268a3e177e0828798ec6bc1ce250 Mon Sep 17 00:00:00 2001 From: Michael Paquier <mich...@paquier.xyz> Date: Thu, 10 Jul 2025 14:13:09 +0900 Subject: [PATCH v13 2/2] psql: Add SERVICEFILE variable --- src/bin/psql/command.c | 7 +++++++ doc/src/sgml/ref/psql-ref.sgml | 9 +++++++++ 2 files changed, 16 insertions(+) diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 0a55901b14e1..0e00d73487c3 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -4481,6 +4481,7 @@ SyncVariables(void) char vbuf[32]; const char *server_version; char *service_name; + char *service_file; /* get stuff from connection */ pset.encoding = PQclientEncoding(pset.db); @@ -4500,6 +4501,11 @@ SyncVariables(void) if (service_name) pg_free(service_name); + service_file = get_conninfo_value("servicefile"); + SetVariable(pset.vars, "SERVICEFILE", service_file); + if (service_file) + pg_free(service_file); + /* this bit should match connection_warnings(): */ /* Try to get full text form of version, might include "devel" etc */ server_version = PQparameterStatus(pset.db, "server_version"); @@ -4529,6 +4535,7 @@ UnsyncVariables(void) { SetVariable(pset.vars, "DBNAME", NULL); SetVariable(pset.vars, "SERVICE", NULL); + SetVariable(pset.vars, "SERVICEFILE", NULL); SetVariable(pset.vars, "USER", NULL); SetVariable(pset.vars, "HOST", NULL); SetVariable(pset.vars, "PORT", NULL); diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml index 95f4cac2467e..4f7b11175c67 100644 --- a/doc/src/sgml/ref/psql-ref.sgml +++ b/doc/src/sgml/ref/psql-ref.sgml @@ -4623,6 +4623,15 @@ bar </listitem> </varlistentry> + <varlistentry id="app-psql-variables-servicefile"> + <term><varname>SERVICEFILE</varname></term> + <listitem> + <para> + The service file name, if applicable. + </para> + </listitem> + </varlistentry> + <varlistentry id="app-psql-variables-shell-error"> <term><varname>SHELL_ERROR</varname></term> <listitem> -- 2.50.0
signature.asc
Description: PGP signature