From: Nathan Bossart <nathandbossart@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
Cc: Bruce Momjian <bruce@momjian.us>
Cc: adam@labkey.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18711: Attempting a connection with a database name longer than 63 characters now fails
Date: Wed, 27 Nov 2024 13:34:22 -0600
Message-ID: <Z0d0PqBQGQQSlxB3@nathan> (raw)
In-Reply-To: <1896106.1732731438@sss.pgh.pa.us>
References: <665338.1732306066@sss.pgh.pa.us>
<Z0Dmw1ryKH8bEkyo@nathan>
<Z0c9XVU6aTd8neRr@nathan>
<1865227.1732723416@sss.pgh.pa.us>
<Z0dKd7bvo3D2NdLt@ip-10-97-1-34.eu-west-3.compute.internal>
<Z0dNyskuXOCM-dOz@nathan>
<Z0daSnNI8FrrSvhW@ip-10-97-1-34.eu-west-3.compute.internal>
<Z0db25rMTSFrK-FN@nathan>
<Z0de/7/6wgGY074a@ip-10-97-1-34.eu-west-3.compute.internal>
<1896106.1732731438@sss.pgh.pa.us>
On Wed, Nov 27, 2024 at 01:17:18PM -0500, Tom Lane wrote:
> I have an idea that might salvage something from what we've just
> been doing. The biggest single problem with the old behavior,
> AIUI, was the possibility of sending back encoding-corrupt names
> in connection failure reports. How about something like:
>
> * Don't truncate the names on sight (so we keep 562bee0fc1).
>
> * Truncate the names at NAMEDATALEN-1 just before catalog lookup
> (this restores the old lookup behavior, solving the current report).
>
> * In lookup-failure messages, be sure to report the un-truncated
> name (avoiding the bad-encoding problem).
>
> * Copy the correctly-truncated names from the catalog to any
> persistent storage such as MyProcPort, so that those are used
> for all post-connection purposes.
One possible issue with truncating the names after-the-fact is that hooks
like ClientAuthentication_hook will have already been called with the
untruncated names. If it's just the connection failure reports we are
worried about, then maybe we should save the "raw" names to MyProcPort
before truncating and be sure to use those in the log messages.
I attached a rough sketch of what I'm imagining. If we aren't thrilled
about adding new fields to "struct Port" in a minor release, we could just
do a straight revert on the back-branches and add that part to v18.
--
nathan
Attachments:save_untruncated_names.patch (text/plain, 5.0K) downloaddiff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.cindex 47e8c91606..d81c3eeba9 100644--- a/src/backend/libpq/auth.c+++ b/src/backend/libpq/auth.c@@ -320,7 +320,7 @@ auth_failed(Port *port, int status, const char *logdetail)
ereport(FATAL,
(errcode(errcode_return),
- errmsg(errstr, port->user_name),+ errmsg(errstr, port->user_name_raw),
logdetail ? errdetail_log("%s", logdetail) : 0));
/* doesn't return */
@@ -459,14 +459,14 @@ ClientAuthentication(Port *port)
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
/* translator: last %s describes encryption state */
errmsg("pg_hba.conf rejects replication connection for host \"%s\", user \"%s\", %s",
- hostinfo, port->user_name,+ hostinfo, port->user_name_raw,
encryption_state)));
else
ereport(FATAL,
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
/* translator: last %s describes encryption state */
errmsg("pg_hba.conf rejects connection for host \"%s\", user \"%s\", database \"%s\", %s",
- hostinfo, port->user_name,+ hostinfo, port->user_name_raw,
port->database_name,
encryption_state)));
break;
@@ -526,7 +526,7 @@ ClientAuthentication(Port *port)
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
/* translator: last %s describes encryption state */
errmsg("no pg_hba.conf entry for replication connection from host \"%s\", user \"%s\", %s",
- hostinfo, port->user_name,+ hostinfo, port->user_name_raw,
encryption_state),
HOSTNAME_LOOKUP_DETAIL(port)));
else
@@ -534,7 +534,7 @@ ClientAuthentication(Port *port)
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
/* translator: last %s describes encryption state */
errmsg("no pg_hba.conf entry for host \"%s\", user \"%s\", database \"%s\", %s",
- hostinfo, port->user_name,+ hostinfo, port->user_name_raw,
port->database_name,
encryption_state),
HOSTNAME_LOOKUP_DETAIL(port)));
diff --git a/src/backend/tcop/backend_startup.c b/src/backend/tcop/backend_startup.cindex 2a96c81f92..3b60559ae9 100644--- a/src/backend/tcop/backend_startup.c+++ b/src/backend/tcop/backend_startup.c@@ -830,6 +830,22 @@ ProcessStartupPacket(Port *port, bool ssl_done, bool gss_done)
if (port->database_name == NULL || port->database_name[0] == '\0')
port->database_name = pstrdup(port->user_name);
+ /*+ * Prior to truncating, save the raw database and user names for log+ * messages.+ */+ port->database_name_raw = pstrdup(port->database_name);+ port->user_name_raw = pstrdup(port->user_name);++ /*+ * Truncate given database and user names to length of a Postgres name.+ * This avoids lookup failures when overlength names are given.+ */+ if (strlen(port->database_name) >= NAMEDATALEN)+ port->database_name[NAMEDATALEN - 1] = '\0';+ if (strlen(port->user_name) >= NAMEDATALEN)+ port->user_name[NAMEDATALEN - 1] = '\0';+
if (am_walsender)
MyBackendType = B_WAL_SENDER;
else
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.cindex 3b7b2ebec0..7e3740fa0f 100644--- a/src/backend/utils/init/miscinit.c+++ b/src/backend/utils/init/miscinit.c@@ -798,7 +798,8 @@ InitializeSessionUserId(const char *rolename, Oid roleid, bool bypass_login_chec
return;
ereport(FATAL,
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
- errmsg("role \"%s\" does not exist", rolename)));+ errmsg("role \"%s\" does not exist",+ MyProcPort ? MyProcPort->user_name_raw : rolename)));
}
}
else
diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.cindex 5b657a3f13..4f949a8f2d 100644--- a/src/backend/utils/init/postinit.c+++ b/src/backend/utils/init/postinit.c@@ -1008,7 +1008,8 @@ InitPostgres(const char *in_dbname, Oid dboid,
if (!HeapTupleIsValid(tuple))
ereport(FATAL,
(errcode(ERRCODE_UNDEFINED_DATABASE),
- errmsg("database \"%s\" does not exist", in_dbname)));+ errmsg("database \"%s\" does not exist",+ MyProcPort ? MyProcPort->database_name_raw : in_dbname)));
dbform = (Form_pg_database) GETSTRUCT(tuple);
dboid = dbform->oid;
}
@@ -1072,7 +1073,8 @@ InitPostgres(const char *in_dbname, Oid dboid,
if (in_dbname)
ereport(FATAL,
(errcode(ERRCODE_UNDEFINED_DATABASE),
- errmsg("database \"%s\" does not exist", in_dbname),+ errmsg("database \"%s\" does not exist",+ MyProcPort ? MyProcPort->database_name_raw : in_dbname),
errdetail("It seems to have just been dropped or renamed.")));
else
ereport(FATAL,
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.hindex 9109b2c334..9031cabfc7 100644--- a/src/include/libpq/libpq-be.h+++ b/src/include/libpq/libpq-be.h@@ -232,6 +232,9 @@ typedef struct Port
char *raw_buf;
ssize_t raw_buf_consumed,
raw_buf_remaining;
++ char *database_name_raw;+ char *user_name_raw;
} Port;
/*
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: nathandbossart@gmail.com, tgl@sss.pgh.pa.us, bertranddrouvot.pg@gmail.com, bruce@momjian.us, adam@labkey.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18711: Attempting a connection with a database name longer than 63 characters now fails
In-Reply-To: <Z0d0PqBQGQQSlxB3@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox