Code review for commit 274bb2b3857cc987cfa21d14775cae9b0dababa5.

Avoid memory leak in conninfo_uri_parse_options.  Use the current host
rather than the comma-separated list of host names when the host name
is needed for GSS, SSPI, or SSL authentication.  Document the way
connect_timeout interacts with multiple host specifications.

Takayuki Tsunakawa

diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index d04dba7493b2d08c9faa7323e0e90b6a1783de39..0f375bf5f251a85ee59f4c603467d15aa4851f1b 100644 (file)
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1009,6 +1009,10 @@ postgresql://%2Fvar%2Flib%2Fpostgresql/dbname
        Maximum wait for connection, in seconds (write as a decimal integer
        string). Zero or not specified means wait indefinitely.  It is not
        recommended to use a timeout of less than 2 seconds.
+       This timeout applies separately to each connection attempt.
+       For example, if you specify two hosts and both of them are unreachable,
+       and <literal>connect_timeout</> is 5, the total time spent waiting for a
+       connection might be up to 10 seconds.
       </para>
       </listitem>
      </varlistentry>

diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 19171fb676af103e5d44d40b743826d07c4be89c..d861dc487b4bcbe1a1b585937d26d91579a6bc64 100644 (file)
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -170,8 +170,9 @@ pg_GSS_startup(PGconn *conn)
                                min_stat;
        int                     maxlen;
        gss_buffer_desc temp_gbuf;
+       char       *host = PQhost(conn);
 
-       if (!(conn->pghost && conn->pghost[0] != '\0'))
+       if (!(host && host[0] != '\0'))
        {
                printfPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("host name must be specified\n"));
@@ -198,7 +199,7 @@ pg_GSS_startup(PGconn *conn)
                return STATUS_ERROR;
        }
        snprintf(temp_gbuf.value, maxlen, "%s@%s",
-                        conn->krbsrvname, conn->pghost);
+                        conn->krbsrvname, host);
        temp_gbuf.length = strlen(temp_gbuf.value);
 
        maj_stat = gss_import_name(&min_stat, &temp_gbuf,
@@ -371,6 +372,7 @@ pg_SSPI_startup(PGconn *conn, int use_negotiate)
 {
        SECURITY_STATUS r;
        TimeStamp       expire;
+       char       *host = PQhost(conn);
 
        conn->sspictx = NULL;
 
@@ -406,19 +408,19 @@ pg_SSPI_startup(PGconn *conn, int use_negotiate)
         * but not more complex. We can skip the @REALM part, because Windows will
         * fill that in for us automatically.
         */
-       if (!(conn->pghost && conn->pghost[0] != '\0'))
+       if (!(host && host[0] != '\0'))
        {
                printfPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("host name must be specified\n"));
                return STATUS_ERROR;
        }
-       conn->sspitarget = malloc(strlen(conn->krbsrvname) + strlen(conn->pghost) + 2);
+       conn->sspitarget = malloc(strlen(conn->krbsrvname) + strlen(host) + 2);
        if (!conn->sspitarget)
        {
                printfPQExpBuffer(&conn->errorMessage, libpq_gettext("out of memory\n"));
                return STATUS_ERROR;
        }
-       sprintf(conn->sspitarget, "%s/%s", conn->krbsrvname, conn->pghost);
+       sprintf(conn->sspitarget, "%s/%s", conn->krbsrvname, host);
 
        /*
         * Indicate that we're in SSPI authentication mode to make sure that

diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index ae85db9dd5aa900a12f7974355a0b485dcc9b51e..3e9c45bc4067e9bd3b2e2dea22566200eb3066fb 100644 (file)
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -4931,7 +4931,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
 {
        int                     prefix_len;
        char       *p;
-       char       *buf;
+       char       *buf = NULL;
        char       *start;
        char            prevchar = '\0';
        char       *user = NULL;
@@ -4946,7 +4946,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
        {
                printfPQExpBuffer(errorMessage,
                                                  libpq_gettext("out of memory\n"));
-               return false;
+               goto cleanup;
        }
 
        /* need a modifiable copy of the input URI */
@@ -4955,7 +4955,7 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
        {
                printfPQExpBuffer(errorMessage,
                                                  libpq_gettext("out of memory\n"));
-               return false;
+               goto cleanup;
        }
        start = buf;
 
@@ -5156,7 +5156,8 @@ conninfo_uri_parse_options(PQconninfoOption *options, const char *uri,
 cleanup:
        termPQExpBuffer(&hostbuf);
        termPQExpBuffer(&portbuf);
-       free(buf);
+       if (buf)
+               free(buf);
        return retval;
 }
 

diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
index f474c96f5fba25ac586bf7ccc78e3fd018730f98..7bdf92701a9556953124d7ba6d6627cd18bee805 100644 (file)
--- a/src/interfaces/libpq/fe-secure-openssl.c
+++ b/src/interfaces/libpq/fe-secure-openssl.c
@@ -483,6 +483,7 @@ verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *name_entry,
        char       *name;
        const unsigned char *namedata;
        int                     result;
+       char       *host = PQhost(conn);
 
        *store_name = NULL;
 
@@ -528,12 +529,12 @@ verify_peer_name_matches_certificate_name(PGconn *conn, ASN1_STRING *name_entry,
                return -1;
        }
 
-       if (pg_strcasecmp(name, conn->pghost) == 0)
+       if (pg_strcasecmp(name, host) == 0)
        {
                /* Exact name match */
                result = 1;
        }
-       else if (wildcard_certificate_match(name, conn->pghost))
+       else if (wildcard_certificate_match(name, host))
        {
                /* Matched wildcard name */
                result = 1;
@@ -563,6 +564,7 @@ verify_peer_name_matches_certificate(PGconn *conn)
        STACK_OF(GENERAL_NAME) *peer_san;
        int                     i;
        int                     rc;
+       char       *host = PQhost(conn);
 
        /*
         * If told not to verify the peer name, don't do it. Return true
@@ -572,7 +574,7 @@ verify_peer_name_matches_certificate(PGconn *conn)
                return true;
 
        /* Check that we have a hostname to compare with. */
-       if (!(conn->pghost && conn->pghost[0] != '\0'))
+       if (!(host && host[0] != '\0'))
        {
                printfPQExpBuffer(&conn->errorMessage,
                                                  libpq_gettext("host name must be specified for a verified SSL connection\n"));
@@ -670,13 +672,13 @@ verify_peer_name_matches_certificate(PGconn *conn)
                                                          libpq_ngettext("server certificate for \"%s\" (and %d other name) does not match host name \"%s\"\n",
                                                                                         "server certificate for \"%s\" (and %d other names) does not match host name \"%s\"\n",
                                                                                         names_examined - 1),
-                                                         first_name, names_examined - 1, conn->pghost);
+                                                         first_name, names_examined - 1, host);
                }
                else if (names_examined == 1)
                {
                        printfPQExpBuffer(&conn->errorMessage,
                                                          libpq_gettext("server certificate for \"%s\" does not match host name \"%s\"\n"),
-                                                         first_name, conn->pghost);
+                                                         first_name, host);
                }
                else
                {