Обсуждение: [PATCH] Add support for postgres:// URIs to PGDATABASE environment variable
The PGDATABASE is documented as behaving the same as the dbname connection
parameter but they differ in the support for postgres:// URIs: the
PGDATABASE will never be expanded even thought expand_dbname is set:
$ psql postgres://localhost/test -c 'select 1' >/dev/null # Works
$ PGDATABASE=postgres://localhost/test psql -c 'select 1' >/dev/null # Doesn't work
psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed: FATAL: database
"postgres://localhost/test"does not exist
In the second command the postgres://localhost/test string has not been
interpreted and the connection fails.
In order to make PGDATABASE and dbname behave the same this patch adds
URIs support to the environment variable. This makes it convenient for
users to pass a single connection string when environment variables are
used.
When both PGDATABASE and dbname are a connection string, the values of
dbname will override the ones of PGDATABASE, e.g.
$ PGDATABASE=postgres://localhost/test?sslmode=require psql postgres://host/db
is equivalent to
$ psql postgres://host/db?sslmode=require
I did not write tests for this patch as I am not sure where to put them
since libpq_uri_regress uses PQconninfoParse() that does not read the
environment variables.
---
src/interfaces/libpq/fe-connect.c | 52 +++++++++++++++++++++++++++----
1 file changed, 46 insertions(+), 6 deletions(-)
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index fcd3d0d9a3..64d6685ad8 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -412,7 +412,7 @@ static PQconninfoOption *conninfo_array_parse(const char *const *keywords,
const char *const *values, PQExpBuffer errorMessage,
bool use_defaults, int expand_dbname);
static bool conninfo_add_defaults(PQconninfoOption *options,
- PQExpBuffer errorMessage);
+ PQExpBuffer errorMessage, int expand_dbname);
static PQconninfoOption *conninfo_uri_parse(const char *uri,
PQExpBuffer errorMessage, bool use_defaults);
static bool conninfo_uri_parse_options(PQconninfoOption *options,
@@ -1791,7 +1791,7 @@ PQconndefaults(void)
if (connOptions != NULL)
{
/* pass NULL errorBuf to ignore errors */
- if (!conninfo_add_defaults(connOptions, NULL))
+ if (!conninfo_add_defaults(connOptions, NULL, false))
{
PQconninfoFree(connOptions);
connOptions = NULL;
@@ -6084,7 +6084,7 @@ conninfo_parse(const char *conninfo, PQExpBuffer errorMessage,
*/
if (use_defaults)
{
- if (!conninfo_add_defaults(options, errorMessage))
+ if (!conninfo_add_defaults(options, errorMessage, false))
{
PQconninfoFree(options);
return NULL;
@@ -6250,7 +6250,7 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
*/
if (use_defaults)
{
- if (!conninfo_add_defaults(options, errorMessage))
+ if (!conninfo_add_defaults(options, errorMessage, expand_dbname))
{
PQconninfoFree(options);
return NULL;
@@ -6272,7 +6272,7 @@ conninfo_array_parse(const char *const *keywords, const char *const *values,
* NULL.
*/
static bool
-conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
+conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage, int expand_dbname)
{
PQconninfoOption *option;
PQconninfoOption *sslmode_default = NULL,
@@ -6296,6 +6296,46 @@ conninfo_add_defaults(PQconninfoOption *options, PQExpBuffer errorMessage)
if (strcmp(option->keyword, "sslrootcert") == 0)
sslrootcert = option; /* save for later */
+ if (expand_dbname && strcmp(option->keyword, "dbname") == 0)
+ {
+ if ((tmp = getenv(option->envvar)) != NULL && recognized_connection_string(tmp))
+ {
+ PQconninfoOption *str_option,
+ *dbname_options = parse_connection_string(tmp, errorMessage, false);
+
+ if (dbname_options == NULL)
+ return false;
+
+ for (str_option = dbname_options; str_option->keyword != NULL; str_option++)
+ {
+ if (str_option->val != NULL)
+ {
+ int k;
+
+ for (k = 0; options[k].keyword; k++)
+ {
+ if (strcmp(options[k].keyword, str_option->keyword) == 0)
+ {
+ if (options[k].val != NULL)
+ continue;
+
+ options[k].val = strdup(str_option->val);
+ if (!options[k].val)
+ {
+ libpq_append_error(errorMessage, "out of memory");
+ PQconninfoFree(dbname_options);
+ return false;
+ }
+ break;
+ }
+ }
+ }
+ }
+ PQconninfoFree(dbname_options);
+ continue;
+ }
+ }
+
if (option->val != NULL)
continue; /* Value was in conninfo or service */
@@ -6428,7 +6468,7 @@ conninfo_uri_parse(const char *uri, PQExpBuffer errorMessage,
*/
if (use_defaults)
{
- if (!conninfo_add_defaults(options, errorMessage))
+ if (!conninfo_add_defaults(options, errorMessage, true))
{
PQconninfoFree(options);
return NULL;
--
2.39.2 (Apple Git-143)
=?UTF-8?q?R=C3=A9mi=20Lapeyre?= <remi.lapeyre@lenstra.fr> writes:
> The PGDATABASE is documented as behaving the same as the dbname connection
> parameter but they differ in the support for postgres:// URIs: the
> PGDATABASE will never be expanded even thought expand_dbname is set:
I think you have misunderstood the documentation. What you are
proposing is equivalent to saying that this should work:
$ psql -d "dbname=postgres://localhost/test"
psql: error: connection to server on socket "/tmp/.s.PGSQL.5432" failed: FATAL: database "postgres://localhost/test"
doesnot exist
That doesn't work, never has, and I think it would be a serious
compatibility break and possibly a security hazard if it did.
The argument of "dbname=" should not be subject to another round
of interpretation, and neither should the content of the PGDATABASE
environment variable.
You can do this:
$ psql -d "postgres://localhost/test"
but that's not the same thing as reinterpreting the dbname field
of what we have already determined to be a connection string.
Perhaps there is a case for inventing a new environment variable
that can do what you're suggesting. But you would have to make
a case that it's worth doing, and also define how it interacts
with all the other PGxxx environment variables. (The lack of
clarity about how that should work is an important part of why
I don't like the idea of letting dbname/PGDATABASE supply anything
but the database name.)
regards, tom lane
Re: [PATCH] Add support for postgres:// URIs to PGDATABASE environment variable
От
Rémi Lapeyre
Дата:
> Le 17 avr. 2023 à 03:25, Tom Lane <tgl@sss.pgh.pa.us> a écrit : > > You can do this: > > $ psql -d "postgres://localhost/test" > > but that's not the same thing as reinterpreting the dbname field > of what we have already determined to be a connection string. > Yes, I know see the difference, I got confused by the way the code reuses the dbname keyword to pass the connection stringand the fact that $ psql --dbname "postgres://localhost/test" works, but the dbname parameter of psql is not the same as the dbname used by libpq. > Perhaps there is a case for inventing a new environment variable > that can do what you're suggesting. But you would have to make > a case that it's worth doing, and also define how it interacts > with all the other PGxxx environment variables. I think it could be convenient to have such an environment variable but given your feedback I will just make it specificto my application rather than a part of libpq. Best, Rémi