Greetings,
* Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote:
> On 24/09/2018 23:10, Stephen Frost wrote:
> > Since we're putting it into common/string.c (which seems pretty
> > reasonable to me, at least), I went ahead and changed it to be
> > 'pg_clean_ascii'. I didn't see any other obvious cases where we could
> > use this function (though typecmds.c does have an interesting ASCII
> > check for type categories..).
>
> I think we might want to contain this functionality to guc.c. In
> general, we should allow more non-ASCII things rather than encourage
> more "ASCII cleaning".
If we're going in that direction then I'd ask again why we are doing
things here to clean down to 'ascii only' when 'user', for example, is
not "cleaned":
2018-09-27 17:26:25.550 EDT [12579] êāŁƛǚ@postgres LOG: connection authorized: user=êāŁƛǚ database=postgres SSL
enabled(protocol=TLSv1.2, cipher=ECDHE-RSA-AES256-GCM-SHA384, compression=off)
Meaning we get all kinds of interesting in the exact same message that
we're talking about here when someone has a funky username.
It's not like we don't *also* update the process title with the username
too:
postgres 14811 0.0 0.0 327596 12432 ? Ss 17:37 0:00 postgres: 10/main: êāŁ?ǚ postgres 127.0.0.1(54252)
idle
Meaning that the "cleaning" for 'cluster name', whose entire purpose is
to go into the process title, hardly makes sense either.
I get that username and database are things that an admin can control
and perhaps there's an argument that someone's broken log parsing system
is "ok" because it only ever has to deal with known-simple usernames or
databases because that's all the admin ever created, but I don't think
we have historically, nor should we now, write code to help keep
objectively broken systems working across major (or even minor, really,
though I'm certainly not pushing for this change to go into
back-branches) releases.
I'd be just as happy to rip out the code that's cleaning things down to
ASCII in all of these.
Of course, if I'm missing something as to why the ascii-cleaning makes
sense or is necessary, I'm all ears, but I'm just not seeing it.
Thanks!
Stephen