Re: BUG #18711: Attempting a connection with a database name longer than 63 characters now fails

Hi,

On Wed, Nov 27, 2024 at 01:34:22PM -0600, Nathan Bossart wrote:
> 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.

+1 for not reporting an encoding-corrupt name in connection failure
reports.

> 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.

Yeah.

> 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 think that makes sense. Bonus point: the ClientAuthentication_hook would have
both names (truncated and non-truncated) at its disposal.

> 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.

If we do a straight revert on 17 then that would mean switching back to
reporting truncated name (currently 17 reports non truncated names) and handle
correct truncation on "ASCII" database name connections.

If we back patch, then "only" one change is made (from an end user point of
view): handle correct truncation on "ASCII" database name connections.

OTOH:

- the 2 new members would have to be at the end of the struct (since they
are 8 bytes long and there is no existing padding holes that could host them).
- I did a quick check in the commit log history and I did not see any change in
the Port struct that has been back-patched.

Based on the above, it might be better to just do a straight revert on 17.

+ /*
+ * Truncate given database and user names to length of a Postgres name.
+ * This avoids lookup failures when overlength names are given.
+ */

This is the exact comment prior to 562bee0fc1, worth to add a few words about
the non-ASCII case?

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com