Re: BUG #18711: Attempting a connection with a database name longer than 63 characters now fails - Mailing list pgsql-bugs

Bertrand Drouvot <bertranddrouvot.pg@gmail.com> writes:
> What about something like in v3 attached? (I did a few tests and that seems to
> work as expected).

After more thought I don't really like the idea of failing if there
are multiple matches.  It means that we might fail in cases where
pre-v17 worked fine (because NAMEDATALEN-1 was accidentally the
right truncation point).  ISTM the entire point of this patch is to
restore the pre-v17 behavior as much as possible, so that seems like
the wrong outcome.

So that means we could do something like the attached.  (There's
room for argument about which error messages in InitPostgres
should use in_dbname versus the truncated name, but I chose to
use in_dbname for the two "does not exist" reports.)

I didn't try to write a test case, but we probably should have one.

            regards, tom lane

diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c
index 5b657a3f13..62be5f2d20 100644
--- a/src/backend/utils/init/postinit.c
+++ b/src/backend/utils/init/postinit.c
@@ -68,6 +68,7 @@
 #include "utils/timeout.h"

 static HeapTuple GetDatabaseTuple(const char *dbname);
+static HeapTuple GetDatabaseTupleInternal(Relation relation, const char *dbname);
 static HeapTuple GetDatabaseTupleByOid(Oid dboid);
 static void PerformAuthentication(Port *port);
 static void CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connections);
@@ -97,12 +98,79 @@ static void process_settings(Oid databaseid, Oid roleid);
  * descriptors for both pg_database and its indexes from the shared relcache
  * cache file, and so we can do an indexscan.  criticalSharedRelcachesBuilt
  * tells whether we got the cached descriptors.
+ *
+ * This function also deals with the trickier-than-it-sounds issue of
+ * truncating an overlength incoming database name.  We do not know which
+ * encoding the name is in, so we can't apply pg_encoding_mbcliplen() as
+ * CREATE DATABASE would have done.  We use the heuristic of truncating one
+ * byte at a time until we find a match in pg_database, relying on the
+ * assumption that the name as stored in pg_database is valid according to
+ * some server-side encoding.  This lets us constrain the search using the
+ * fact that all bytes of a multibyte character have their high bit set.
  */
 static HeapTuple
 GetDatabaseTuple(const char *dbname)
 {
-    HeapTuple    tuple;
+    HeapTuple    tuple = NULL;
     Relation    relation;
+    char        tname[NAMEDATALEN];
+
+    relation = table_open(DatabaseRelationId, AccessShareLock);
+
+    if (strlen(dbname) < NAMEDATALEN)
+    {
+        /* Typical, easy case: no truncation needed */
+        tuple = GetDatabaseTupleInternal(relation, dbname);
+    }
+    else if (!IS_HIGHBIT_SET(dbname[NAMEDATALEN - 1]) ||
+             !IS_HIGHBIT_SET(dbname[NAMEDATALEN - 2]))
+    {
+        /* Also easy: truncation at NAMEDATALEN - 1 cannot break an MB char */
+        memcpy(tname, dbname, sizeof(tname));
+        tname[NAMEDATALEN - 1] = '\0';
+        tuple = GetDatabaseTupleInternal(relation, tname);
+    }
+    else
+    {
+        /* Hard case: try the successive-truncation heuristic */
+        memcpy(tname, dbname, sizeof(tname));
+        for (int i = NAMEDATALEN - 1;
+             i >= NAMEDATALEN - MAX_MULTIBYTE_CHAR_LEN;
+             i--)
+        {
+            /*
+             * If the byte we're about to remove is ASCII, it cannot be part
+             * of a multibyte character, so truncating it would be incorrect.
+             */
+            if (!IS_HIGHBIT_SET(tname[i]))
+                break;
+            tname[i] = '\0';
+
+            tuple = GetDatabaseTupleInternal(relation, tname);
+
+            /*
+             * Take the first match.  It's theoretically possible that we
+             * could find more than one match if we continued to try shorter
+             * truncations (which is what makes this a heuristic).  This is
+             * unlikely though.  We considered failing if there are multiple
+             * candidate matches, but that cure seems worse than the disease:
+             * it might reject cases that worked fine before v17.
+             */
+            if (HeapTupleIsValid(tuple))
+                break;
+        }
+    }
+
+    table_close(relation, AccessShareLock);
+
+    return tuple;
+}
+
+/* One tuple fetch for GetDatabaseTuple */
+static HeapTuple
+GetDatabaseTupleInternal(Relation relation, const char *dbname)
+{
+    HeapTuple    tuple;
     SysScanDesc scan;
     ScanKeyData key[1];

@@ -115,11 +183,10 @@ GetDatabaseTuple(const char *dbname)
                 CStringGetDatum(dbname));

     /*
-     * Open pg_database and fetch a tuple.  Force heap scan if we haven't yet
-     * built the critical shared relcache entries (i.e., we're starting up
-     * without a shared relcache cache file).
+     * Try to fetch the tuple.  Force heap scan if we haven't yet built the
+     * critical shared relcache entries (i.e., we're starting up without a
+     * shared relcache cache file).
      */
-    relation = table_open(DatabaseRelationId, AccessShareLock);
     scan = systable_beginscan(relation, DatabaseNameIndexId,
                               criticalSharedRelcachesBuilt,
                               NULL,
@@ -133,7 +200,6 @@ GetDatabaseTuple(const char *dbname)

     /* all done */
     systable_endscan(scan);
-    table_close(relation, AccessShareLock);

     return tuple;
 }
@@ -1011,6 +1077,8 @@ InitPostgres(const char *in_dbname, Oid dboid,
                      errmsg("database \"%s\" does not exist", in_dbname)));
         dbform = (Form_pg_database) GETSTRUCT(tuple);
         dboid = dbform->oid;
+        /* Save the real (possibly truncated) database name */
+        strlcpy(dbname, NameStr(dbform->datname), sizeof(dbname));
     }
     else if (!OidIsValid(dboid))
     {
@@ -1066,8 +1134,9 @@ InitPostgres(const char *in_dbname, Oid dboid,
         if (HeapTupleIsValid(tuple))
             datform = (Form_pg_database) GETSTRUCT(tuple);

+        /* Note comparison here must be against the truncated DB name */
         if (!HeapTupleIsValid(tuple) ||
-            (in_dbname && namestrcmp(&datform->datname, in_dbname)))
+            (in_dbname && namestrcmp(&datform->datname, dbname) != 0))
         {
             if (in_dbname)
                 ereport(FATAL,