Re: ProcessStartupPacket(): database_name and user_name truncation

2023-07-03 Thread Drouvot, Bertrand
Hi, On 7/3/23 10:34 PM, Nathan Bossart wrote: On Sat, Jul 01, 2023 at 04:02:06PM +0200, Drouvot, Bertrand wrote: Please find V2 attached where it's failing as soon as the database name or user name are detected as overlength. Thanks, Bertrand. I chickened out and ended up committing v1 for n

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-07-03 Thread Tom Lane
Nathan Bossart writes: > Thanks, Bertrand. I chickened out and ended up committing v1 for now > (i.e., simply removing the truncation code). WFM. > If anyone disagrees and wants to see the FATALs emitted from > ProcessStartupPacket() directly, please let me know and we can work on > adding them

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-07-03 Thread Nathan Bossart
On Sat, Jul 01, 2023 at 04:02:06PM +0200, Drouvot, Bertrand wrote: > Please find V2 attached where it's failing as soon as the database name or > user name are detected as overlength. Thanks, Bertrand. I chickened out and ended up committing v1 for now (i.e., simply removing the truncation code).

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-07-02 Thread Kyotaro Horiguchi
At Mon, 03 Jul 2023 10:50:45 +0900 (JST), Kyotaro Horiguchi wrote in > For the record, if I understand Nathan correctly, it is what I > suggested in my initial post. If this is correct, +1 for the suggestion. > > me> I think we might want to consider outright rejecting the > me> estblishment of

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-07-02 Thread Kyotaro Horiguchi
At Fri, 30 Jun 2023 19:32:50 +0200, "Drouvot, Bertrand" wrote in > Hi, > > On 6/30/23 5:54 PM, Tom Lane wrote: > > Nathan Bossart writes: > >> After taking another look at this, I wonder if it'd be better to fail > >> as > >> soon as we see the database or user name is too long instead of > >>

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-07-01 Thread Drouvot, Bertrand
Hi, On 6/30/23 7:32 PM, Drouvot, Bertrand wrote: Hi, On 6/30/23 5:54 PM, Tom Lane wrote: Nathan Bossart writes: After taking another look at this, I wonder if it'd be better to fail as soon as we see the database or user name is too long instead of lugging them around when authentication is

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-30 Thread Drouvot, Bertrand
Hi, On 6/30/23 5:54 PM, Tom Lane wrote: Nathan Bossart writes: After taking another look at this, I wonder if it'd be better to fail as soon as we see the database or user name is too long instead of lugging them around when authentication is destined to fail. If we're agreed that we aren't

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-30 Thread Tom Lane
Nathan Bossart writes: > After taking another look at this, I wonder if it'd be better to fail as > soon as we see the database or user name is too long instead of lugging > them around when authentication is destined to fail. If we're agreed that we aren't going to truncate these identifiers, th

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-30 Thread Nathan Bossart
After taking another look at this, I wonder if it'd be better to fail as soon as we see the database or user name is too long instead of lugging them around when authentication is destined to fail. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-21 Thread Drouvot, Bertrand
Hi, On 6/22/23 1:37 AM, Michael Paquier wrote: On Wed, Jun 21, 2023 at 12:55:15PM -0700, Nathan Bossart wrote: LGTM. I think this can wait for v17 since the current behavior has been around since 2001 and AFAIK this is the first report. While it's arguably a bug fix, the patch also breaks som

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-21 Thread Michael Paquier
On Wed, Jun 21, 2023 at 12:55:15PM -0700, Nathan Bossart wrote: > LGTM. I think this can wait for v17 since the current behavior has been > around since 2001 and AFAIK this is the first report. While it's arguably > a bug fix, the patch also breaks some cases that work today. Agreed that anythin

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-21 Thread Nathan Bossart
On Wed, Jun 21, 2023 at 09:02:49PM +0200, Drouvot, Bertrand wrote: > Please find attached a patch doing so (which is basically a revert of > d18c1d1f51). LGTM. I think this can wait for v17 since the current behavior has been around since 2001 and AFAIK this is the first report. While it's argu

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-21 Thread Drouvot, Bertrand
On 6/21/23 4:22 PM, Drouvot, Bertrand wrote: Hi, On 6/21/23 3:43 PM, Tom Lane wrote: Kyotaro Horiguchi writes: At Wed, 21 Jun 2023 09:43:50 +0200, "Drouvot, Bertrand" wrote in Trying to connect with the 64 bytes name: $ psql -d psql: error: connection to

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-21 Thread Nathan Bossart
On Wed, Jun 21, 2023 at 09:43:38AM -0400, Tom Lane wrote: > Kyotaro Horiguchi writes: >> IMHO, I'm not sure we should allow connections without the exact name >> being provided. In that sense, I think we might want to consider >> outright rejecting the estblishment of a connection when the given >

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-21 Thread Drouvot, Bertrand
Hi, On 6/21/23 3:43 PM, Tom Lane wrote: Kyotaro Horiguchi writes: At Wed, 21 Jun 2023 09:43:50 +0200, "Drouvot, Bertrand" wrote in Trying to connect with the 64 bytes name: $ psql -d psql: error: connection to server on socket "/tmp/.s.PGSQL.55448" failed: F

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-21 Thread Tom Lane
Kyotaro Horiguchi writes: > At Wed, 21 Jun 2023 09:43:50 +0200, "Drouvot, Bertrand" > wrote in >> Trying to connect with the 64 bytes name: >> $ psql -d >> psql: error: connection to server on socket "/tmp/.s.PGSQL.55448" >> failed: FATAL: database "

Re: ProcessStartupPacket(): database_name and user_name truncation

2023-06-21 Thread Kyotaro Horiguchi
At Wed, 21 Jun 2023 09:43:50 +0200, "Drouvot, Bertrand" wrote in > Trying to connect with the 64 bytes name: > > $ psql -d > psql: error: connection to server on socket "/tmp/.s.PGSQL.55448" > failed: FATAL: database "äää" does not >

ProcessStartupPacket(): database_name and user_name truncation

2023-06-21 Thread Drouvot, Bertrand
Hi hackers, Please find attached a patch to truncate (in ProcessStartupPacket()) the port->database_name and port->user_name in such a way to not break multibyte character boundary. Indeed, currently, one could create a database that way: postgres=# create database ä