Re: Client Messages

Lists: pgsql-hackers
From: Jim Mlodgenski <jimmy76(at)gmail(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: Client Messages
Date: 2012-01-05 16:38:21
Message-ID: CAB_5SRdN1_rx0vkoZ8tC9PmcXCiy0QFEs_=vscg-UrFHeJdB=w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

I have a need to send banner messages to a psql client that I can set
on the server and will be displayed on any psql client that connects
to the database. This would be mostly used as an additional indicator
to which database you are connecting, but could also be used by people
to force their users to see an security message when connecting to the
database. The attached patch will allow you to execute

ALTER DATABASE postgres SET
client_message=E'********************************************************************************\nBEWARE:
You are connecting to a production database. If you do anything to\n
bring this server down, you will be destroyed by your supreme
overlord.\n********************************************************************************\n';

And then when you connect to psql, you will see:

[e3(at)workstation bin]$ ./psql -U user1 postgres
psql (9.2devel)
********************************************************************************
BEWARE: You are connecting to a production database. If you do anything to
bring this server down, you will be destroyed by your supreme overlord.
********************************************************************************

Type "help" for help.

postgres=>

Any feedback is welcome.

Thanks

Attachment Content-Type Size
client_message.patch text/x-patch 3.5 KB

From: "Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov>
To: "Jim Mlodgenski" <jimmy76(at)gmail(dot)com>, <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Client Messages
Date: 2012-01-05 16:41:59
Message-ID: 4F057E7702000025000443FA@gw.wicourts.gov
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Jim Mlodgenski <jimmy76(at)gmail(dot)com> wrote:

> Any feedback is welcome.

You might want to add it here to make sure it doesn't slip through the
cracks:

https://commitfest.postgresql.org/action/commitfest_view/open

-Kevin


From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Jim Mlodgenski <jimmy76(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Client Messages
Date: 2012-01-18 05:49:57
Message-ID: CAHGQGwHM=U+Z_4ezO_tcW9bukBV2bdHg0ve3zMExhqZyX3h8ng@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Jan 6, 2012 at 1:38 AM, Jim Mlodgenski <jimmy76(at)gmail(dot)com> wrote:
> I have a need to send banner messages to a psql client that I can set
> on the server and will be displayed on any psql client that connects
> to the database. This would be mostly used as an additional indicator
> to which database you are connecting, but could also be used by people
> to force their users to see an security message when connecting to the
> database. The attached patch will allow you to execute
>
> ALTER DATABASE postgres SET
> client_message=E'********************************************************************************\nBEWARE:
> You are connecting to a production database. If you do anything to\n
>     bring this server down, you will be destroyed by your supreme
> overlord.\n********************************************************************************\n';
>
> And then when you connect to psql, you will see:
>
> [e3(at)workstation bin]$ ./psql -U user1 postgres
> psql (9.2devel)
> ********************************************************************************
> BEWARE: You are connecting to a production database. If you do anything to
>        bring this server down, you will be destroyed by your supreme overlord.
> ********************************************************************************
>
> Type "help" for help.
>
> postgres=>
>
>
> Any feedback is welcome.

Adding new GUC parameter only for the purpose of warning psql users
seems overkill to me. Basically we try to reduce the number of GUC
parameters to make a configuration easier to a user, so I don't think that
it's good idea to add new GUC for such a small benefit. Instead, how
about using .psqlrc file and writing a warning message in it by using
\echo command?

Anyway, I found one problem in the patch. The patch defines client_message
as PGC_USERSET parameter, which means that any psql can falsify a
warning message, e.g., by setting the environment variable PGOPTIONS
to "-c client_message=hoge". This seems to be something to avoid from
security point of view.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Jim Mlodgenski <jimmy76(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Client Messages
Date: 2012-01-18 08:08:40
Message-ID: 4F167E08.9030802@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 18.01.2012 07:49, Fujii Masao wrote:
> On Fri, Jan 6, 2012 at 1:38 AM, Jim Mlodgenski<jimmy76(at)gmail(dot)com> wrote:
>> I have a need to send banner messages to a psql client that I can set
>> on the server and will be displayed on any psql client that connects
>> to the database. This would be mostly used as an additional indicator
>> to which database you are connecting, but could also be used by people
>> to force their users to see an security message when connecting to the
>> database. The attached patch will allow you to execute
>>
>> ALTER DATABASE postgres SET
>> client_message=E'********************************************************************************\nBEWARE:
>> You are connecting to a production database. If you do anything to\n
>> bring this server down, you will be destroyed by your supreme
>> overlord.\n********************************************************************************\n';
>>
>> And then when you connect to psql, you will see:
>>
>> [e3(at)workstation bin]$ ./psql -U user1 postgres
>> psql (9.2devel)
>> ********************************************************************************
>> BEWARE: You are connecting to a production database. If you do anything to
>> bring this server down, you will be destroyed by your supreme overlord.
>> ********************************************************************************
>>
>> Type "help" for help.
>>
>> postgres=>
>>
>>
>> Any feedback is welcome.
>
> Adding new GUC parameter only for the purpose of warning psql users
> seems overkill to me. Basically we try to reduce the number of GUC
> parameters to make a configuration easier to a user, so I don't think that
> it's good idea to add new GUC for such a small benefit.

It seems quite useful to me...

> Instead, how
> about using .psqlrc file and writing a warning message in it by using
> \echo command?

That's not the same thing at all. Each client would need to put the
warning in that file, and you'd get it regardless of the database you
connect to.

> Anyway, I found one problem in the patch. The patch defines client_message
> as PGC_USERSET parameter, which means that any psql can falsify a
> warning message, e.g., by setting the environment variable PGOPTIONS
> to "-c client_message=hoge". This seems to be something to avoid from
> security point of view.

I don't think that's a problem, it's just a free-form message to
display. But it also doesn't seem very useful to have it PGC_USERSET: if
it's only displayed at connect time, there's no point in changing it
after connecting.

The only security problem that I can think of is a malicious server
(man-in-the-middle perhaps), that sends a banner that confuses

Docs for PQparameterStatus() needs adjustment, now that client_message
is also one of the settings automatically reported to the client.

The placement of the banner in psql looks currently like this:

> $ psql postgres
> psql (9.2devel)
> Hello world!
> Type "help" for help.

or

> postgres=# \c postgres
> Hello world!
> You are now connected to database "postgres" as user "heikki".

Are we happy with that? I think it would be better to print the banner
just before the prompt:

> psql (9.2devel)
> Type "help" for help.
>
> Hello world!
>
> postgres=# \c postgres
> You are now connected to database "postgres" as user "heikki".
> Hello world!
> postgres=#

Should we prefix the banner with something that makes it clear that it's
a message coming from the server? Something like:

> psql (9.2devel)
> Type "help" for help.
>
> Notice from server: Hello world!
>
> postgres=# \c postgres
> You are now connected to database "postgres" as user "heikki".
> Notice from server: Hello world!
> postgres=#

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com


From: Jim Mlodgenski <jimmy76(at)gmail(dot)com>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Client Messages
Date: 2012-01-18 14:19:13
Message-ID: CAB_5SRefrOd4CM8m=snDicCUDU1QAqt2f+3SfT9eV+wPGWS_6A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Jan 18, 2012 at 3:08 AM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> On 18.01.2012 07:49, Fujii Masao wrote:
>>
>> On Fri, Jan 6, 2012 at 1:38 AM, Jim Mlodgenski<jimmy76(at)gmail(dot)com>  wrote:
>>>
>>> I have a need to send banner messages to a psql client that I can set
>>> on the server and will be displayed on any psql client that connects
>>> to the database. This would be mostly used as an additional indicator
>>> to which database you are connecting, but could also be used by people
>>> to force their users to see an security message when connecting to the
>>> database. The attached patch will allow you to execute
>>>
>>> ALTER DATABASE postgres SET
>>>
>>> client_message=E'********************************************************************************\nBEWARE:
>>> You are connecting to a production database. If you do anything to\n
>>>     bring this server down, you will be destroyed by your supreme
>>>
>>> overlord.\n********************************************************************************\n';
>>>
>>> And then when you connect to psql, you will see:
>>>
>>> [e3(at)workstation bin]$ ./psql -U user1 postgres
>>> psql (9.2devel)
>>>
>>> ********************************************************************************
>>> BEWARE: You are connecting to a production database. If you do anything
>>> to
>>>        bring this server down, you will be destroyed by your supreme
>>> overlord.
>>>
>>> ********************************************************************************
>>>
>>> Type "help" for help.
>>>
>>> postgres=>
>>>
>>>
>>> Any feedback is welcome.
>>
>>
>> Adding new GUC parameter only for the purpose of warning psql users
>> seems overkill to me.  Basically we try to reduce the number of GUC
>> parameters to make a configuration easier to a user, so I don't think that
>> it's good idea to add new GUC for such a small benefit.
>
>
> It seems quite useful to me...
>
>
>> Instead, how
>> about using .psqlrc file and writing a warning message in it by using
>> \echo command?
>
>
> That's not the same thing at all. Each client would need to put the warning
> in that file, and you'd get it regardless of the database you connect to.
>
>
>> Anyway, I found one problem in the patch. The patch defines client_message
>> as PGC_USERSET parameter, which means that any psql can falsify a
>> warning message, e.g., by setting the environment variable PGOPTIONS
>> to "-c client_message=hoge". This seems to be something to avoid from
>> security point of view.
>
>
> I don't think that's a problem, it's just a free-form message to display.
> But it also doesn't seem very useful to have it PGC_USERSET: if it's only
> displayed at connect time, there's no point in changing it after connecting.
Should we make it PGC_BACKEND?

>
> The only security problem that I can think of is a malicious server
> (man-in-the-middle perhaps), that sends a banner that confuses
>
> Docs for PQparameterStatus() needs adjustment, now that client_message is
> also one of the settings automatically reported to the client.
I'll add the docs for that..

>
> The placement of the banner in psql looks currently like this:
>
>> $ psql postgres
>>
>> psql (9.2devel)
>> Hello world!
>> Type "help" for help.
>
>
> or
>
>> postgres=# \c postgres
>> Hello world!
>> You are now connected to database "postgres" as user "heikki".
>
>
> Are we happy with that? I think it would be better to print the banner just
> before the prompt:
I like that better. I'll make that change as well.

>
>> psql (9.2devel)
>> Type "help" for help.
>>
>> Hello world!
>>
>> postgres=# \c postgres
>> You are now connected to database "postgres" as user "heikki".
>
>> Hello world!
>> postgres=#
>
> Should we prefix the banner with something that makes it clear that it's a
> message coming from the server? Something like:
I don't think the default prefix adds much for the user. If the
administrator wants to let the user know that its from the server, he
can add it to the message.

>
>> psql (9.2devel)
>> Type "help" for help.
>>
>> Notice from server: Hello world!
>>
>> postgres=# \c postgres
>> You are now connected to database "postgres" as user "heikki".
>> Notice from server: Hello world!
>> postgres=#
>
> --
>  Heikki Linnakangas
>  EnterpriseDB   http://www.enterprisedb.com


From: Jim Mlodgenski <jimmy76(at)gmail(dot)com>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Client Messages
Date: 2012-01-23 20:52:35
Message-ID: CAB_5SRd8xxWn5wAJ6iMoQqJQ1Wfqkbeh8Rhex+T8Uc1tHqSqfw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Jan 18, 2012 at 9:19 AM, Jim Mlodgenski <jimmy76(at)gmail(dot)com> wrote:
> On Wed, Jan 18, 2012 at 3:08 AM, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> On 18.01.2012 07:49, Fujii Masao wrote:
>>>
>>> On Fri, Jan 6, 2012 at 1:38 AM, Jim Mlodgenski<jimmy76(at)gmail(dot)com>  wrote:
>>>>
>>>> I have a need to send banner messages to a psql client that I can set
>>>> on the server and will be displayed on any psql client that connects
>>>> to the database. This would be mostly used as an additional indicator
>>>> to which database you are connecting, but could also be used by people
>>>> to force their users to see an security message when connecting to the
>>>> database. The attached patch will allow you to execute
>>>>
>>>> ALTER DATABASE postgres SET
>>>>
>>>> client_message=E'********************************************************************************\nBEWARE:
>>>> You are connecting to a production database. If you do anything to\n
>>>>     bring this server down, you will be destroyed by your supreme
>>>>
>>>> overlord.\n********************************************************************************\n';
>>>>
>>>> And then when you connect to psql, you will see:
>>>>
>>>> [e3(at)workstation bin]$ ./psql -U user1 postgres
>>>> psql (9.2devel)
>>>>
>>>> ********************************************************************************
>>>> BEWARE: You are connecting to a production database. If you do anything
>>>> to
>>>>        bring this server down, you will be destroyed by your supreme
>>>> overlord.
>>>>
>>>> ********************************************************************************
>>>>
>>>> Type "help" for help.
>>>>
>>>> postgres=>
>>>>
>>>>
>>>> Any feedback is welcome.
>>>
>>>
>>> Adding new GUC parameter only for the purpose of warning psql users
>>> seems overkill to me.  Basically we try to reduce the number of GUC
>>> parameters to make a configuration easier to a user, so I don't think that
>>> it's good idea to add new GUC for such a small benefit.
>>
>>
>> It seems quite useful to me...
>>
>>
>>> Instead, how
>>> about using .psqlrc file and writing a warning message in it by using
>>> \echo command?
>>
>>
>> That's not the same thing at all. Each client would need to put the warning
>> in that file, and you'd get it regardless of the database you connect to.
>>
>>
>>> Anyway, I found one problem in the patch. The patch defines client_message
>>> as PGC_USERSET parameter, which means that any psql can falsify a
>>> warning message, e.g., by setting the environment variable PGOPTIONS
>>> to "-c client_message=hoge". This seems to be something to avoid from
>>> security point of view.
>>
>>
>> I don't think that's a problem, it's just a free-form message to display.
>> But it also doesn't seem very useful to have it PGC_USERSET: if it's only
>> displayed at connect time, there's no point in changing it after connecting.
> Should we make it PGC_BACKEND?
>
>>
>> The only security problem that I can think of is a malicious server
>> (man-in-the-middle perhaps), that sends a banner that confuses
>>
>> Docs for PQparameterStatus() needs adjustment, now that client_message is
>> also one of the settings automatically reported to the client.
> I'll add the docs for that..
>
>>
>> The placement of the banner in psql looks currently like this:
>>
>>> $ psql postgres
>>>
>>> psql (9.2devel)
>>> Hello world!
>>> Type "help" for help.
>>
>>
>> or
>>
>>> postgres=# \c postgres
>>> Hello world!
>>> You are now connected to database "postgres" as user "heikki".
>>
>>
>> Are we happy with that? I think it would be better to print the banner just
>> before the prompt:
> I like that better. I'll make that change as well.

Here is the revised patch based on the feedback.

>
>>
>>> psql (9.2devel)
>>> Type "help" for help.
>>>
>>> Hello world!
>>>
>>> postgres=# \c postgres
>>> You are now connected to database "postgres" as user "heikki".
>>
>>> Hello world!
>>> postgres=#
>>
>> Should we prefix the banner with something that makes it clear that it's a
>> message coming from the server? Something like:
> I don't think the default prefix adds much for the user. If the
> administrator wants to let the user know that its from the server, he
> can add it to the message.
>
>>
>>> psql (9.2devel)
>>> Type "help" for help.
>>>
>>> Notice from server: Hello world!
>>>
>>> postgres=# \c postgres
>>> You are now connected to database "postgres" as user "heikki".
>>> Notice from server: Hello world!
>>> postgres=#
>>
>> --
>>  Heikki Linnakangas
>>  EnterpriseDB   http://www.enterprisedb.com

Attachment Content-Type Size
client_message_v2.patch text/x-patch 5.0 KB

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Jim Mlodgenski <jimmy76(at)gmail(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Client Messages
Date: 2012-01-24 12:38:04
Message-ID: 4F1EA62C.8010203@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 23.01.2012 22:52, Jim Mlodgenski wrote:
> On Wed, Jan 18, 2012 at 9:19 AM, Jim Mlodgenski<jimmy76(at)gmail(dot)com> wrote:
>> On Wed, Jan 18, 2012 at 3:08 AM, Heikki Linnakangas
>>> I don't think that's a problem, it's just a free-form message to display.
>>> But it also doesn't seem very useful to have it PGC_USERSET: if it's only
>>> displayed at connect time, there's no point in changing it after connecting.
>> Should we make it PGC_BACKEND?

In hindsight, making it PGC_BACKEND makes it much less useful, because
then you can't set it on a per-database basis with "ALTER DATABASE foo
SET ..." So I made it PGC_USERSET again.

> Here is the revised patch based on the feedback.

Thanks! I renamed the GUC to "welcome_message", to avoid confusion with
"client_min_messages". I also moved it to "Connection Settings"
category. Although it's technically settable within a session, the
purpose is to display a message when connecting, so "Connection
Settings" feels more fitting.

There's one little problem remaining with this, which is what to do if
the message is in a different encoding than used by the client? That's
not a new problem, we have the same problem with any string GUC, if you
do "SHOW <setting>". We restricted application_name to ASCII characters,
which is an obvious way to avoid the problem, but I think it would be a
shame to restrict this to ASCII-only.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com

Attachment Content-Type Size
welcome_message-v3.patch text/x-diff 544.0 KB

From: Jim Mlodgenski <jimmy76(at)gmail(dot)com>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Client Messages
Date: 2012-01-25 13:29:57
Message-ID: CAB_5SRc3LX3hhN3Qm380VaGkq_Eh8xRz+nriPf1va51uYAQH7w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Jan 24, 2012 at 7:38 AM, Heikki Linnakangas
<heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> On 23.01.2012 22:52, Jim Mlodgenski wrote:
>>
>> On Wed, Jan 18, 2012 at 9:19 AM, Jim Mlodgenski<jimmy76(at)gmail(dot)com>  wrote:
>>>
>>> On Wed, Jan 18, 2012 at 3:08 AM, Heikki Linnakangas
>>>>
>>>> I don't think that's a problem, it's just a free-form message to
>>>> display.
>>>>
>>>> But it also doesn't seem very useful to have it PGC_USERSET: if it's
>>>> only
>>>> displayed at connect time, there's no point in changing it after
>>>> connecting.
>>>
>>> Should we make it PGC_BACKEND?
>
>
> In hindsight, making it PGC_BACKEND makes it much less useful, because then
> you can't set it on a per-database basis with "ALTER DATABASE foo SET ..."
> So I made it PGC_USERSET again.
>
>
>> Here is the revised patch based on the feedback.
>
>
> Thanks! I renamed the GUC to "welcome_message", to avoid confusion with
> "client_min_messages". I also moved it to "Connection Settings" category.
> Although it's technically settable within a session, the purpose is to
> display a message when connecting, so "Connection Settings" feels more
> fitting.
>
> There's one little problem remaining with this, which is what to do if the
> message is in a different encoding than used by the client? That's not a new
> problem, we have the same problem with any string GUC, if you do "SHOW
> <setting>". We restricted application_name to ASCII characters, which is an
> obvious way to avoid the problem, but I think it would be a shame to
> restrict this to ASCII-only.
Isn't that an issue for the administrator understanding his audience.
Maybe some additional documentation explaining the encoding issue?

>
>
> --
>  Heikki Linnakangas
>  EnterpriseDB   http://www.enterprisedb.com


From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Jim Mlodgenski <jimmy76(at)gmail(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Client Messages
Date: 2012-01-26 08:34:50
Message-ID: 4F21102A.2080506@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 25.01.2012 15:29, Jim Mlodgenski wrote:
> On Tue, Jan 24, 2012 at 7:38 AM, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> There's one little problem remaining with this, which is what to do if the
>> message is in a different encoding than used by the client? That's not a new
>> problem, we have the same problem with any string GUC, if you do "SHOW
>> <setting>". We restricted application_name to ASCII characters, which is an
>> obvious way to avoid the problem, but I think it would be a shame to
>> restrict this to ASCII-only.
> Isn't that an issue for the administrator understanding his audience.
> Maybe some additional documentation explaining the encoding issue?

The thing is, there's currently no encoding conversion happening, so if
you have one database in LATIN1 encoding and another in UTF-8, for
example, whatever you put in your postgresql.conf is going to be wrong
for one database. I'm happy to just document the issue for per-database
messages, "ALTER DATABASE ... SET welcome_message", the encoding used
there need to match the encoding of the database, or it's displayed as
garbage. But what about per-user messages, when the user has access to
several databases, or postgresql.conf?

For postgresql.conf I think we could make a rule that it's always in
UTF-8. We haven't had to take a stance on the encoding used in
postgresql.conf before, but IMHO UTF-8 only would be quite reasonable.
We already have a problem there if for example you have two database
with different encodings, a schema with non-ascii characters in it, and
you try to put that schema in search_path in postgresql.conf. That's
such a rare situation that we haven't heard any complaints, but it's not
so unreasonable for welcome_message.

That still leaves the problem with per-user messages, though. We've
managed to dodge the encoding issue in shared catalogs this far. It
would be pretty hard to enforce any given encoding there, I think.
Perhaps we could just document that, and advice to create per-database
and per-user settings in that case.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Jim Mlodgenski <jimmy76(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Client Messages
Date: 2012-01-26 15:31:56
Message-ID: 4484.1327591916@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> The thing is, there's currently no encoding conversion happening, so if
> you have one database in LATIN1 encoding and another in UTF-8, for
> example, whatever you put in your postgresql.conf is going to be wrong
> for one database. I'm happy to just document the issue for per-database
> messages, "ALTER DATABASE ... SET welcome_message", the encoding used
> there need to match the encoding of the database, or it's displayed as
> garbage. But what about per-user messages, when the user has access to
> several databases, or postgresql.conf?

I've not looked at the patch, but what exactly will happen if the string
has the wrong encoding?

The idea that occurs to me is to have the code that uses the GUC do a
verify_mbstr(noerror) on it, and silently ignore it if it doesn't pass
(maybe with a LOG message). This would have to be documented of course,
but it seems better than the potential consequences of trying to send a
wrongly-encoded string.

regards, tom lane


From: Peter Eisentraut <peter_e(at)gmx(dot)net>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Jim Mlodgenski <jimmy76(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Client Messages
Date: 2012-01-26 17:49:45
Message-ID: 1327600185.26839.2.camel@vanquo.pezone.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On tor, 2012-01-26 at 10:34 +0200, Heikki Linnakangas wrote:
> For postgresql.conf I think we could make a rule that it's always in
> UTF-8. We haven't had to take a stance on the encoding used in
> postgresql.conf before, but IMHO UTF-8 only would be quite reasonable.

We have so far violently resisted forcing UTF-8 anywhere, and this
doesn't seem like a good place to start.

We could perhaps recognize an encoding declaration like this in the
file:

http://docs.python.org/reference/lexical_analysis.html#encoding-declarations


From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Jim Mlodgenski <jimmy76(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Client Messages
Date: 2012-01-26 18:58:58
Message-ID: 4F21A272.3000703@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 26.01.2012 17:31, Tom Lane wrote:
> Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> The thing is, there's currently no encoding conversion happening, so if
>> you have one database in LATIN1 encoding and another in UTF-8, for
>> example, whatever you put in your postgresql.conf is going to be wrong
>> for one database. I'm happy to just document the issue for per-database
>> messages, "ALTER DATABASE ... SET welcome_message", the encoding used
>> there need to match the encoding of the database, or it's displayed as
>> garbage. But what about per-user messages, when the user has access to
>> several databases, or postgresql.conf?
>
> I've not looked at the patch, but what exactly will happen if the string
> has the wrong encoding?

You get an incorrectly encoded string, ie. garbage, in your console,
when you log in with psql.

You can also use current_setting() to copy the incorrectly-encoded
string elsewhere in the system. If you insert it into a table and run
pg_dump, I think the dump might not be restorable. That's a bit of a
stretch, perhaps, but it would be nice to avoid that.

BTW, you can already do that if you set e.g default_text_search_config
to something non-ASCII in postgresql.conf. Or if you do it with
search_path, you get a warning at login. For example, I did "ALTER USER
foouser set search_path ='kääk';" in a LATIN1 database, and then
connected to a UTF-8 database and got:

$ ~/pgsql.master/bin/psql postgres foouser
WARNING: invalid value for parameter "search_path": ""k��k""
DETAIL: schema "k��k" does not exist
psql (9.2devel)
Type "help" for help.

(in case that didn't get across right, I set the search_path to a string
containing two a-with-umlauts, and in the warning, they got replaced
with question marks with inverse colors, which is apparently a character
that the console uses to display bytes that are not valid UTF-8).

The problem with welcome_message would look just like that. No-one is
likely to run into that with search_path, but it's quite reasonable and
expected to use your native language in a welcome message.

> The idea that occurs to me is to have the code that uses the GUC do a
> verify_mbstr(noerror) on it, and silently ignore it if it doesn't pass
> (maybe with a LOG message). This would have to be documented of course,
> but it seems better than the potential consequences of trying to send a
> wrongly-encoded string.

Hmm, fine with me. It would be nice to plug the hole that these bogus
characters can leak elsewhere into the system through current_setting,
though. Perhaps we could put the verify_mbstr() call somewhere in guc.c,
to forbid incorrectly encoded characters from being stored in the guc
variable in the first place.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com


From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jim Mlodgenski <jimmy76(at)gmail(dot)com>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Client Messages
Date: 2012-02-29 19:13:42
Message-ID: 1330542661-sup-9415@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Excerpts from Heikki Linnakangas's message of jue ene 26 15:58:58 -0300 2012:
> On 26.01.2012 17:31, Tom Lane wrote:

> > The idea that occurs to me is to have the code that uses the GUC do a
> > verify_mbstr(noerror) on it, and silently ignore it if it doesn't pass
> > (maybe with a LOG message). This would have to be documented of course,
> > but it seems better than the potential consequences of trying to send a
> > wrongly-encoded string.
>
> Hmm, fine with me. It would be nice to plug the hole that these bogus
> characters can leak elsewhere into the system through current_setting,
> though. Perhaps we could put the verify_mbstr() call somewhere in guc.c,
> to forbid incorrectly encoded characters from being stored in the guc
> variable in the first place.

This patch is listed as "Needs review" but that seems to be wrong --
it's "waiting on author", I think. Do we have an updated patch? Fujii?

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jim Mlodgenski <jimmy76(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Client Messages
Date: 2012-03-01 02:39:53
Message-ID: CAHGQGwGxEWrAonEpTteadqk_mrMELsgwa1P4t8vWfNG3Km+1gQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Mar 1, 2012 at 4:13 AM, Alvaro Herrera
<alvherre(at)commandprompt(dot)com> wrote:
>
> Excerpts from Heikki Linnakangas's message of jue ene 26 15:58:58 -0300 2012:
>> On 26.01.2012 17:31, Tom Lane wrote:
>
>> > The idea that occurs to me is to have the code that uses the GUC do a
>> > verify_mbstr(noerror) on it, and silently ignore it if it doesn't pass
>> > (maybe with a LOG message).  This would have to be documented of course,
>> > but it seems better than the potential consequences of trying to send a
>> > wrongly-encoded string.
>>
>> Hmm, fine with me. It would be nice to plug the hole that these bogus
>> characters can leak elsewhere into the system through current_setting,
>> though. Perhaps we could put the verify_mbstr() call somewhere in guc.c,
>> to forbid incorrectly encoded characters from being stored in the guc
>> variable in the first place.
>
> This patch is listed as "Needs review" but that seems to be wrong --
> it's "waiting on author", I think.

Yes. I marked the patch as "waiting on author".

>  Do we have an updated patch?  Fujii?

No. I believe that the author Jim will submit the updated version.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jim Mlodgenski <jimmy76(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Client Messages
Date: 2012-03-08 13:40:28
Message-ID: CA+TgmoYjtXCwPE4LFkN7ua1=MYoS0U6wio0NrYwPBJyW4NWu8Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Feb 29, 2012 at 9:39 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>  Do we have an updated patch?  Fujii?
>
> No. I believe that the author Jim will submit the updated version.

Jim, are you going to submit an updated version?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Jim Mlodgenski <jimmy76(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Client Messages
Date: 2012-03-15 01:30:27
Message-ID: CA+TgmoZfE5hAw0jhQTFCNsK0zqd+SPdtP6SBxbpR27jbuFCT8Q@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, Mar 8, 2012 at 8:40 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Wed, Feb 29, 2012 at 9:39 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>>  Do we have an updated patch?  Fujii?
>>
>> No. I believe that the author Jim will submit the updated version.
>
> Jim, are you going to submit an updated version?

Hearing no response, I'm marking this Returned with Feedback.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company