Greg Smith <g...@2ndquadrant.com> writes:
Thank you for the review, Greg!
> Given that, there really isn't a useful path forward that helps out
> all those developers without supporting both prefixes. That's where
> this left off before, I just wanted to emphasize how clear that need
> seems now.
OK, I've used the code from your earlier review to support the short
prefix. I sincerely hope we don't make the situation any worse by being
flexible about the prefix...
> Next thing, also mentioned at that Flask page. SQLite has
> standardized the idea that sqlite:////absolute/path/to/foo.db is a URI
> pointing to a file. Given that, I wonder if Alex's syntax for
> specifying a socket file name might adopt that syntax, rather than
> requiring the hex encoding: postgresql://%2Fvar%2Fpgsql%2Ftmp/mydb
> It's not a big deal, but it would smooth another rough edge toward
> making the Postgres URI implementation look as close as possible to
> others.
Yeah, this is really appealing, however how do you tell if the part
after the last slash is a socket directory name or a dbname? E.g:
psql postgres:///path/to/different/socket/dir (default dbname)
psql postgres:///path/to/different/socket/dir/other (dbname=other ?)
If we treat the whole URI string as the path to the socket dir (which I
find the most intuitive way to do it,) the only way to specify a
non-default dbname is to use query parameter:
psql postgres:///path/to/different/socket/dir?dbname=other
or pass another -d flag to psql *after* the URI:
psql [-d] postgres:///path/to/different/socket/dir -d other
Reasonable?
> So far I've found only one syntax that I expected this to handle that
> it rejects:
>
> psql -d postgresql://gsmith@localhost
>
> It's picky about needing that third slash, but that shouldn't be hard
> to fix.
Yeah, good that you've spotted it. If my reading of the URI RFC (2396)
is correct, the question mark and query parameters may follow the
hostname, w/o that slash too, like this:
psql -d postgresql://localhost?user=gsmith
So this made me relax some checks and rewrite the code a bit.
> I started collecting up all the variants that do work as an initial
> shell script regression test, so that changes don't break something
> that already works. Here are all the variations that already work,
> setup so that a series of "1" outputs is passing:
>
[snip]
Yes, the original code was just a bit too picky about URI component
separators. Attached also is a simplified test shell script.
I have also added a warning message for when a query parameter is not
recognized and being ignored. Not sure if plain fprintf to stderr is
accepted practice for libpq, please correct if you have better idea.
--
Regards,
Alex
*** a/src/interfaces/libpq/fe-connect.c
--- b/src/interfaces/libpq/fe-connect.c
***************
*** 282,287 **** static const PQEnvironmentOption EnvironmentOptions[] =
--- 282,290 ----
}
};
+ /* The connection URI must start with one the following designators: */
+ static const char uri_designator[] = "postgresql://";
+ static const char short_uri_designator[] = "postgres://";
static bool connectOptions1(PGconn *conn, const char *conninfo);
static bool connectOptions2(PGconn *conn);
***************
*** 297,303 **** static PQconninfoOption *conninfo_parse(const char *conninfo,
static PQconninfoOption *conninfo_array_parse(const char *const * keywords,
const char *const * values, PQExpBuffer errorMessage,
bool use_defaults, int expand_dbname);
! static char *conninfo_getval(PQconninfoOption *connOptions,
const char *keyword);
static void defaultNoticeReceiver(void *arg, const PGresult *res);
static void defaultNoticeProcessor(void *arg, const char *message);
--- 300,325 ----
static PQconninfoOption *conninfo_array_parse(const char *const * keywords,
const char *const * values, PQExpBuffer errorMessage,
bool use_defaults, int expand_dbname);
! static PQconninfoOption *conninfo_uri_parse(const char *uri,
! PQExpBuffer errorMessage);
! static bool conninfo_uri_parse_options(PQconninfoOption *options,
! const char *uri, char *buf,
! PQExpBuffer errorMessage);
! static bool conninfo_uri_parse_params(char *params,
! PQconninfoOption *connOptions,
! PQExpBuffer errorMessage);
! static char *conninfo_uri_decode(const char *str, PQExpBuffer errorMessage);
! static bool get_hexdigit(char digit, int *value);
! static const char *conninfo_getval(PQconninfoOption *connOptions,
! const char *keyword);
! static PQconninfoOption *conninfo_storeval(PQconninfoOption *connOptions,
! const char *keyword, const char *value,
! PQExpBuffer errorMessage, bool ignoreMissing);
! static PQconninfoOption *conninfo_store_uri_encoded_value(
! PQconninfoOption *connOptions,
! const char *keyword, const char *encoded_value,
! PQExpBuffer errorMessage, bool ignoreMissing);
! static PQconninfoOption *conninfo_find(PQconninfoOption *connOptions,
const char *keyword);
static void defaultNoticeReceiver(void *arg, const PGresult *res);
static void defaultNoticeProcessor(void *arg, const char *message);
***************
*** 580,586 **** PQconnectStart(const char *conninfo)
static void
fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
{
! char *tmp;
/*
* Move option values into conn structure
--- 602,608 ----
static void
fillPGconn(PGconn *conn, PQconninfoOption *connOptions)
{
! const char *tmp;
/*
* Move option values into conn structure
***************
*** 3739,3745 **** ldapServiceLookup(const char *purl, PQconninfoOption *options,
static int
parseServiceInfo(PQconninfoOption *options, PQExpBuffer errorMessage)
{
! char *service = conninfo_getval(options, "service");
char serviceFile[MAXPGPATH];
char *env;
bool group_found = false;
--- 3761,3767 ----
static int
parseServiceInfo(PQconninfoOption *options, PQExpBuffer errorMessage)
{
! const char *service = conninfo_getval(options, "service");
char serviceFile[MAXPGPATH];
char *env;
bool group_found = false;
***************
*** 4129,4161 **** conninfo_parse(const char *conninfo, PQExpBuffer errorMessage,
}
/*
! * Now we have the name and the value. Search for the param record.
*/
! for (option = options; option->keyword != NULL; option++)
! {
! if (strcmp(option->keyword, pname) == 0)
! break;
! }
! if (option->keyword == NULL)
! {
! printfPQExpBuffer(errorMessage,
! libpq_gettext("invalid connection option \"%s\"\n"),
! pname);
! PQconninfoFree(options);
! free(buf);
! return NULL;
! }
!
! /*
! * Store the value
! */
! if (option->val)
! free(option->val);
! option->val = strdup(pval);
! if (!option->val)
{
- printfPQExpBuffer(errorMessage,
- libpq_gettext("out of memory\n"));
PQconninfoFree(options);
free(buf);
return NULL;
--- 4151,4160 ----
}
/*
! * Now that we have the name and the value, store the record.
*/
! if (!conninfo_storeval(options, pname, pval, errorMessage, false))
{
PQconninfoFree(options);
free(buf);
return NULL;
***************
*** 4276,4283 **** conninfo_array_parse(const char *const * keywords, const char *const * values,
/* first find "dbname" if any */
if (strcmp(pname, "dbname") == 0)
{
/* next look for "=" in the value */
! if (pvalue && strchr(pvalue, '='))
{
/*
* Must be a conninfo string, so parse it, but do not use
--- 4275,4297 ----
/* first find "dbname" if any */
if (strcmp(pname, "dbname") == 0)
{
+ /*
+ * Look for URI prefix. This has to come before the check for "=",
+ * since URI might as well contain "=" if extra parameters are
+ * given.
+ */
+ if (pvalue && (
+ (strncmp(pvalue, uri_designator,
+ sizeof(uri_designator) - 1) == 0) ||
+ (strncmp(pvalue, short_uri_designator,
+ sizeof(short_uri_designator) - 1) == 0)))
+ {
+ str_options = conninfo_uri_parse(pvalue, errorMessage);
+ if (str_options == NULL)
+ return NULL;
+ }
/* next look for "=" in the value */
! else if (pvalue && strchr(pvalue, '='))
{
/*
* Must be a conninfo string, so parse it, but do not use
***************
*** 4452,4467 **** conninfo_array_parse(const char *const * keywords, const char *const * values,
return options;
}
static char *
conninfo_getval(PQconninfoOption *connOptions,
const char *keyword)
{
PQconninfoOption *option;
for (option = connOptions; option->keyword != NULL; option++)
{
if (strcmp(option->keyword, keyword) == 0)
! return option->val;
}
return NULL;
--- 4466,4955 ----
return options;
}
+ /*
+ * Connection URI parser routine
+ *
+ * If successful, a malloc'd PQconninfoOption array is returned.
+ * If not successful, NULL is returned and an error message is
+ * left in errorMessage.
+ *
+ * This is only a wrapper for conninfo_uri_parse_options, which does all of the
+ * heavy lifting, while this function handles proper (de-)allocation of memory
+ * buffers.
+ */
+ static PQconninfoOption *
+ conninfo_uri_parse(const char *uri, PQExpBuffer errorMessage)
+ {
+ PQconninfoOption *options;
+ char *buf;
+
+ resetPQExpBuffer(errorMessage);
+
+ /* Make a working copy of PQconninfoOptions */
+ options = malloc(sizeof(PQconninfoOptions));
+ if (options == NULL)
+ {
+ printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
+ return NULL;
+ }
+ memcpy(options, PQconninfoOptions, sizeof(PQconninfoOptions));
+
+ /* Make a writable copy of the URI buffer */
+ buf = strdup(uri);
+ if (buf == NULL)
+ {
+ printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
+ free(options);
+ return NULL;
+ }
+
+ if (!conninfo_uri_parse_options(options, uri, buf, errorMessage))
+ {
+ free(options);
+ options = NULL;
+ }
+
+ free(buf);
+ return options;
+ }
+
+ /*
+ * Connection URI parser: actual parser routine
+ *
+ * If successful, returns true while the options array is filled with parsed
+ * options from the passed URI
+ * If not successful, returns false and fills errorMessage accordingly.
+ *
+ * Parses the connection URI string in 'buf' (while destructively modifying
+ * it,) according to the URI syntax below. The 'uri' parameter is only used
+ * for error reporting and 'buf' should initially contain a writable copy of
+ * 'uri'.
+ *
+ * The general form for connection URI is the following:
+ *
+ * postgresql://user:pw@host:port/database
+ *
+ * To specify a IPv6 host address, enclose the address in square brackets:
+ *
+ * postgresql://[::1]/database
+ *
+ * Connection parameters may follow the base URI using this syntax:
+ *
+ * postgresql://host/database?param1=value1¶m2=value2&...
+ */
+ static bool
+ conninfo_uri_parse_options(PQconninfoOption *options, const char* uri,
+ char *buf, PQExpBuffer errorMessage)
+ {
+ const char *host;
+
+ char *start = buf;
+ char *p;
+ char lastc;
+
+ /* Assume URI prefix is already verified by the caller */
+ if (strncmp(start, uri_designator, sizeof(uri_designator) - 1) == 0)
+ start += sizeof(uri_designator) - 1;
+ else
+ start += sizeof(short_uri_designator) - 1;
+ p = start;
+
+ /* Look ahead for possible user credentials designator */
+ while (*p && *p != '@' && *p != '/')
+ ++p;
+ if (*p != '@')
+ {
+ /* Reset to start of URI and parse as hostname/addr instead */
+ p = start;
+ }
+ else
+ {
+ char *user = start;
+
+ p = user;
+ while (*p != ':' && *p != '@')
+ ++p;
+
+ /* Save last char and cut off at end of user name */
+ lastc = *p;
+ *p = '\0';
+
+ if (!conninfo_store_uri_encoded_value(options, "user", user,
+ errorMessage, false))
+ return false;
+
+ if (lastc == ':')
+ {
+ const char *password = p + 1;
+
+ while (*p != '@')
+ ++p;
+ *p = '\0';
+
+ if (!conninfo_store_uri_encoded_value(options, "password", password,
+ errorMessage, false))
+ return false;
+ }
+
+ /* Advance past end of parsed user name or password token */
+ ++p;
+ }
+
+ /* Look for IPv6 address */
+ if (*p == '[')
+ {
+ host = ++p;
+ while (*p && *p != ']')
+ ++p;
+ if (!*p)
+ {
+ printfPQExpBuffer(errorMessage,
+ libpq_gettext("end of string reached when looking for matching ']' in IPv6 host address in URI: %s\n"),
+ uri);
+ return false;
+ }
+ if (p == host)
+ {
+ printfPQExpBuffer(errorMessage,
+ libpq_gettext("IPv6 host address may not be empty in URI: %s\n"),
+ uri);
+ return false;
+ }
+
+ /* Cut off the bracket and advance */
+ *(p++) = '\0';
+
+ /*
+ * The address must be followed by a port specifier or a slash or a
+ * query.
+ */
+ if (*p && *p != ':' && *p != '/' && *p != '?')
+ {
+ printfPQExpBuffer(errorMessage,
+ libpq_gettext("unexpected '%c' at position %td in URI (expecting ':' or '/'): %s\n"),
+ *p, p - buf + 1, uri);
+ return false;
+ }
+ }
+ else /* no IPv6 address */
+ {
+ host = p;
+ /*
+ * Look for port specifier (colon) or end of host specifier (slash), or
+ * query (question mark).
+ */
+ while (*p && *p != ':' && *p != '/' && *p != '?')
+ ++p;
+ }
+
+ /* Save the hostname terminator before we null it */
+ lastc = *p;
+ *p = '\0';
+
+ if (!conninfo_store_uri_encoded_value(options, "host", host, errorMessage,
+ false))
+ return false;
+
+ if (lastc == ':')
+ {
+ const char *port = ++p; /* advance past host terminator */
+
+ while (*p && *p != '/' && *p != '?')
+ ++p;
+
+ lastc = *p;
+ *p = '\0';
+
+ if (!conninfo_store_uri_encoded_value(options, "port", port,
+ errorMessage, false))
+ return false;
+ }
+
+ if (lastc && lastc != '?')
+ {
+ const char *dbname = ++p; /* advance past host terminator */
+
+ /* Look for query parameters */
+ while (*p && *p != '?')
+ ++p;
+
+ lastc = *p;
+ *p = '\0';
+
+ if (!conninfo_store_uri_encoded_value(options, "dbname", dbname,
+ errorMessage, false))
+ return false;
+ }
+
+ if (lastc)
+ {
+ ++p; /* advance past terminator */
+
+ if (!conninfo_uri_parse_params(p, options, errorMessage))
+ return false;
+ }
+
+ return true;
+ }
+
+ /*
+ * Connection URI parameters parser routine
+ *
+ * If successful, returns true while connOptions is filled with parsed
+ * parameters.
+ * If not successful, returns false and fills errorMessage accordingly.
+ *
+ * Destructively modifies 'params' buffer.
+ */
+ static bool
+ conninfo_uri_parse_params(char *params,
+ PQconninfoOption *connOptions,
+ PQExpBuffer errorMessage)
+ {
+ char *token;
+ char *savep;
+
+ while ((token = strtok_r(params, "&", &savep)))
+ {
+ const char *keyword;
+ const char *value;
+ char *p = strchr(token, '=');
+
+ if (p == NULL)
+ {
+ printfPQExpBuffer(errorMessage,
+ libpq_gettext("missing key/value separator '=' in URI query parameter: %s\n"),
+ token);
+ return false;
+ }
+
+ /* Cut off keyword and advance to value */
+ *(p++) = '\0';
+
+ keyword = token;
+ value = p;
+
+ /* Special keyword handling for improved JDBC compatibility */
+ if (strcmp(keyword, "ssl") == 0 &&
+ strcmp(value, "true") == 0)
+ {
+ keyword = "sslmode";
+ value = "require";
+ }
+
+ /*
+ * Store the value if corresponding option was found -- ignore
+ * otherwise.
+ *
+ * In theory, the keyword might be also percent-encoded, but hardly
+ * that's ever needed by anyone.
+ */
+ if (!conninfo_store_uri_encoded_value(connOptions, keyword, value,
+ errorMessage, true))
+ {
+ fprintf(stderr,
+ libpq_gettext("WARNING: ignoring unrecognized URI query parameter: %s\n"),
+ keyword);
+ }
+
+ params = NULL; /* proceed to the next token with strtok_r */
+ }
+
+ return true;
+ }
+
+ /*
+ * Connection URI decoder routine
+ *
+ * If successful, returns the malloc'd decoded string.
+ * If not successful, returns NULL and fills errorMessage accordingly.
+ *
+ * The string is decoded by replacing any percent-encoded tokens with
+ * corresponding characters, while preserving any non-encoded characters. A
+ * percent-encoded token is a character triplet: a percent sign, followed by a
+ * pair of hexadecimal digits (0-9A-F), where lower- and upper-case letters are
+ * treated identically.
+ */
static char *
+ conninfo_uri_decode(const char *str, PQExpBuffer errorMessage)
+ {
+ char *buf = malloc(strlen(str) + 1);
+ char *p = buf;
+ const char *q = str;
+
+ if (buf == NULL)
+ {
+ printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
+ return NULL;
+ }
+
+ for (;;)
+ {
+ if (*q != '%')
+ {
+ /* copy and check for NUL terminator */
+ if (!(*(p++) = *(q++)))
+ break;
+ }
+ else
+ {
+ int hi;
+ int lo;
+
+ ++q; /* skip the percent sign itself */
+
+ if (!(get_hexdigit(*q++, &hi) && get_hexdigit(*q++, &lo)))
+ {
+ printfPQExpBuffer(errorMessage,
+ libpq_gettext("invalid percent-encoded token (a pair of hexadecimal digits expected after every '%%' symbol, use '%%25' to encode percent symbol itself): %s\n"),
+ str);
+ free(buf);
+ return NULL;
+ }
+
+ *(p++) = (hi << 4) | lo;
+ }
+ }
+
+ return buf;
+ }
+
+ /*
+ * Convert hexadecimal digit character to it's integer value.
+ *
+ * If successful, returns true and value is filled with digit's base 16 value.
+ * If not successful, returns false.
+ *
+ * Lower- and upper-case letters in the range A-F are treated identically.
+ */
+ static bool
+ get_hexdigit(char digit, int *value)
+ {
+ if ('0' <= digit && digit <= '9')
+ {
+ *value = digit - '0';
+ }
+ else if ('A' <= digit && digit <= 'F')
+ {
+ *value = digit - 'A' + 10;
+ }
+ else if ('a' <= digit && digit <= 'f')
+ {
+ *value = digit - 'a' + 10;
+ }
+ else
+ {
+ return false;
+ }
+
+ return true;
+ }
+
+ /*
+ * Find an option value corresponding to the keyword in the connOptions array.
+ *
+ * If successful, returns a pointer to the corresponding option's value.
+ * If not successful, returns NULL.
+ */
+ static const char *
conninfo_getval(PQconninfoOption *connOptions,
const char *keyword)
{
+ PQconninfoOption *option = conninfo_find(connOptions, keyword);
+
+ return option ? option->val : NULL;
+ }
+
+ /*
+ * Store a (new) value for an option corresponding to the keyword in
+ * connOptions array.
+ *
+ * If successful, returns a pointer to the corresponding PQconninfoOption,
+ * which value is replaced with a strdup'd copy of the passed value string.
+ * The existing value for the option is free'd before replacing, if any.
+ *
+ * If not successful, returns NULL and fills errorMessage accordingly.
+ */
+ static PQconninfoOption *
+ conninfo_storeval(PQconninfoOption *connOptions,
+ const char *keyword, const char *value,
+ PQExpBuffer errorMessage, bool ignoreMissing)
+ {
+ PQconninfoOption *option = conninfo_find(connOptions, keyword);
+ char *value_copy;
+
+ if (option == NULL)
+ {
+ if (!ignoreMissing)
+ printfPQExpBuffer(errorMessage,
+ libpq_gettext("invalid connection option \"%s\"\n"),
+ keyword);
+ return NULL;
+ }
+
+ value_copy = strdup(value);
+ if (value_copy == NULL)
+ {
+ printfPQExpBuffer(errorMessage, libpq_gettext("out of memory\n"));
+ return NULL;
+ }
+
+ if (option->val)
+ free(option->val);
+ option->val = value_copy;
+
+ return option;
+ }
+
+ /*
+ * Store a (new) possibly URI-encoded value for an option corresponding to the
+ * keyword in connOptions array.
+ *
+ * If successful, returns a pointer to the corresponding PQconninfoOption,
+ * which value is replaced with a URI-decoded copy of the passed value string.
+ * The existing value for the option is free'd before replacing, if any.
+ *
+ * If not successful, returns NULL and fills errorMessage accordingly. If the
+ * corresponding option was not found, but ignoreMissing is true: doesn't fill
+ * errorMessage.
+ *
+ * See also conninfo_storeval.
+ */
+ static PQconninfoOption *
+ conninfo_store_uri_encoded_value(PQconninfoOption *connOptions,
+ const char *keyword, const char *encoded_value,
+ PQExpBuffer errorMessage, bool ignoreMissing)
+ {
+ PQconninfoOption *option;
+ char *decoded_value = conninfo_uri_decode(encoded_value, errorMessage);
+
+ if (decoded_value == NULL)
+ return NULL;
+
+ option = conninfo_storeval(connOptions, keyword, decoded_value,
+ errorMessage, ignoreMissing);
+ free(decoded_value);
+
+ return option;
+ }
+
+ /*
+ * Find a PQconninfoOption option corresponding to the keyword in the
+ * connOptions array.
+ *
+ * If successful, returns a pointer to the corresponding PQconninfoOption
+ * structure.
+ * If not successful, returns NULL.
+ */
+ static PQconninfoOption *
+ conninfo_find(PQconninfoOption *connOptions, const char *keyword)
+ {
PQconninfoOption *option;
for (option = connOptions; option->keyword != NULL; option++)
{
if (strcmp(option->keyword, keyword) == 0)
! return option;
}
return NULL;
#!/bin/sh
while read uri; do psql -d ${uri} -At -c "SELECT 1"; done <<EOF
postgresql://${USER}@localhost:5432/${USER}
postgresql://${USER}@localhost/${USER}
postgresql://localhost:5432/${USER}
postgresql://localhost/${USER}
postgresql://${USER}@localhost:5432/
postgresql://${USER}@localhost/
postgresql://localhost:5432/
postgresql://localhost:5432
postgresql://localhost/${USER}
postgresql://localhost/
postgresql://localhost
postgresql:///
postgresql://
postgresql://%6Cocalhost/
postgresql://localhost/${USER}?user=${USER}
postgresql://localhost/${USER}?user=${USER}&port=5432
postgresql://localhost/${USER}?user=${USER}&port=5432
postgresql://localhost:5432?user=${USER}
postgresql://localhost?user=${USER}
postgresql://localhost?uzer=
postgresql://localhost?
postgresql://[::1]:5432/${USER}
postgresql://[::1]/${USER}
postgresql://[::1]/
postgresql://[::1]
postgres://
EOF
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
