abstract Unix-domain sockets
During the discussion on Unix-domain sockets on Windows, someone pointed
out[0]/messages/by-id/20191218142419.fvv4ikm4wq4gnkco@isc.upenn.edu abstract Unix-domain sockets. This is a variant of the normal
Unix-domain sockets that don't use the file system but a separate
"abstract" namespace. At the user interface, such sockets are
represented by names starting with "@". I took a look at this and it
wasn't hard to get working, so here is a patch. It's supposed to be
supported on Linux and Windows right now, but I haven't tested on Windows.
I figure, there are so many different deployment options nowadays, this
could be useful somewhere. It relieves you from dealing with the file
system, you don't have to set up /tmp or something under /var/run, you
don't need to make sure file system permissions are right. Also, there
is no need for a lock file or file cleanup. (Unlike file-system
namespace sockets, abstract namespace sockets give an EADDRINUSE when
trying to bind to a name already in use.) Conversely, of course, you
don't get to use file-system permissions to manage access to the socket,
but that isn't essential functionality, so it's a trade-off users can
make on their own.
And then some extra patches for surrounding cleanup. During testing I
noticed that the bind() failure hint "Is another postmaster already
running ..." was shown in inappropriate situations, so I changed that to
only show for EADDRINUSE errors. (Maybe other error codes could be
appropriate, but I couldn't find any more.)
And then looking for other uses of EADDRINUSE I found some dead
Windows-related code that can be cleaned up.
[0]: /messages/by-id/20191218142419.fvv4ikm4wq4gnkco@isc.upenn.edu
/messages/by-id/20191218142419.fvv4ikm4wq4gnkco@isc.upenn.edu
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Add-support-for-abstract-Unix-domain-sockets.patchtext/plain; charset=UTF-8; name=0001-Add-support-for-abstract-Unix-domain-sockets.patch; x-mac-creator=0; x-mac-type=0Download
From 5afc3342a306a1d9599bb5e6c77aeb2581b6799c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 9 Oct 2020 09:10:41 +0200
Subject: [PATCH 1/3] Add support for abstract Unix-domain sockets
This is a variant of the normal Unix-domain sockets that don't use the
file system but a separate "abstract" namespace. At the user
interface, such sockets are represented by names starting with "@".
Supported on Linux and Windows right now.
---
doc/src/sgml/config.sgml | 24 +++++++++++++++++++++++-
doc/src/sgml/libpq.sgml | 5 ++++-
src/backend/libpq/pqcomm.c | 8 ++++++++
src/bin/psql/command.c | 15 +++++----------
src/bin/psql/prompt.c | 3 ++-
src/common/ip.c | 24 +++++++++++++++++++++++-
src/include/libpq/pqcomm.h | 9 +++++++++
src/interfaces/libpq/fe-connect.c | 4 ++--
8 files changed, 76 insertions(+), 16 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index ee914740cc..8fc76bb40f 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -749,6 +749,21 @@ <title>Connection Settings</title>
An empty value
specifies not listening on any Unix-domain sockets, in which case
only TCP/IP sockets can be used to connect to the server.
+ </para>
+
+ <para>
+ A value that starts with <literal>@</literal> specifies that a
+ Unix-domain socket in the abstract namespace should be created
+ (currently supported on Linux and Windows). In that case, this value
+ does not specify a <quote>directory</quote> but a prefix from which
+ the actual socket name is computed in the same manner as for the
+ file-system namespace. While the abstract socket name prefix can be
+ chosen freely, since it is not a file-system location, the convention
+ is to nonetheless use file-system-like values such as
+ <literal>@/tmp</literal>.
+ </para>
+
+ <para>
The default value is normally
<filename>/tmp</filename>, but that can be changed at build time.
On Windows, the default is empty, which means no Unix-domain socket is
@@ -763,6 +778,7 @@ <title>Connection Settings</title>
named <literal>.s.PGSQL.<replaceable>nnnn</replaceable>.lock</literal> will be
created in each of the <varname>unix_socket_directories</varname> directories.
Neither file should ever be removed manually.
+ For sockets in the abstract namespace, no lock file is created.
</para>
</listitem>
</varlistentry>
@@ -787,7 +803,8 @@ <title>Connection Settings</title>
<para>
This parameter is not supported on Windows. Any setting will be
- ignored.
+ ignored. Also, sockets in the abstract namespace have no file owner,
+ so this setting is also ignored in that case.
</para>
</listitem>
</varlistentry>
@@ -834,6 +851,11 @@ <title>Connection Settings</title>
similar effect by pointing <varname>unix_socket_directories</varname> to a
directory having search permission limited to the desired audience.
</para>
+
+ <para>
+ Sockets in the abstract namespace have no file permissions, so this
+ setting is also ignored in that case.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 3315f1dd05..040750b2b2 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1046,7 +1046,10 @@ <title>Parameter Key Words</title>
communication; the value is the name of the directory in which the
socket file is stored. (On Unix, an absolute path name begins with a
slash. On Windows, paths starting with drive letters are also
- recognized.) The default behavior when <literal>host</literal> is not
+ recognized.) If the host name starts with <literal>@</literal>, it is
+ taken as a Unix-domain socket in the abstract namespace (currently
+ supported on Linux and Windows).
+ The default behavior when <literal>host</literal> is not
specified, or is empty, is to connect to a Unix-domain
socket<indexterm><primary>Unix domain socket</primary></indexterm> in
<filename>/tmp</filename> (or whatever socket directory was specified
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index ac986c0505..84568508c5 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -611,6 +611,10 @@ StreamServerPort(int family, const char *hostName, unsigned short portNumber,
static int
Lock_AF_UNIX(const char *unixSocketDir, const char *unixSocketPath)
{
+ /* no lock file for abstract sockets */
+ if (unixSocketPath[0] == '@')
+ return STATUS_OK;
+
/*
* Grab an interlock file associated with the socket file.
*
@@ -642,6 +646,10 @@ Lock_AF_UNIX(const char *unixSocketDir, const char *unixSocketPath)
static int
Setup_AF_UNIX(const char *sock_path)
{
+ /* no file system permissions for abstract sockets */
+ if (sock_path[0] == '@')
+ return STATUS_OK;
+
/*
* Fix socket ownership/permission if requested. Note we must do this
* before we listen() to avoid a window where unwanted connections could
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index d4aa0976b5..2a542eaa4c 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -37,6 +37,7 @@
#include "input.h"
#include "large_obj.h"
#include "libpq-fe.h"
+#include "libpq/pqcomm.h"
#include "mainloop.h"
#include "portability/instr_time.h"
#include "pqexpbuffer.h"
@@ -604,12 +605,9 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch)
char *host = PQhost(pset.db);
char *hostaddr = PQhostaddr(pset.db);
- /*
- * If the host is an absolute path, the connection is via socket
- * unless overridden by hostaddr
- */
- if (is_absolute_path(host))
+ if (is_unixsock_path(host))
{
+ /* hostaddr overrides host */
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));
@@ -3302,12 +3300,9 @@ do_connect(enum trivalue reuse_previous_specification,
char *host = PQhost(pset.db);
char *hostaddr = PQhostaddr(pset.db);
- /*
- * If the host is an absolute path, the connection is via socket
- * unless overridden by hostaddr
- */
- if (is_absolute_path(host))
+ if (is_unixsock_path(host))
{
+ /* hostaddr overrides host */
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));
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index ef503ec41b..f42c3dfc74 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -15,6 +15,7 @@
#include "common.h"
#include "common/string.h"
#include "input.h"
+#include "libpq/pqcomm.h"
#include "prompt.h"
#include "settings.h"
@@ -136,7 +137,7 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
const char *host = PQhost(pset.db);
/* INET socket */
- if (host && host[0] && !is_absolute_path(host))
+ if (host && host[0] && !is_unixsock_path(host))
{
strlcpy(buf, host, sizeof(buf));
if (*p == 'm')
diff --git a/src/common/ip.c b/src/common/ip.c
index 69fcca8479..bcc779e00c 100644
--- a/src/common/ip.c
+++ b/src/common/ip.c
@@ -217,6 +217,21 @@ getaddrinfo_unix(const char *path, const struct addrinfo *hintsp,
strcpy(unp->sun_path, path);
+ /*
+ * If the supplied path starts with @, replace that with a zero byte for
+ * the internal representation. In that mode, the entire sun_path is the
+ * address, including trailing zero bytes. But we set the address length
+ * to only include the length of the original string. That way the
+ * trailing zero bytes won't show up in any network or socket lists of the
+ * operating system. This is just a convention, also followed by other
+ * packages.
+ */
+ if (path[0] == '@')
+ {
+ unp->sun_path[0] = '\0';
+ aip->ai_addrlen = offsetof(struct sockaddr_un, sun_path) + strlen(path);
+ }
+
#ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
unp->sun_len = sizeof(struct sockaddr_un);
#endif
@@ -249,7 +264,14 @@ getnameinfo_unix(const struct sockaddr_un *sa, int salen,
if (service)
{
- ret = snprintf(service, servicelen, "%s", sa->sun_path);
+ /*
+ * Check whether it looks like an abstract socket, but it could also
+ * just be an empty string.
+ */
+ if (sa->sun_path[0] == '\0' && sa->sun_path[1] != '\0')
+ ret = snprintf(service, servicelen, "@%s", sa->sun_path + 1);
+ else
+ ret = snprintf(service, servicelen, "%s", sa->sun_path);
if (ret < 0 || ret >= servicelen)
return EAI_MEMORY;
}
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 781d86c8ef..cf967c3987 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -85,6 +85,15 @@ typedef struct
*/
#define UNIXSOCK_PATH_BUFLEN sizeof(((struct sockaddr_un *) NULL)->sun_path)
+/*
+ * A host that looks either like an absolute path or starts with @ is
+ * interpreted as a Unix-domain socket address.
+ */
+static inline bool
+is_unixsock_path(const char *path)
+{
+ return is_absolute_path(path) || path[0] == '@';
+}
/*
* These manipulate the frontend/backend protocol version number.
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index af27fee6b5..d83850a4b6 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1093,7 +1093,7 @@ connectOptions2(PGconn *conn)
{
ch->type = CHT_HOST_NAME;
#ifdef HAVE_UNIX_SOCKETS
- if (is_absolute_path(ch->host))
+ if (is_unixsock_path(ch->host))
ch->type = CHT_UNIX_SOCKET;
#endif
}
@@ -6942,7 +6942,7 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname,
/* 'localhost' matches pghost of '' or the default socket directory */
if (hostname == NULL || hostname[0] == '\0')
hostname = DefaultHost;
- else if (is_absolute_path(hostname))
+ else if (is_unixsock_path(hostname))
/*
* We should probably use canonicalize_path(), but then we have to
--
2.28.0
0002-Make-error-hint-from-bind-failure-more-accurate.patchtext/plain; charset=UTF-8; name=0002-Make-error-hint-from-bind-failure-more-accurate.patch; x-mac-creator=0; x-mac-type=0Download
From 88c9420c63c9cfe8b0006341cb242bf2e37c3a24 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 9 Oct 2020 09:10:41 +0200
Subject: [PATCH 2/3] Make error hint from bind() failure more accurate
The hint "Is another postmaster already running ..." should only be
printed for errors that are really about something else already using
the address. In other cases it is misleading.
---
src/backend/libpq/pqcomm.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 84568508c5..b8be7de70e 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -530,18 +530,21 @@ StreamServerPort(int family, const char *hostName, unsigned short portNumber,
err = bind(fd, addr->ai_addr, addr->ai_addrlen);
if (err < 0)
{
+ int saved_errno = errno;
+
ereport(LOG,
(errcode_for_socket_access(),
/* translator: first %s is IPv4, IPv6, or Unix */
errmsg("could not bind %s address \"%s\": %m",
familyDesc, addrDesc),
- (IS_AF_UNIX(addr->ai_family)) ?
- errhint("Is another postmaster already running on port %d?"
- " If not, remove socket file \"%s\" and retry.",
- (int) portNumber, service) :
- errhint("Is another postmaster already running on port %d?"
- " If not, wait a few seconds and retry.",
- (int) portNumber)));
+ saved_errno == EADDRINUSE ?
+ (IS_AF_UNIX(addr->ai_family) ?
+ errhint("Is another postmaster already running on port %d?"
+ " If not, remove socket file \"%s\" and retry.",
+ (int) portNumber, service) :
+ errhint("Is another postmaster already running on port %d?"
+ " If not, wait a few seconds and retry.",
+ (int) portNumber)) : 0));
closesocket(fd);
continue;
}
--
2.28.0
0003-Remove-obsolete-ifdefs.patchtext/plain; charset=UTF-8; name=0003-Remove-obsolete-ifdefs.patch; x-mac-creator=0; x-mac-type=0Download
From 54d79b20d78d4cdd48e49b3d6d9902414db78971 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 9 Oct 2020 09:10:41 +0200
Subject: [PATCH 3/3] Remove obsolete ifdefs
Commit 8dace66e0735ca39b779922d02c24ea2686e6521 added #ifdefs for a
number of errno symbols because they were not present on Windows.
Later, commit 125ad539a275db5ab8f4647828b80a16d02eabd2 added
replacement #defines for some of those symbols. So some of the
changes from the first commit are made dead code by the second commit
and can now be removed.
---
src/port/strerror.c | 14 --------------
1 file changed, 14 deletions(-)
diff --git a/src/port/strerror.c b/src/port/strerror.c
index 375edb0f5a..55f8eb3917 100644
--- a/src/port/strerror.c
+++ b/src/port/strerror.c
@@ -118,14 +118,10 @@ get_errno_symbol(int errnum)
return "E2BIG";
case EACCES:
return "EACCES";
-#ifdef EADDRINUSE
case EADDRINUSE:
return "EADDRINUSE";
-#endif
-#ifdef EADDRNOTAVAIL
case EADDRNOTAVAIL:
return "EADDRNOTAVAIL";
-#endif
case EAFNOSUPPORT:
return "EAFNOSUPPORT";
#ifdef EAGAIN
@@ -146,10 +142,8 @@ get_errno_symbol(int errnum)
return "EBUSY";
case ECHILD:
return "ECHILD";
-#ifdef ECONNABORTED
case ECONNABORTED:
return "ECONNABORTED";
-#endif
case ECONNREFUSED:
return "ECONNREFUSED";
#ifdef ECONNRESET
@@ -166,10 +160,8 @@ get_errno_symbol(int errnum)
return "EFAULT";
case EFBIG:
return "EFBIG";
-#ifdef EHOSTUNREACH
case EHOSTUNREACH:
return "EHOSTUNREACH";
-#endif
case EIDRM:
return "EIDRM";
case EINPROGRESS:
@@ -180,10 +172,8 @@ get_errno_symbol(int errnum)
return "EINVAL";
case EIO:
return "EIO";
-#ifdef EISCONN
case EISCONN:
return "EISCONN";
-#endif
case EISDIR:
return "EISDIR";
#ifdef ELOOP
@@ -214,20 +204,16 @@ get_errno_symbol(int errnum)
return "ENOSPC";
case ENOSYS:
return "ENOSYS";
-#ifdef ENOTCONN
case ENOTCONN:
return "ENOTCONN";
-#endif
case ENOTDIR:
return "ENOTDIR";
#if defined(ENOTEMPTY) && (ENOTEMPTY != EEXIST) /* same code on AIX */
case ENOTEMPTY:
return "ENOTEMPTY";
#endif
-#ifdef ENOTSOCK
case ENOTSOCK:
return "ENOTSOCK";
-#endif
#ifdef ENOTSUP
case ENOTSUP:
return "ENOTSUP";
--
2.28.0
Updated patch set after some conflicts had emerged.
On 2020-10-09 09:28, Peter Eisentraut wrote:
During the discussion on Unix-domain sockets on Windows, someone pointed
out[0] abstract Unix-domain sockets. This is a variant of the normal
Unix-domain sockets that don't use the file system but a separate
"abstract" namespace. At the user interface, such sockets are
represented by names starting with "@". I took a look at this and it
wasn't hard to get working, so here is a patch. It's supposed to be
supported on Linux and Windows right now, but I haven't tested on Windows.I figure, there are so many different deployment options nowadays, this
could be useful somewhere. It relieves you from dealing with the file
system, you don't have to set up /tmp or something under /var/run, you
don't need to make sure file system permissions are right. Also, there
is no need for a lock file or file cleanup. (Unlike file-system
namespace sockets, abstract namespace sockets give an EADDRINUSE when
trying to bind to a name already in use.) Conversely, of course, you
don't get to use file-system permissions to manage access to the socket,
but that isn't essential functionality, so it's a trade-off users can
make on their own.And then some extra patches for surrounding cleanup. During testing I
noticed that the bind() failure hint "Is another postmaster already
running ..." was shown in inappropriate situations, so I changed that to
only show for EADDRINUSE errors. (Maybe other error codes could be
appropriate, but I couldn't find any more.)And then looking for other uses of EADDRINUSE I found some dead
Windows-related code that can be cleaned up.
This last piece has been committed.
[0]:
/messages/by-id/20191218142419.fvv4ikm4wq4gnkco@isc.upenn.edu
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v2-0001-Add-support-for-abstract-Unix-domain-sockets.patchtext/plain; charset=UTF-8; name=v2-0001-Add-support-for-abstract-Unix-domain-sockets.patch; x-mac-creator=0; x-mac-type=0Download
From 537b5bbfc231551a76f55de3ff0cba5722b20298 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 22 Oct 2020 08:47:52 +0200
Subject: [PATCH v2 1/2] Add support for abstract Unix-domain sockets
This is a variant of the normal Unix-domain sockets that don't use the
file system but a separate "abstract" namespace. At the user
interface, such sockets are represented by names starting with "@".
Supported on Linux and Windows right now.
Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
---
doc/src/sgml/config.sgml | 24 +++++++++++++++++++++++-
doc/src/sgml/libpq.sgml | 5 ++++-
src/backend/libpq/pqcomm.c | 8 ++++++++
src/bin/psql/command.c | 15 +++++----------
src/bin/psql/prompt.c | 3 ++-
src/common/ip.c | 24 +++++++++++++++++++++++-
src/include/libpq/pqcomm.h | 9 +++++++++
src/interfaces/libpq/fe-connect.c | 4 ++--
8 files changed, 76 insertions(+), 16 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index f043433e31..e2d92dd523 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -749,6 +749,21 @@ <title>Connection Settings</title>
An empty value
specifies not listening on any Unix-domain sockets, in which case
only TCP/IP sockets can be used to connect to the server.
+ </para>
+
+ <para>
+ A value that starts with <literal>@</literal> specifies that a
+ Unix-domain socket in the abstract namespace should be created
+ (currently supported on Linux and Windows). In that case, this value
+ does not specify a <quote>directory</quote> but a prefix from which
+ the actual socket name is computed in the same manner as for the
+ file-system namespace. While the abstract socket name prefix can be
+ chosen freely, since it is not a file-system location, the convention
+ is to nonetheless use file-system-like values such as
+ <literal>@/tmp</literal>.
+ </para>
+
+ <para>
The default value is normally
<filename>/tmp</filename>, but that can be changed at build time.
On Windows, the default is empty, which means no Unix-domain socket is
@@ -763,6 +778,7 @@ <title>Connection Settings</title>
named <literal>.s.PGSQL.<replaceable>nnnn</replaceable>.lock</literal> will be
created in each of the <varname>unix_socket_directories</varname> directories.
Neither file should ever be removed manually.
+ For sockets in the abstract namespace, no lock file is created.
</para>
</listitem>
</varlistentry>
@@ -787,7 +803,8 @@ <title>Connection Settings</title>
<para>
This parameter is not supported on Windows. Any setting will be
- ignored.
+ ignored. Also, sockets in the abstract namespace have no file owner,
+ so this setting is also ignored in that case.
</para>
</listitem>
</varlistentry>
@@ -834,6 +851,11 @@ <title>Connection Settings</title>
similar effect by pointing <varname>unix_socket_directories</varname> to a
directory having search permission limited to the desired audience.
</para>
+
+ <para>
+ Sockets in the abstract namespace have no file permissions, so this
+ setting is also ignored in that case.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index de60281fcb..612c1c94be 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1031,7 +1031,10 @@ <title>Parameter Key Words</title>
communication; the value is the name of the directory in which the
socket file is stored. (On Unix, an absolute path name begins with a
slash. On Windows, paths starting with drive letters are also
- recognized.) The default behavior when <literal>host</literal> is not
+ recognized.) If the host name starts with <literal>@</literal>, it is
+ taken as a Unix-domain socket in the abstract namespace (currently
+ supported on Linux and Windows).
+ The default behavior when <literal>host</literal> is not
specified, or is empty, is to connect to a Unix-domain
socket<indexterm><primary>Unix domain socket</primary></indexterm> in
<filename>/tmp</filename> (or whatever socket directory was specified
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index ac986c0505..84568508c5 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -611,6 +611,10 @@ StreamServerPort(int family, const char *hostName, unsigned short portNumber,
static int
Lock_AF_UNIX(const char *unixSocketDir, const char *unixSocketPath)
{
+ /* no lock file for abstract sockets */
+ if (unixSocketPath[0] == '@')
+ return STATUS_OK;
+
/*
* Grab an interlock file associated with the socket file.
*
@@ -642,6 +646,10 @@ Lock_AF_UNIX(const char *unixSocketDir, const char *unixSocketPath)
static int
Setup_AF_UNIX(const char *sock_path)
{
+ /* no file system permissions for abstract sockets */
+ if (sock_path[0] == '@')
+ return STATUS_OK;
+
/*
* Fix socket ownership/permission if requested. Note we must do this
* before we listen() to avoid a window where unwanted connections could
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index ba3ea39aaa..da9e21a2bb 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -37,6 +37,7 @@
#include "input.h"
#include "large_obj.h"
#include "libpq-fe.h"
+#include "libpq/pqcomm.h"
#include "mainloop.h"
#include "portability/instr_time.h"
#include "pqexpbuffer.h"
@@ -604,12 +605,9 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch)
char *host = PQhost(pset.db);
char *hostaddr = PQhostaddr(pset.db);
- /*
- * If the host is an absolute path, the connection is via socket
- * unless overridden by hostaddr
- */
- if (is_absolute_path(host))
+ if (is_unixsock_path(host))
{
+ /* hostaddr overrides host */
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));
@@ -3380,12 +3378,9 @@ do_connect(enum trivalue reuse_previous_specification,
char *host = PQhost(pset.db);
char *hostaddr = PQhostaddr(pset.db);
- /*
- * If the host is an absolute path, the connection is via socket
- * unless overridden by hostaddr
- */
- if (is_absolute_path(host))
+ if (is_unixsock_path(host))
{
+ /* hostaddr overrides host */
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));
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index ef503ec41b..f42c3dfc74 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -15,6 +15,7 @@
#include "common.h"
#include "common/string.h"
#include "input.h"
+#include "libpq/pqcomm.h"
#include "prompt.h"
#include "settings.h"
@@ -136,7 +137,7 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
const char *host = PQhost(pset.db);
/* INET socket */
- if (host && host[0] && !is_absolute_path(host))
+ if (host && host[0] && !is_unixsock_path(host))
{
strlcpy(buf, host, sizeof(buf));
if (*p == 'm')
diff --git a/src/common/ip.c b/src/common/ip.c
index 69fcca8479..bcc779e00c 100644
--- a/src/common/ip.c
+++ b/src/common/ip.c
@@ -217,6 +217,21 @@ getaddrinfo_unix(const char *path, const struct addrinfo *hintsp,
strcpy(unp->sun_path, path);
+ /*
+ * If the supplied path starts with @, replace that with a zero byte for
+ * the internal representation. In that mode, the entire sun_path is the
+ * address, including trailing zero bytes. But we set the address length
+ * to only include the length of the original string. That way the
+ * trailing zero bytes won't show up in any network or socket lists of the
+ * operating system. This is just a convention, also followed by other
+ * packages.
+ */
+ if (path[0] == '@')
+ {
+ unp->sun_path[0] = '\0';
+ aip->ai_addrlen = offsetof(struct sockaddr_un, sun_path) + strlen(path);
+ }
+
#ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
unp->sun_len = sizeof(struct sockaddr_un);
#endif
@@ -249,7 +264,14 @@ getnameinfo_unix(const struct sockaddr_un *sa, int salen,
if (service)
{
- ret = snprintf(service, servicelen, "%s", sa->sun_path);
+ /*
+ * Check whether it looks like an abstract socket, but it could also
+ * just be an empty string.
+ */
+ if (sa->sun_path[0] == '\0' && sa->sun_path[1] != '\0')
+ ret = snprintf(service, servicelen, "@%s", sa->sun_path + 1);
+ else
+ ret = snprintf(service, servicelen, "%s", sa->sun_path);
if (ret < 0 || ret >= servicelen)
return EAI_MEMORY;
}
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 781d86c8ef..cf967c3987 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -85,6 +85,15 @@ typedef struct
*/
#define UNIXSOCK_PATH_BUFLEN sizeof(((struct sockaddr_un *) NULL)->sun_path)
+/*
+ * A host that looks either like an absolute path or starts with @ is
+ * interpreted as a Unix-domain socket address.
+ */
+static inline bool
+is_unixsock_path(const char *path)
+{
+ return is_absolute_path(path) || path[0] == '@';
+}
/*
* These manipulate the frontend/backend protocol version number.
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index b0ca37c2ed..6aa7b41d2b 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1093,7 +1093,7 @@ connectOptions2(PGconn *conn)
{
ch->type = CHT_HOST_NAME;
#ifdef HAVE_UNIX_SOCKETS
- if (is_absolute_path(ch->host))
+ if (is_unixsock_path(ch->host))
ch->type = CHT_UNIX_SOCKET;
#endif
}
@@ -6945,7 +6945,7 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname,
/* 'localhost' matches pghost of '' or the default socket directory */
if (hostname == NULL || hostname[0] == '\0')
hostname = DefaultHost;
- else if (is_absolute_path(hostname))
+ else if (is_unixsock_path(hostname))
/*
* We should probably use canonicalize_path(), but then we have to
--
2.28.0
v2-0002-Make-error-hint-from-bind-failure-more-accurate.patchtext/plain; charset=UTF-8; name=v2-0002-Make-error-hint-from-bind-failure-more-accurate.patch; x-mac-creator=0; x-mac-type=0Download
From 49d48efe4cc4af558b06c474a0bec22316f4bf55 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 22 Oct 2020 08:47:52 +0200
Subject: [PATCH v2 2/2] Make error hint from bind() failure more accurate
The hint "Is another postmaster already running ..." should only be
printed for errors that are really about something else already using
the address. In other cases it is misleading.
Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
---
src/backend/libpq/pqcomm.c | 17 ++++++++++-------
1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 84568508c5..b8be7de70e 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -530,18 +530,21 @@ StreamServerPort(int family, const char *hostName, unsigned short portNumber,
err = bind(fd, addr->ai_addr, addr->ai_addrlen);
if (err < 0)
{
+ int saved_errno = errno;
+
ereport(LOG,
(errcode_for_socket_access(),
/* translator: first %s is IPv4, IPv6, or Unix */
errmsg("could not bind %s address \"%s\": %m",
familyDesc, addrDesc),
- (IS_AF_UNIX(addr->ai_family)) ?
- errhint("Is another postmaster already running on port %d?"
- " If not, remove socket file \"%s\" and retry.",
- (int) portNumber, service) :
- errhint("Is another postmaster already running on port %d?"
- " If not, wait a few seconds and retry.",
- (int) portNumber)));
+ saved_errno == EADDRINUSE ?
+ (IS_AF_UNIX(addr->ai_family) ?
+ errhint("Is another postmaster already running on port %d?"
+ " If not, remove socket file \"%s\" and retry.",
+ (int) portNumber, service) :
+ errhint("Is another postmaster already running on port %d?"
+ " If not, wait a few seconds and retry.",
+ (int) portNumber)) : 0));
closesocket(fd);
continue;
}
--
2.28.0
On Thu, Oct 22, 2020 at 09:03:49AM +0200, Peter Eisentraut wrote:
On 2020-10-09 09:28, Peter Eisentraut wrote:
During the discussion on Unix-domain sockets on Windows, someone pointed
out[0] abstract Unix-domain sockets. This is a variant of the normal
Unix-domain sockets that don't use the file system but a separate
"abstract" namespace. At the user interface, such sockets are
represented by names starting with "@". I took a look at this and it
wasn't hard to get working, so here is a patch. It's supposed to be
supported on Linux and Windows right now, but I haven't tested on Windows.
Yeah, peaking at the Windows docs, what you are trying to do here
should be supported (please note that I have not tested ). One
reference:
https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/
And then some extra patches for surrounding cleanup. During testing I
noticed that the bind() failure hint "Is another postmaster already
running ..." was shown in inappropriate situations, so I changed that to
only show for EADDRINUSE errors. (Maybe other error codes could be
appropriate, but I couldn't find any more.)And then looking for other uses of EADDRINUSE I found some dead
Windows-related code that can be cleaned up.This last piece has been committed.
+ <para>
+ A value that starts with <literal>@</literal> specifies that a
+ Unix-domain socket in the abstract namespace should be created
+ (currently supported on Linux and Windows). In that case, this value
+ does not specify a <quote>directory</quote> but a prefix from which
+ the actual socket name is computed in the same manner as for the
+ file-system namespace. While the abstract socket name prefix can be
+ chosen freely, since it is not a file-system location, the convention
+ is to nonetheless use file-system-like values such as
+ <literal>@/tmp</literal>.
+ </para>As abstract namespaces don't have permissions, anyone knowing the name
of the path, which should be unique, can have an access to the server.
Do you think that the documentation should warn the user about that?
This feature is about easing the management part of the socket paths
while throwing away the security aspect of it.
When attempting to start a server that listens to the same port and
uses the same abstract path, the second server started still shows
a hint referring to a file that does not exist:
LOG: could not bind Unix address "@tmp/.s.PGSQL.5432": Address already
in use
HINT: Is another postmaster already running on port 5432? If not,
remove socket file "@tmp/.s.PGSQL.5432" and retry.
Instead of showing paths with at signs, wouldn't it be better to
mention it is an abstract socket address?
I am not sure that 0002 is an improvement. It would be more readable
to move the part choosing what hint is adapted into a first block that
selects the hint string rather than have the whole thing in a single
elog() call.
--
Michael
On 2020-11-09 07:08, Michael Paquier wrote:
As abstract namespaces don't have permissions, anyone knowing the name
of the path, which should be unique, can have an access to the server.
Do you think that the documentation should warn the user about that?
This feature is about easing the management part of the socket paths
while throwing away the security aspect of it.
We could modify the documentation further. But note that the
traditional way of putting the socket into /tmp has the same properties,
so this shouldn't be a huge shock.
When attempting to start a server that listens to the same port and
uses the same abstract path, the second server started still shows
a hint referring to a file that does not exist:
LOG: could not bind Unix address "@tmp/.s.PGSQL.5432": Address already
in use
HINT: Is another postmaster already running on port 5432? If not,
remove socket file "@tmp/.s.PGSQL.5432" and retry.Instead of showing paths with at signs, wouldn't it be better to
mention it is an abstract socket address?
The @ is the standard way of representing this in the user interface and
the configuration, so it seems sensible to me that way.
I am not sure that 0002 is an improvement. It would be more readable
to move the part choosing what hint is adapted into a first block that
selects the hint string rather than have the whole thing in a single
elog() call.
Can you sketch how you would structure this? I realize it's not very
elegant, but I couldn't come up with a better way that didn't involve
having to duplicate some of the error messages into multiple branches.
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
On 11/9/20 9:04 AM, Peter Eisentraut wrote:
On 2020-11-09 07:08, Michael Paquier wrote:
As abstract namespaces don't have permissions, anyone knowing the name
of the path, which should be unique, can have an access to the server.
Do you think that the documentation should warn the user about that?
This feature is about easing the management part of the socket paths
while throwing away the security aspect of it.We could modify the documentation further.� But note that the
traditional way of putting the socket into /tmp has the same properties,
so this shouldn't be a huge shock.
One issue with them is that they interact differently with kernel
namespaces than normal unix sockets do. Abstract sockets are handled by
the network namespaces, and not the file system namespaces. But I am not
sure that this is our job to document.
Andreas
On Mon, Nov 09, 2020 at 09:04:24AM +0100, Peter Eisentraut wrote:
On 2020-11-09 07:08, Michael Paquier wrote:
The @ is the standard way of representing this in the user interface and the
configuration, so it seems sensible to me that way.
Ok.
Can you sketch how you would structure this? I realize it's not very
elegant, but I couldn't come up with a better way that didn't involve having
to duplicate some of the error messages into multiple branches.
I think that I would use a StringInfo to build each sentence of the
hint separately. The first sentence, "Is another postmaster already
running on port %d?" is already known. Then the second sentence could
be built depending on the two other conditions. FWIW, I think that it
is confusing to mention in the hint to remove a socket file that
cannot be removed.
--
Michael
On 2020-11-10 07:24, Michael Paquier wrote:
Can you sketch how you would structure this? I realize it's not very
elegant, but I couldn't come up with a better way that didn't involve having
to duplicate some of the error messages into multiple branches.I think that I would use a StringInfo to build each sentence of the
hint separately. The first sentence, "Is another postmaster already
running on port %d?" is already known. Then the second sentence could
be built depending on the two other conditions.
I'm not sure concatenating sentences like that is okay for translatability.
FWIW, I think that it
is confusing to mention in the hint to remove a socket file that
cannot be removed.
Thinking about it further, I think the hint in the Unix-domain socket
case is bogus. A socket in the file-system namespace never reports
EADDRINUSE anyway, it just overwrites the file. For sockets in the
abstract namespace, you can get this error, but of course there is no
file to remove.
Perhaps we should change the hint in both the Unix and the IP cases to:
"Is another postmaster already running at this address?"
(This also resolves the confusing reference to "port" in the Unix case.)
Or we just drop the hint in the Unix case. The primary error message is
clear enough.
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
On Wed, Nov 11, 2020 at 01:39:17PM +0100, Peter Eisentraut wrote:
Thinking about it further, I think the hint in the Unix-domain socket case
is bogus. A socket in the file-system namespace never reports EADDRINUSE
anyway, it just overwrites the file. For sockets in the abstract namespace,
you can get this error, but of course there is no file to remove.Perhaps we should change the hint in both the Unix and the IP cases to:
"Is another postmaster already running at this address?"
(This also resolves the confusing reference to "port" in the Unix case.)
Er, it is perfectly possible for two postmasters to use the same unix
socket path, abstract or not, as long as they listen to different
ports (all nodes in a single TAP test do that for example). So we
should keep a reference to the port used in the log message, no?
Or we just drop the hint in the Unix case. The primary error message is
clear enough.
Dropping the hint for the abstract case sounds fine to me.
--
Michael
On 2020-11-12 08:12, Michael Paquier wrote:
On Wed, Nov 11, 2020 at 01:39:17PM +0100, Peter Eisentraut wrote:
Thinking about it further, I think the hint in the Unix-domain socket case
is bogus. A socket in the file-system namespace never reports EADDRINUSE
anyway, it just overwrites the file. For sockets in the abstract namespace,
you can get this error, but of course there is no file to remove.Perhaps we should change the hint in both the Unix and the IP cases to:
"Is another postmaster already running at this address?"
(This also resolves the confusing reference to "port" in the Unix case.)Er, it is perfectly possible for two postmasters to use the same unix
socket path, abstract or not, as long as they listen to different
ports (all nodes in a single TAP test do that for example). So we
should keep a reference to the port used in the log message, no?
"Port" is not a real thing for Unix-domain sockets, it's just something
we use internally and append to the socket file. The error message is
currently something like
ERROR: could not bind Unix address "/tmp/.s.PGSQL.5432": Address
already in use
HINT: Is another postmaster already running on port 5432? If not,
remove socket file "/tmp/.s.PGSQL.5432" and retry.
So the mention of the "port" doesn't really add any information here and
just introduces new terminology that isn't really relevant.
My idea is to change the message to:
ERROR: could not bind Unix address "/tmp/.s.PGSQL.5432": Address
already in use
HINT: Is another postmaster already running at this address?
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
On Tue, Nov 17, 2020 at 11:18:12PM +0100, Peter Eisentraut wrote:
So the mention of the "port" doesn't really add any information here and
just introduces new terminology that isn't really relevant.My idea is to change the message to:
ERROR: could not bind Unix address "/tmp/.s.PGSQL.5432": Address already in
use
HINT: Is another postmaster already running at this address?
Are you saying that you would remove the hint telling to remove the
socket file even for the case of non-abstract files? For abstract
paths, this makes sense. For both, removing the "port" part is indeed
a good idea as long as you keep around the full socket file name.
--
Michael
On Fri, Oct 9, 2020 at 3:28 PM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
During the discussion on Unix-domain sockets on Windows, someone pointed
out[0] abstract Unix-domain sockets.
This reminds me on a somewhat random note that SSPI mode authentication
should work out of the box for unix domain sockets on Windows.
The main reason we probably can't use it as a default is that SSPI isn't
easy to implement for pure language drivers, it requires Windows API calls
to interact with the windows auth services. It's a pain in JDBC for example.
On Tue, Nov 17, 2020 at 7:00 PM Michael Paquier <michael@paquier.xyz> wrote:
On Tue, Nov 17, 2020 at 11:18:12PM +0100, Peter Eisentraut wrote:
So the mention of the "port" doesn't really add any information here and
just introduces new terminology that isn't really relevant.My idea is to change the message to:
ERROR: could not bind Unix address "/tmp/.s.PGSQL.5432": Address
already in
use
HINT: Is another postmaster already running at this address?Are you saying that you would remove the hint telling to remove the
socket file even for the case of non-abstract files? For abstract
paths, this makes sense. For both, removing the "port" part is indeed
a good idea as long as you keep around the full socket file name.
(resending to the list)
Given that "port" is a postgresql.conf setting its use here (and elsewhere)
should be taken to mean the value of that specific variable. To that end,
I find the current description of port to be lacking - it should mention
its usage as a qualifier when dealing with unix socket files (in addition
to the existing wording under unix_socket_directories).
If we are going to debate semantics here "bind unix address" doesn't seem
correct. could not create Unix socket file /tmp/.s.PGSQL.5432, it already
exists.
The hint would be better written: Is another postmaster running with
unix_socket_directories = /tmp and port = 5432? If not, remove the unix
socket file /tmp/.s.PGSQL.5432 and retry.
I don't see much benefit in trying to share logic/wording between the
various messages and hints for the different ways the server can establish
communication points.
I agree that there isn't a useful hint for the abstract case as it
shouldn't happen unless there is indeed another running instance with the
same configuration. Though a hint similar to the above, but without the
"remove and retry" bit, probably wouldn't hurt.
David J.
On 2020-11-18 04:35, David G. Johnston wrote:
Given that "port" is a postgresql.conf setting its use here (and
elsewhere) should be taken to mean the value of that specific variable.
To that end, I find the current description of port to be lacking - it
should mention its usage as a qualifier when dealing with unix socket
files (in addition to the existing wording under unix_socket_directories).If we are going to debate semantics here "bind unix address" doesn't
seem correct. could not create Unix socket file /tmp/.s.PGSQL.5432, it
already exists.The hint would be better written: Is another postmaster running with
unix_socket_directories = /tmp and port = 5432? If not, remove the unix
socket file /tmp/.s.PGSQL.5432 and retry.I don't see much benefit in trying to share logic/wording between the
various messages and hints for the different ways the server can
establish communication points.I agree that there isn't a useful hint for the abstract case as it
shouldn't happen unless there is indeed another running instance with
the same configuration. Though a hint similar to the above, but without
the "remove and retry" bit, probably wouldn't hurt.
I think we are getting a bit sidetracked here with the message wording.
The reason I looked at this was that "remove socket file and retry" is
never an appropriate action with abstract sockets. And on further
analysis, it is never an appropriate action with any Unix-domain socket
(because with file system namespace sockets, you never get an
EADDRINUSE, so it's dead code). So my proposal here is to just delete
that line from the hint and leave the rest the same. There could be
value in further refining and rephrasing this, but it ought to be a
separate thread.
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
Attachments:
v3-0001-Add-support-for-abstract-Unix-domain-sockets.patchtext/plain; charset=UTF-8; name=v3-0001-Add-support-for-abstract-Unix-domain-sockets.patch; x-mac-creator=0; x-mac-type=0Download
From a4967dd79e490390bc148d3bae92452ba4a34c9c Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 22 Oct 2020 08:47:52 +0200
Subject: [PATCH v3 1/2] Add support for abstract Unix-domain sockets
This is a variant of the normal Unix-domain sockets that don't use the
file system but a separate "abstract" namespace. At the user
interface, such sockets are represented by names starting with "@".
Supported on Linux and Windows right now.
Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
---
doc/src/sgml/config.sgml | 24 +++++++++++++++++++++++-
doc/src/sgml/libpq.sgml | 5 ++++-
src/backend/libpq/pqcomm.c | 8 ++++++++
src/bin/psql/command.c | 15 +++++----------
src/bin/psql/prompt.c | 3 ++-
src/common/ip.c | 24 +++++++++++++++++++++++-
src/include/libpq/pqcomm.h | 9 +++++++++
src/interfaces/libpq/fe-connect.c | 4 ++--
8 files changed, 76 insertions(+), 16 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index a632cf98ba..d05045fb20 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -749,6 +749,21 @@ <title>Connection Settings</title>
An empty value
specifies not listening on any Unix-domain sockets, in which case
only TCP/IP sockets can be used to connect to the server.
+ </para>
+
+ <para>
+ A value that starts with <literal>@</literal> specifies that a
+ Unix-domain socket in the abstract namespace should be created
+ (currently supported on Linux and Windows). In that case, this value
+ does not specify a <quote>directory</quote> but a prefix from which
+ the actual socket name is computed in the same manner as for the
+ file-system namespace. While the abstract socket name prefix can be
+ chosen freely, since it is not a file-system location, the convention
+ is to nonetheless use file-system-like values such as
+ <literal>@/tmp</literal>.
+ </para>
+
+ <para>
The default value is normally
<filename>/tmp</filename>, but that can be changed at build time.
On Windows, the default is empty, which means no Unix-domain socket is
@@ -763,6 +778,7 @@ <title>Connection Settings</title>
named <literal>.s.PGSQL.<replaceable>nnnn</replaceable>.lock</literal> will be
created in each of the <varname>unix_socket_directories</varname> directories.
Neither file should ever be removed manually.
+ For sockets in the abstract namespace, no lock file is created.
</para>
</listitem>
</varlistentry>
@@ -787,7 +803,8 @@ <title>Connection Settings</title>
<para>
This parameter is not supported on Windows. Any setting will be
- ignored.
+ ignored. Also, sockets in the abstract namespace have no file owner,
+ so this setting is also ignored in that case.
</para>
</listitem>
</varlistentry>
@@ -834,6 +851,11 @@ <title>Connection Settings</title>
similar effect by pointing <varname>unix_socket_directories</varname> to a
directory having search permission limited to the desired audience.
</para>
+
+ <para>
+ Sockets in the abstract namespace have no file permissions, so this
+ setting is also ignored in that case.
+ </para>
</listitem>
</varlistentry>
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index 9d4b6ab4a8..06bd412044 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -1031,7 +1031,10 @@ <title>Parameter Key Words</title>
communication; the value is the name of the directory in which the
socket file is stored. (On Unix, an absolute path name begins with a
slash. On Windows, paths starting with drive letters are also
- recognized.) The default behavior when <literal>host</literal> is not
+ recognized.) If the host name starts with <literal>@</literal>, it is
+ taken as a Unix-domain socket in the abstract namespace (currently
+ supported on Linux and Windows).
+ The default behavior when <literal>host</literal> is not
specified, or is empty, is to connect to a Unix-domain
socket<indexterm><primary>Unix domain socket</primary></indexterm> in
<filename>/tmp</filename> (or whatever socket directory was specified
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index ac986c0505..84568508c5 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -611,6 +611,10 @@ StreamServerPort(int family, const char *hostName, unsigned short portNumber,
static int
Lock_AF_UNIX(const char *unixSocketDir, const char *unixSocketPath)
{
+ /* no lock file for abstract sockets */
+ if (unixSocketPath[0] == '@')
+ return STATUS_OK;
+
/*
* Grab an interlock file associated with the socket file.
*
@@ -642,6 +646,10 @@ Lock_AF_UNIX(const char *unixSocketDir, const char *unixSocketPath)
static int
Setup_AF_UNIX(const char *sock_path)
{
+ /* no file system permissions for abstract sockets */
+ if (sock_path[0] == '@')
+ return STATUS_OK;
+
/*
* Fix socket ownership/permission if requested. Note we must do this
* before we listen() to avoid a window where unwanted connections could
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index c7a83d5dfc..55b349d55a 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -37,6 +37,7 @@
#include "input.h"
#include "large_obj.h"
#include "libpq-fe.h"
+#include "libpq/pqcomm.h"
#include "mainloop.h"
#include "portability/instr_time.h"
#include "pqexpbuffer.h"
@@ -604,12 +605,9 @@ exec_command_conninfo(PsqlScanState scan_state, bool active_branch)
char *host = PQhost(pset.db);
char *hostaddr = PQhostaddr(pset.db);
- /*
- * If the host is an absolute path, the connection is via socket
- * unless overridden by hostaddr
- */
- if (is_absolute_path(host))
+ if (is_unixsock_path(host))
{
+ /* hostaddr overrides host */
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));
@@ -3407,12 +3405,9 @@ do_connect(enum trivalue reuse_previous_specification,
char *host = PQhost(pset.db);
char *hostaddr = PQhostaddr(pset.db);
- /*
- * If the host is an absolute path, the connection is via socket
- * unless overridden by hostaddr
- */
- if (is_absolute_path(host))
+ if (is_unixsock_path(host))
{
+ /* hostaddr overrides host */
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));
diff --git a/src/bin/psql/prompt.c b/src/bin/psql/prompt.c
index ef503ec41b..f42c3dfc74 100644
--- a/src/bin/psql/prompt.c
+++ b/src/bin/psql/prompt.c
@@ -15,6 +15,7 @@
#include "common.h"
#include "common/string.h"
#include "input.h"
+#include "libpq/pqcomm.h"
#include "prompt.h"
#include "settings.h"
@@ -136,7 +137,7 @@ get_prompt(promptStatus_t status, ConditionalStack cstack)
const char *host = PQhost(pset.db);
/* INET socket */
- if (host && host[0] && !is_absolute_path(host))
+ if (host && host[0] && !is_unixsock_path(host))
{
strlcpy(buf, host, sizeof(buf));
if (*p == 'm')
diff --git a/src/common/ip.c b/src/common/ip.c
index 69fcca8479..bcc779e00c 100644
--- a/src/common/ip.c
+++ b/src/common/ip.c
@@ -217,6 +217,21 @@ getaddrinfo_unix(const char *path, const struct addrinfo *hintsp,
strcpy(unp->sun_path, path);
+ /*
+ * If the supplied path starts with @, replace that with a zero byte for
+ * the internal representation. In that mode, the entire sun_path is the
+ * address, including trailing zero bytes. But we set the address length
+ * to only include the length of the original string. That way the
+ * trailing zero bytes won't show up in any network or socket lists of the
+ * operating system. This is just a convention, also followed by other
+ * packages.
+ */
+ if (path[0] == '@')
+ {
+ unp->sun_path[0] = '\0';
+ aip->ai_addrlen = offsetof(struct sockaddr_un, sun_path) + strlen(path);
+ }
+
#ifdef HAVE_STRUCT_SOCKADDR_STORAGE_SS_LEN
unp->sun_len = sizeof(struct sockaddr_un);
#endif
@@ -249,7 +264,14 @@ getnameinfo_unix(const struct sockaddr_un *sa, int salen,
if (service)
{
- ret = snprintf(service, servicelen, "%s", sa->sun_path);
+ /*
+ * Check whether it looks like an abstract socket, but it could also
+ * just be an empty string.
+ */
+ if (sa->sun_path[0] == '\0' && sa->sun_path[1] != '\0')
+ ret = snprintf(service, servicelen, "@%s", sa->sun_path + 1);
+ else
+ ret = snprintf(service, servicelen, "%s", sa->sun_path);
if (ret < 0 || ret >= servicelen)
return EAI_MEMORY;
}
diff --git a/src/include/libpq/pqcomm.h b/src/include/libpq/pqcomm.h
index 781d86c8ef..cf967c3987 100644
--- a/src/include/libpq/pqcomm.h
+++ b/src/include/libpq/pqcomm.h
@@ -85,6 +85,15 @@ typedef struct
*/
#define UNIXSOCK_PATH_BUFLEN sizeof(((struct sockaddr_un *) NULL)->sun_path)
+/*
+ * A host that looks either like an absolute path or starts with @ is
+ * interpreted as a Unix-domain socket address.
+ */
+static inline bool
+is_unixsock_path(const char *path)
+{
+ return is_absolute_path(path) || path[0] == '@';
+}
/*
* These manipulate the frontend/backend protocol version number.
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
index e7781d010f..7d04d3664e 100644
--- a/src/interfaces/libpq/fe-connect.c
+++ b/src/interfaces/libpq/fe-connect.c
@@ -1093,7 +1093,7 @@ connectOptions2(PGconn *conn)
{
ch->type = CHT_HOST_NAME;
#ifdef HAVE_UNIX_SOCKETS
- if (is_absolute_path(ch->host))
+ if (is_unixsock_path(ch->host))
ch->type = CHT_UNIX_SOCKET;
#endif
}
@@ -6945,7 +6945,7 @@ passwordFromFile(const char *hostname, const char *port, const char *dbname,
/* 'localhost' matches pghost of '' or the default socket directory */
if (hostname == NULL || hostname[0] == '\0')
hostname = DefaultHost;
- else if (is_absolute_path(hostname))
+ else if (is_unixsock_path(hostname))
/*
* We should probably use canonicalize_path(), but then we have to
base-commit: 16f96c74d48e65da23d28665103e2c4c9d3414cc
--
2.29.2
v3-0002-Make-error-hint-from-bind-failure-more-accurate.patchtext/plain; charset=UTF-8; name=v3-0002-Make-error-hint-from-bind-failure-more-accurate.patch; x-mac-creator=0; x-mac-type=0Download
From d996e9431adb50d0a45437d6aca14e8d7dcc7448 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Fri, 20 Nov 2020 15:59:18 +0100
Subject: [PATCH v3 2/2] Make error hint from bind() failure more accurate
The hint "Is another postmaster already running ..." should only be
printed for errors that are really about something else already using
the address. In other cases it is misleading. So only show that hint
if errno == EADDRINUSE.
Also, since Unix-domain sockets in the file-system namespace never
report EADDRINUSE for an existing file (they would just overwrite it),
the part of the hint saying "If not, remove socket file \"%s\" and
retry." can never happen, so remove it. Unix-domain sockets in the
abstract namespace can report EADDRINUSE, but in that case there is no
file to remove, so the hint doesn't work there either.
Discussion: https://www.postgresql.org/message-id/flat/6dee8574-b0ad-fc49-9c8c-2edc796f0033@2ndquadrant.com
---
src/backend/libpq/pqcomm.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/src/backend/libpq/pqcomm.c b/src/backend/libpq/pqcomm.c
index 84568508c5..0e4efc8c10 100644
--- a/src/backend/libpq/pqcomm.c
+++ b/src/backend/libpq/pqcomm.c
@@ -530,18 +530,20 @@ StreamServerPort(int family, const char *hostName, unsigned short portNumber,
err = bind(fd, addr->ai_addr, addr->ai_addrlen);
if (err < 0)
{
+ int saved_errno = errno;
+
ereport(LOG,
(errcode_for_socket_access(),
/* translator: first %s is IPv4, IPv6, or Unix */
errmsg("could not bind %s address \"%s\": %m",
familyDesc, addrDesc),
- (IS_AF_UNIX(addr->ai_family)) ?
- errhint("Is another postmaster already running on port %d?"
- " If not, remove socket file \"%s\" and retry.",
- (int) portNumber, service) :
- errhint("Is another postmaster already running on port %d?"
- " If not, wait a few seconds and retry.",
- (int) portNumber)));
+ saved_errno == EADDRINUSE ?
+ (IS_AF_UNIX(addr->ai_family) ?
+ errhint("Is another postmaster already running on port %d?",
+ (int) portNumber) :
+ errhint("Is another postmaster already running on port %d?"
+ " If not, wait a few seconds and retry.",
+ (int) portNumber)) : 0));
closesocket(fd);
continue;
}
--
2.29.2
On Friday, November 20, 2020, Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
On 2020-11-18 04:35, David G. Johnston wrote:
I agree that there isn't a useful hint for the abstract case as it
shouldn't happen unless there is indeed another running instance with the
same configuration. Though a hint similar to the above, but without the
"remove and retry" bit, probably wouldn't hurt.I think we are getting a bit sidetracked here with the message wording.
The reason I looked at this was that "remove socket file and retry" is
never an appropriate action with abstract sockets. And on further
analysis, it is never an appropriate action with any Unix-domain socket
(because with file system namespace sockets, you never get an EADDRINUSE,
so it's dead code). So my proposal here is to just delete that line from
the hint and leave the rest the same. There could be value in further
refining and rephrasing this, but it ought to be a separate thread.
If there is dead code there is an underlying problem to address/discover,
not just removing the dead code. In this case are we saying that a new
server won’t ever fail to start because the socket file exists but instead
will just clobber the file with its own? Because given that error, and a
server process that failed to clean up after itself, the correction to take
would indeed seem to be to manually remove the file as the hint says. IOW,
fix the code, not the message?
David J.
On 2020-11-20 18:23, David G. Johnston wrote:
If there is dead code there is an underlying problem to
address/discover, not just removing the dead code. In this case are we
saying that a new server won’t ever fail to start because the socket
file exists but instead will just clobber the file with its own?
Yes. (In practice, there will be an error with respect to the lock file
before you even get to that question, but that is different code elsewhere.)
Because given that error, and a server process that failed to clean up
after itself, the correction to take would indeed seem to be to manually
remove the file as the hint says. IOW, fix the code, not the message?
I don't understand that.
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
On Mon, Nov 23, 2020 at 6:50 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
On 2020-11-20 18:23, David G. Johnston wrote:
If there is dead code there is an underlying problem to
address/discover, not just removing the dead code. In this case are we
saying that a new server won’t ever fail to start because the socket
file exists but instead will just clobber the file with its own?Yes. (In practice, there will be an error with respect to the lock file
before you even get to that question, but that is different code
elsewhere.)Because given that error, and a server process that failed to clean up
after itself, the correction to take would indeed seem to be to manually
remove the file as the hint says. IOW, fix the code, not the message?I don't understand that.
So presently there is no functioning code to prevent two PostgreSQL
instances from using the same socket so long as they do not also use the
same data directory? We only handle the case of an unclean crash - where
the pid and socket are both left behind - having the system tell the user
to remove the pid lock file but then auto-replacing the socket (I was
conflating the behavior with the pid lock file and the socket file).
I would expect that we handle port misconfiguration also, by not
auto-replacing the socket and instead have the existing error message (with
modified hint) remain behind. This provides behavior consistent with TCP
port binding. Or is it the case that we always attempt to bind the TCP/IP
port, regardless of the presence of a socket file, in which case the
failure for port binding does cover the socket situation as well? If this
is the case, pointing that out in [1]https://www.postgresql.org/docs/13/server-start.html#SERVER-START-FAILURES and a code comment, while removing
that particular error as "dead code", would work.
David J.
[1]: https://www.postgresql.org/docs/13/server-start.html#SERVER-START-FAILURES
https://www.postgresql.org/docs/13/server-start.html#SERVER-START-FAILURES
On Fri, Nov 20, 2020 at 04:06:43PM +0100, Peter Eisentraut wrote:
I think we are getting a bit sidetracked here with the message wording. The
reason I looked at this was that "remove socket file and retry" is never an
appropriate action with abstract sockets. And on further analysis, it is
never an appropriate action with any Unix-domain socket (because with file
system namespace sockets, you never get an EADDRINUSE, so it's dead code).
So my proposal here is to just delete that line from the hint and leave the
rest the same.
Reading again this thread, +1 on that.
--
Michael
On Mon, Nov 23, 2020 at 9:00 AM David G. Johnston <
david.g.johnston@gmail.com> wrote:
Or is it the case that we always attempt to bind the TCP/IP port,
regardless of the presence of a socket file, in which case the failure for
port binding does cover the socket situation as well?
This cannot always be the case since the listened-to IP address matters.
I think the socket file error message hint is appropriate. I'd consider it
a bug if that code is effectively unreachable (the fact that the hint
exists supports this conclusion). If we add "abstract unix sockets" where
we likewise prevent two servers from listening on the same channel, the
absence of such a check for the socket file is even more unexpected. At
minimum we should at least declare whether we will even try and whether
such a socket file check is best effort or simply generally reliable.
David J.
On 2020-11-23 17:00, David G. Johnston wrote:
So presently there is no functioning code to prevent two PostgreSQL
instances from using the same socket so long as they do not also use the
same data directory? We only handle the case of an unclean crash -
where the pid and socket are both left behind - having the system tell
the user to remove the pid lock file but then auto-replacing the socket
(I was conflating the behavior with the pid lock file and the socket file).I would expect that we handle port misconfiguration also, by not
auto-replacing the socket and instead have the existing error message
(with modified hint) remain behind. This provides behavior consistent
with TCP port binding. Or is it the case that we always attempt to bind
the TCP/IP port, regardless of the presence of a socket file, in which
case the failure for port binding does cover the socket situation as
well? If this is the case, pointing that out in [1] and a code comment,
while removing that particular error as "dead code", would work.
We're subject to whatever the kernel behavior is. If the kernel doesn't
report address conflicts for Unix-domain sockets, then we can't do
anything about that. Having an error message ready in case the kernel
does report such an error is not useful if it never does.
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
On Tue, Nov 24, 2020 at 8:45 AM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:
We're subject to whatever the kernel behavior is. If the kernel doesn't
report address conflicts for Unix-domain sockets, then we can't do
anything about that. Having an error message ready in case the kernel
does report such an error is not useful if it never does.
It's a file, we can check for its existence in user-space.
David J.
On 2020-11-24 02:57, Michael Paquier wrote:
On Fri, Nov 20, 2020 at 04:06:43PM +0100, Peter Eisentraut wrote:
I think we are getting a bit sidetracked here with the message wording. The
reason I looked at this was that "remove socket file and retry" is never an
appropriate action with abstract sockets. And on further analysis, it is
never an appropriate action with any Unix-domain socket (because with file
system namespace sockets, you never get an EADDRINUSE, so it's dead code).
So my proposal here is to just delete that line from the hint and leave the
rest the same.Reading again this thread, +1 on that.
committed, thanks
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/
On 2020-11-24 16:49, David G. Johnston wrote:
On Tue, Nov 24, 2020 at 8:45 AM Peter Eisentraut
<peter.eisentraut@2ndquadrant.com
<mailto:peter.eisentraut@2ndquadrant.com>> wrote:We're subject to whatever the kernel behavior is. If the kernel
doesn't
report address conflicts for Unix-domain sockets, then we can't do
anything about that. Having an error message ready in case the kernel
does report such an error is not useful if it never does.It's a file, we can check for its existence in user-space.
But not without race conditions. That's why we have the separate lock
file, so we can do this properly.
Also, even if one were to add code to check the file existence first,
this would be separate code and would not affect the behavior of the
bind() call that we are discussing here.
--
Peter Eisentraut
2ndQuadrant, an EDB company
https://www.2ndquadrant.com/