On 2018-Nov-17, Fabien COELHO wrote: > Here is the updated v2 > > - libpq internal function getHostaddr get a length, > and I added an assert about it. > - added a few braces on if/if/else/if/else/else > - added an UNKNOWN_HOST macro to hide "???" > - moved host_addr[] declaration earlier to avoid some braces
You forgot to free(conn->connip) during freePGconn(). I found the UNKNOWN_HOST business quite dubious -- not only in your patch but also in the existing coding. I changed the getHostname API so that instead of returning "???" it sets the output buffer to the empty string. AFAICS the only user-visible behavior is that we no longer display the "???" in a parenthical comment next to the server address when a connection fails (this is pre-existing behavior, not changed by your patch.) Now, maybe the thinking was that upon seeing this message: could not connect to server: some error here Is the server running on host "pg.mines-paristech.fr" (???) and accepting connections on port 5432? the user was going to think "oh, my machine must have run out of memory, I'll reboot and retry". However, I highly doubt that anybody would reach that conclusion without reading the source code. So I deem this useless. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>From 342559f657f0c2d7cdf20c29536652a9ceca1d2a Mon Sep 17 00:00:00 2001 From: Alvaro Herrera <alvhe...@alvh.no-ip.org> Date: Mon, 19 Nov 2018 12:23:15 -0300 Subject: [PATCH] psql: Show IP address in \conninfo, if different from host When hostaddr is given, the actual IP address that psql is connected to can be totally unexpected for the given host. The more verbose output we now generate makes things clearer. Since the "host" and "hostaddr" parts of the conninfo could come from wildly different sources (say one is in the service specification or a URI-style conninfo and the other is not), this is useful. This is also useful if a hostname resolves to multiple addresses. Author: Fabien Coelho Reviewed-by: Pavel Stehule, Arthur Zakirov Discussion: https://postgr.es/m/alpine.DEB.2.21.1810261532380.27686@lancre https://postgr.es/m/alpine.DEB.2.21.1808201323020.13832@lancre --- doc/src/sgml/libpq.sgml | 30 ++++++++++++ src/bin/psql/command.c | 82 +++++++++++++++++++++++++------- src/interfaces/libpq/exports.txt | 1 + src/interfaces/libpq/fe-connect.c | 98 +++++++++++++++++++++++++++++---------- src/interfaces/libpq/libpq-fe.h | 1 + src/interfaces/libpq/libpq-int.h | 1 + 6 files changed, 172 insertions(+), 41 deletions(-) diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml index 601091c570..d2e5b08541 100644 --- a/doc/src/sgml/libpq.sgml +++ b/doc/src/sgml/libpq.sgml @@ -1735,6 +1735,36 @@ char *PQhost(const PGconn *conn); </listitem> </varlistentry> + + <varlistentry id="libpq-pqhostaddr"> + <term> + <function>PQhostaddr</function> + <indexterm> + <primary>PQhostaddr</primary> + </indexterm> + </term> + + <listitem> + <para> + Returns the server IP address of the active connection. + This can be the address that a host name resolved to, + or an IP address provided through the <literal>hostaddr</literal> + parameter. +<synopsis> +char *PQhostaddr(const PGconn *conn); +</synopsis> + </para> + + <para> + <function>PQhostaddr</function> returns <symbol>NULL</symbol> if the + <parameter>conn</parameter> argument is <symbol>NULL</symbol>. + Otherwise, if there is an error producing the host information + (perhaps if the connection has not been fully established or + there was an error), it returns an empty string. + </para> + </listitem> + </varlistentry> + <varlistentry id="libpq-pqport"> <term> <function>PQport</function> diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c index 04e227b5a6..ee88e1ca5c 100644 --- a/src/bin/psql/command.c +++ b/src/bin/psql/command.c @@ -596,14 +596,30 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch) else { char *host = PQhost(pset.db); + char *hostaddr = PQhostaddr(pset.db); - /* If the host is an absolute path, the connection is via socket */ + /* + * If the host is an absolute path, the connection is via socket + * unless overriden by hostaddr + */ if (is_absolute_path(host)) - printf(_("You are connected to database \"%s\" as user \"%s\" via socket in \"%s\" at port \"%s\".\n"), - db, PQuser(pset.db), host, PQport(pset.db)); + { + if (hostaddr && *hostaddr) + printf(_("You are connected to database \"%s\" as user \"%s\" on address \"%s\" at port \"%s\".\n"), + db, PQuser(pset.db), hostaddr, PQport(pset.db)); + else + printf(_("You are connected to database \"%s\" as user \"%s\" via socket in \"%s\" at port \"%s\".\n"), + db, PQuser(pset.db), host, PQport(pset.db)); + } else - printf(_("You are connected to database \"%s\" as user \"%s\" on host \"%s\" at port \"%s\".\n"), - db, PQuser(pset.db), host, PQport(pset.db)); + { + if (hostaddr && *hostaddr && strcmp(host, hostaddr) != 0) + printf(_("You are connected to database \"%s\" as user \"%s\" on host \"%s\" (address \"%s\") at port \"%s\".\n"), + db, PQuser(pset.db), host, hostaddr, PQport(pset.db)); + else + printf(_("You are connected to database \"%s\" as user \"%s\" on host \"%s\" at port \"%s\".\n"), + db, PQuser(pset.db), host, PQport(pset.db)); + } printSSLInfo(); } } @@ -2854,6 +2870,7 @@ do_connect(enum trivalue reuse_previous_specification, PGconn *o_conn = pset.db, *n_conn; char *password = NULL; + char *hostaddr = NULL; bool keep_password; bool has_connection_string; bool reuse_previous; @@ -2894,12 +2911,27 @@ do_connect(enum trivalue reuse_previous_specification, } /* grab missing values from the old connection */ - if (!user && reuse_previous) - user = PQuser(o_conn); - if (!host && reuse_previous) - host = PQhost(o_conn); - if (!port && reuse_previous) - port = PQport(o_conn); + if (reuse_previous) + { + if (!user) + user = PQuser(o_conn); + if (host && strcmp(host, PQhost(o_conn)) == 0) + { + /* + * if we are targetting the same host, reuse its hostaddr for + * consistency + */ + hostaddr = PQhostaddr(o_conn); + } + if (!host) + { + host = PQhost(o_conn); + /* also set hostaddr for consistency */ + hostaddr = PQhostaddr(o_conn); + } + if (!port) + port = PQport(o_conn); + } /* * Any change in the parameters read above makes us discard the password. @@ -2961,13 +2993,18 @@ do_connect(enum trivalue reuse_previous_specification, while (true) { -#define PARAMS_ARRAY_SIZE 8 +#define PARAMS_ARRAY_SIZE 9 const char **keywords = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*keywords)); const char **values = pg_malloc(PARAMS_ARRAY_SIZE * sizeof(*values)); int paramnum = -1; keywords[++paramnum] = "host"; values[paramnum] = host; + if (hostaddr && *hostaddr) + { + keywords[++paramnum] = "hostaddr"; + values[paramnum] = hostaddr; + } keywords[++paramnum] = "port"; values[paramnum] = port; keywords[++paramnum] = "user"; @@ -3071,14 +3108,27 @@ do_connect(enum trivalue reuse_previous_specification, param_is_newly_set(PQport(o_conn), PQport(pset.db))) { char *host = PQhost(pset.db); + char *hostaddr = PQhostaddr(pset.db); /* If the host is an absolute path, the connection is via socket */ if (is_absolute_path(host)) - printf(_("You are now connected to database \"%s\" as user \"%s\" via socket in \"%s\" at port \"%s\".\n"), - PQdb(pset.db), PQuser(pset.db), host, PQport(pset.db)); + { + if (hostaddr && *hostaddr) + printf(_("You are now connected to database \"%s\" as user \"%s\" on address \"%s\" at port \"%s\".\n"), + PQdb(pset.db), PQuser(pset.db), hostaddr, PQport(pset.db)); + else + printf(_("You are now connected to database \"%s\" as user \"%s\" via socket in \"%s\" at port \"%s\".\n"), + PQdb(pset.db), PQuser(pset.db), host, PQport(pset.db)); + } else - printf(_("You are now connected to database \"%s\" as user \"%s\" on host \"%s\" at port \"%s\".\n"), - PQdb(pset.db), PQuser(pset.db), host, PQport(pset.db)); + { + if (hostaddr && *hostaddr && strcmp(host, hostaddr) != 0) + printf(_("You are now connected to database \"%s\" as user \"%s\" on host \"%s\" (address \"%s\") at port \"%s\".\n"), + PQdb(pset.db), PQuser(pset.db), host, hostaddr, PQport(pset.db)); + else + printf(_("You are now connected to database \"%s\" as user \"%s\" on host \"%s\" at port \"%s\".\n"), + PQdb(pset.db), PQuser(pset.db), host, PQport(pset.db)); + } } else printf(_("You are now connected to database \"%s\" as user \"%s\".\n"), diff --git a/src/interfaces/libpq/exports.txt b/src/interfaces/libpq/exports.txt index 4359fae30d..cc9ee9ce6b 100644 --- a/src/interfaces/libpq/exports.txt +++ b/src/interfaces/libpq/exports.txt @@ -173,3 +173,4 @@ PQsetErrorContextVisibility 170 PQresultVerboseErrorMessage 171 PQencryptPasswordConn 172 PQresultMemorySize 173 +PQhostaddr 174 diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c index d001bc513d..bc456fec0c 100644 --- a/src/interfaces/libpq/fe-connect.c +++ b/src/interfaces/libpq/fe-connect.c @@ -1471,6 +1471,39 @@ connectNoDelay(PGconn *conn) return 1; } +/* ---------- + * Write currently connected IP address into host_addr (of len host_addr_len). + * If unable to, set it to the empty string. + * ---------- + */ +static void +getHostaddr(PGconn *conn, char *host_addr, int host_addr_len) +{ + struct sockaddr_storage *addr = &conn->raddr.addr; + + if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS) + strlcpy(host_addr, conn->connhost[conn->whichhost].hostaddr, host_addr_len); + else if (addr->ss_family == AF_INET) + { + if (inet_net_ntop(AF_INET, + &((struct sockaddr_in *) addr)->sin_addr.s_addr, + 32, + host_addr, host_addr_len) == NULL) + host_addr[0] = '\0'; + } +#ifdef HAVE_IPV6 + else if (addr->ss_family == AF_INET6) + { + if (inet_net_ntop(AF_INET6, + &((struct sockaddr_in6 *) addr)->sin6_addr.s6_addr, + 128, + host_addr, host_addr_len) == NULL) + host_addr[0] = '\0'; + } +#endif + else + host_addr[0] = '\0'; +} /* ---------- * connectFailureMessage - @@ -1504,34 +1537,12 @@ connectFailureMessage(PGconn *conn, int errorno) char host_addr[NI_MAXHOST]; const char *displayed_host; const char *displayed_port; - struct sockaddr_storage *addr = &conn->raddr.addr; /* * Optionally display the network address with the hostname. This is * useful to distinguish between IPv4 and IPv6 connections. */ - if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS) - strlcpy(host_addr, conn->connhost[conn->whichhost].hostaddr, NI_MAXHOST); - else if (addr->ss_family == AF_INET) - { - if (inet_net_ntop(AF_INET, - &((struct sockaddr_in *) addr)->sin_addr.s_addr, - 32, - host_addr, sizeof(host_addr)) == NULL) - strcpy(host_addr, "???"); - } -#ifdef HAVE_IPV6 - else if (addr->ss_family == AF_INET6) - { - if (inet_net_ntop(AF_INET6, - &((struct sockaddr_in6 *) addr)->sin6_addr.s6_addr, - 128, - host_addr, sizeof(host_addr)) == NULL) - strcpy(host_addr, "???"); - } -#endif - else - strcpy(host_addr, "???"); + getHostaddr(conn, host_addr, NI_MAXHOST); /* To which host and port were we actually connecting? */ if (conn->connhost[conn->whichhost].type == CHT_HOST_ADDRESS) @@ -1548,14 +1559,14 @@ connectFailureMessage(PGconn *conn, int errorno) * looked-up IP address. */ if (conn->connhost[conn->whichhost].type != CHT_HOST_ADDRESS && + strlen(host_addr) > 0 && strcmp(displayed_host, host_addr) != 0) appendPQExpBuffer(&conn->errorMessage, libpq_gettext("could not connect to server: %s\n" "\tIs the server running on host \"%s\" (%s) and accepting\n" "\tTCP/IP connections on port %s?\n"), SOCK_STRERROR(errorno, sebuf, sizeof(sebuf)), - displayed_host, - host_addr, + displayed_host, host_addr, displayed_port); else appendPQExpBuffer(&conn->errorMessage, @@ -2286,6 +2297,7 @@ keep_going: /* We will come back to here until there is */ { struct addrinfo *addr_cur = conn->addr_cur; + char host_addr[NI_MAXHOST]; /* * Advance to next possible host, if we've tried all of @@ -2302,6 +2314,21 @@ keep_going: /* We will come back to here until there is addr_cur->ai_addrlen); conn->raddr.salen = addr_cur->ai_addrlen; + /* set connip */ + if (conn->connip != NULL) + { + free(conn->connip); + conn->connip = NULL; + } + + getHostaddr(conn, host_addr, NI_MAXHOST); + if (strlen(host_addr) > 0) + conn->connip = strdup(host_addr); + /* + * purposely ignore strdup failure; not a big problem if + * it fails anyway. + */ + conn->sock = socket(addr_cur->ai_family, SOCK_STREAM, 0); if (conn->sock == PGINVALID_SOCKET) { @@ -3665,6 +3692,8 @@ freePGconn(PGconn *conn) free(conn->sslcompression); if (conn->requirepeer) free(conn->requirepeer); + if (conn->connip) + free(conn->connip); #if defined(ENABLE_GSS) || defined(ENABLE_SSPI) if (conn->krbsrvname) free(conn->krbsrvname); @@ -6173,6 +6202,25 @@ PQhost(const PGconn *conn) } char * +PQhostaddr(const PGconn *conn) +{ + if (!conn) + return NULL; + + if (conn->connhost != NULL) + { + if (conn->connhost[conn->whichhost].hostaddr != NULL && + conn->connhost[conn->whichhost].hostaddr[0] != '\0') + return conn->connhost[conn->whichhost].hostaddr; + + if (conn->connip != NULL) + return conn->connip; + } + + return ""; +} + +char * PQport(const PGconn *conn) { if (!conn) diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index 52bd5d2cd8..3f13ddf092 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -312,6 +312,7 @@ extern char *PQdb(const PGconn *conn); extern char *PQuser(const PGconn *conn); extern char *PQpass(const PGconn *conn); extern char *PQhost(const PGconn *conn); +extern char *PQhostaddr(const PGconn *conn); extern char *PQport(const PGconn *conn); extern char *PQtty(const PGconn *conn); extern char *PQoptions(const PGconn *conn); diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h index 975ab33d02..66fd317b94 100644 --- a/src/interfaces/libpq/libpq-int.h +++ b/src/interfaces/libpq/libpq-int.h @@ -397,6 +397,7 @@ struct pg_conn int nconnhost; /* # of hosts named in conn string */ int whichhost; /* host we're currently trying/connected to */ pg_conn_host *connhost; /* details about each named host */ + char *connip; /* IP address for current network connection */ /* Connection data */ pgsocket sock; /* FD for socket, PGINVALID_SOCKET if -- 2.11.0
