Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-28 Thread Don Seiler
Thanks for all the guidance! Don. On Fri, Sep 28, 2018, 18:12 Stephen Frost wrote: > Greetings, > > * Stephen Frost (sfr...@snowman.net) wrote: > > * Stephen Frost (sfr...@snowman.net) wrote: > > > I still don't see that as a reason for tools to be suseptible to > serious > > > issues if a funk

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-28 Thread Stephen Frost
Greetings, * Stephen Frost (sfr...@snowman.net) wrote: > * Stephen Frost (sfr...@snowman.net) wrote: > > I still don't see that as a reason for tools to be suseptible to serious > > issues if a funky user gets created and I'd be surprised if there > > weren't other ways to get funky characters int

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-27 Thread Stephen Frost
Greetings, all, * Stephen Frost (sfr...@snowman.net) wrote: > I still don't see that as a reason for tools to be suseptible to serious > issues if a funky user gets created and I'd be surprised if there > weren't other ways to get funky characters into the log file, but that's > all ultimately an

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-27 Thread Stephen Frost
Greetings, * Andres Freund (and...@anarazel.de) wrote: > On September 27, 2018 2:55:56 PM PDT, Stephen Frost > wrote: > >* Andres Freund (and...@anarazel.de) wrote: > >> On 2018-09-27 17:41:56 -0400, Stephen Frost wrote: > >> > Of course, if I'm missing something as to why the ascii-cleaning > >

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-27 Thread Andres Freund
On September 27, 2018 2:55:56 PM PDT, Stephen Frost wrote: >Greetings, > >* Andres Freund (and...@anarazel.de) wrote: >> On 2018-09-27 17:41:56 -0400, Stephen Frost wrote: >> > Of course, if I'm missing something as to why the ascii-cleaning >makes >> > sense or is necessary, I'm all ears, but

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-27 Thread Stephen Frost
Greetings, * Andres Freund (and...@anarazel.de) wrote: > On 2018-09-27 17:41:56 -0400, Stephen Frost wrote: > > 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. > > There's many reasons. For example you can

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-27 Thread Andres Freund
Hi, On 2018-09-27 17:41:56 -0400, Stephen Frost wrote: > 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. There's many reasons. For example you can send terminal control characters to the server. When somebod

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-27 Thread Stephen Frost
Greetings, * Peter Eisentraut (peter.eisentr...@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 o

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-27 Thread Peter Eisentraut
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 i

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-25 Thread Don Seiler
On Mon, Sep 24, 2018 at 4:10 PM Stephen Frost wrote: > > * Don Seiler (d...@seiler.us) wrote: > > > > OK I created a new function called clean_ascii() in common/string.c. I > call > > this from my new logic in postmaster.c as well as replacing the logic in > > guc.c's check_application_name() and

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-24 Thread Stephen Frost
Greetings Don! * Don Seiler (d...@seiler.us) wrote: > On Tue, Aug 7, 2018 at 12:32 PM Tom Lane wrote: > > Don Seiler writes: > > > > > 1. We want to make a generic, central ascii-lobotomizing function similar > > > to check_application_name that we can re-use there and for other checks > > (eg >

Re: [PATCH] Include application_name in "connection authorized" log message

2018-09-21 Thread Don Seiler
On Tue, Aug 7, 2018 at 12:32 PM Tom Lane wrote: > Don Seiler writes: > > > 1. We want to make a generic, central ascii-lobotomizing function similar > > to check_application_name that we can re-use there and for other checks > (eg > > user name). > > 2. Change check_application_name to call this

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Tom Lane
Don Seiler writes: > On Tue, Aug 7, 2018 at 11:29 AM, Tom Lane wrote: >> Well, if you're going to insist on that part, it's probably not worth >> making the application_name GUC have inconsistent behavior. > OK so just to make sure I understand: > 1. We want to make a generic, central ascii-lob

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Don Seiler
On Tue, Aug 7, 2018 at 11:29 AM, Tom Lane wrote: > Stephen Frost writes: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> But having said that, I don't exactly see why you couldn't force it > >> with an ultimately-redundant SetConfigOption call to put the value > >> in place before the ereport ha

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Tom Lane
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> But having said that, I don't exactly see why you couldn't force it >> with an ultimately-redundant SetConfigOption call to put the value >> in place before the ereport happens. The GUC machinery is surely >> functional before we d

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Stephen Frost
Greetings Tom, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Tom Lane (t...@sss.pgh.pa.us) wrote: > >> Moreover, if you don't check it then the appname recorded > >> by log_connections would not match appearances for the same session > >> later in the log, which puts the en

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Tom Lane
Stephen Frost writes: > * Tom Lane (t...@sss.pgh.pa.us) wrote: >> Moreover, if you don't check it then the appname recorded >> by log_connections would not match appearances for the same session >> later in the log, which puts the entire use-case for this patch into >> question. So no, this conce

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Stephen Frost
Greetings Don, * Don Seiler (d...@seiler.us) wrote: > On Tue, Aug 7, 2018 at 8:46 AM, Stephen Frost wrote: > > * Don Seiler (d...@seiler.us) wrote: > > > On Mon, Jul 30, 2018 at 5:20 AM, Peter Eisentraut < > > > peter.eisentr...@2ndquadrant.com> wrote: > > > > > > > On 13/07/2018 20:20, Don Seile

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Stephen Frost
Greetings, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Stephen Frost writes: > > * Don Seiler (d...@seiler.us) wrote: > >> Is the concern that any user can set their client's application name value > >> to any string they want? Is there a reason we can't call > >> check_application_name() before set

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Don Seiler
On Tue, Aug 7, 2018 at 8:46 AM, Stephen Frost wrote: > Greetings, > > * Don Seiler (d...@seiler.us) wrote: > > On Mon, Jul 30, 2018 at 5:20 AM, Peter Eisentraut < > > peter.eisentr...@2ndquadrant.com> wrote: > > > > > On 13/07/2018 20:20, Don Seiler wrote: > > > > See attached for latest revision

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Tom Lane
Stephen Frost writes: > * Don Seiler (d...@seiler.us) wrote: >> Is the concern that any user can set their client's application name value >> to any string they want? Is there a reason we can't call >> check_application_name() before setting it in the Port struct in >> postmaster.c? > I've not lo

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Stephen Frost
Greetings, * Don Seiler (d...@seiler.us) wrote: > On Mon, Jul 30, 2018 at 5:20 AM, Peter Eisentraut < > peter.eisentr...@2ndquadrant.com> wrote: > > > On 13/07/2018 20:20, Don Seiler wrote: > > > See attached for latest revision. > > > > This doesn't compile with SSL enabled because there is a co

Re: [PATCH] Include application_name in "connection authorized" log message

2018-08-07 Thread Don Seiler
On Mon, Jul 30, 2018 at 5:20 AM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > On 13/07/2018 20:20, Don Seiler wrote: > > See attached for latest revision. > > This doesn't compile with SSL enabled because there is a comma missing. > Hmm I'll check this out tonight. Sorry I wasn't

Re: [PATCH] Include application_name in "connection authorized" log message

2018-07-30 Thread Peter Eisentraut
On 13/07/2018 20:20, Don Seiler wrote: > See attached for latest revision. This doesn't compile with SSL enabled because there is a comma missing. This implementation doesn't run the application_name through check_application_name(), so it could end up logging application_name values that are oth

Re: [PATCH] Include application_name in "connection authorized" log message

2018-07-13 Thread Don Seiler
On Fri, Jul 13, 2018 at 10:13 AM, Don Seiler wrote: > On Fri, Jul 13, 2018 at 9:37 AM, Stephen Frost wrote: > >> >> Don, do you want to update the patch accordingly? If not, I'm happy to >> handle it when I go to commit it, which I'm thinking of doing sometime >> this weekend as it seems to be

Re: [PATCH] Include application_name in "connection authorized" log message

2018-07-13 Thread Don Seiler
On Fri, Jul 13, 2018 at 9:37 AM, Stephen Frost wrote: > > Don, do you want to update the patch accordingly? If not, I'm happy to > handle it when I go to commit it, which I'm thinking of doing sometime > this weekend as it seems to be pretty uncontroversial at this point. Whatever is easiest f

Re: [PATCH] Include application_name in "connection authorized" log message

2018-07-13 Thread Stephen Frost
Greetings, * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote: > On 02.07.18 15:12, Don Seiler wrote: > > On Mon, Jul 2, 2018 at 2:13 AM, Peter Eisentraut > > > > wrote: > > > > On 21.06.18 16:21, Don Seiler wrote: > > > -               

Re: [PATCH] Include application_name in "connection authorized" log message

2018-07-13 Thread Peter Eisentraut
On 02.07.18 15:12, Don Seiler wrote: > On Mon, Jul 2, 2018 at 2:13 AM, Peter Eisentraut > > wrote: > > On 21.06.18 16:21, Don Seiler wrote: > > -                                               (errmsg("connection > > authorized: user=%s database

Re: [PATCH] Include application_name in "connection authorized" log message

2018-07-02 Thread Don Seiler
On Mon, Jul 2, 2018 at 2:13 AM, Peter Eisentraut < peter.eisentr...@2ndquadrant.com> wrote: > On 21.06.18 16:21, Don Seiler wrote: > > - (errmsg("connection > > authorized: user=%s database=%s", > > - > > port->user_name, port->database_name))); > > +

Re: [PATCH] Include application_name in "connection authorized" log message

2018-07-02 Thread Peter Eisentraut
On 21.06.18 16:21, Don Seiler wrote: > -                                               (errmsg("connection > authorized: user=%s database=%s", > -                                                              >  port->user_name, port->database_name))); > +                                           

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-25 Thread Gilles Darold
Le 25/06/2018 à 22:32, Stephen Frost a écrit : Greetings On Mon, Jun 25, 2018 at 15:57 Don Seiler > wrote: On Mon, Jun 25, 2018 at 10:50 AM, Gilles Darold mailto:gilles.dar...@dalibo.com>> wrote: I would prefer the following output to conform to elog.c o

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-25 Thread Gilles Darold
Le 25/06/2018 à 21:57, Don Seiler a écrit : On Mon, Jun 25, 2018 at 10:50 AM, Gilles Darold mailto:gilles.dar...@dalibo.com>> wrote: I would prefer the following output to conform to elog.c output:     2018-06-25 17:32:46.854 CEST [23265] LOG: connection authorized: user=postgres d

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-25 Thread Stephen Frost
Greetings On Mon, Jun 25, 2018 at 15:57 Don Seiler wrote: > On Mon, Jun 25, 2018 at 10:50 AM, Gilles Darold > wrote: > >> I would prefer the following output to conform to elog.c output: >> >> 2018-06-25 17:32:46.854 CEST [23265] LOG: connection authorized: >> user=postgres database=postgr

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-25 Thread Don Seiler
On Mon, Jun 25, 2018 at 10:50 AM, Gilles Darold wrote: > I would prefer the following output to conform to elog.c output: > > 2018-06-25 17:32:46.854 CEST [23265] LOG: connection authorized: > user=postgres database=postgres application=[unknown] > Hello Gilles, The different cases were for

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-25 Thread Gilles Darold
Le 21/06/2018 à 16:21, Don Seiler a écrit : On Wed, Jun 20, 2018 at 2:45 PM, Stephen Frost > wrote: diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h index 7698cd1f88..088ef346a8 100644 --- a/src/include/libpq/libpq-be.h +++

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-22 Thread Alvaro Herrera
On 2018-Jun-22, Andres Freund wrote: > On 2018-06-22 15:50:06 -0400, Alvaro Herrera wrote: > > On 2018-Jun-22, Andres Freund wrote: > > > > > On 2018-06-22 15:26:06 -0400, Alvaro Herrera wrote: > > > > On 2018-Jun-22, Andres Freund wrote: > > > > > I think a fair argument could be made that you'd

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-22 Thread Andres Freund
On 2018-06-22 15:50:06 -0400, Alvaro Herrera wrote: > On 2018-Jun-22, Andres Freund wrote: > > > On 2018-06-22 15:26:06 -0400, Alvaro Herrera wrote: > > > On 2018-Jun-22, Andres Freund wrote: > > > > I think a fair argument could be made that you'd want to have > > > > application_name logged exac

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-22 Thread Alvaro Herrera
On 2018-Jun-22, Andres Freund wrote: > On 2018-06-22 15:26:06 -0400, Alvaro Herrera wrote: > > On 2018-Jun-22, Andres Freund wrote: > > > > > On 2018-06-22 12:16:18 -0400, Robert Haas wrote: > > > > > > OK, that makes more sense, but I'm still skeptical of adding a special > > > > case particula

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-22 Thread Andres Freund
On 2018-06-22 15:26:06 -0400, Alvaro Herrera wrote: > On 2018-Jun-22, Andres Freund wrote: > > > On 2018-06-22 12:16:18 -0400, Robert Haas wrote: > > > > OK, that makes more sense, but I'm still skeptical of adding a special > > > case particularly for application_name. > > > > I think a fair ar

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-22 Thread Alvaro Herrera
On 2018-Jun-22, Andres Freund wrote: > On 2018-06-22 12:16:18 -0400, Robert Haas wrote: > > OK, that makes more sense, but I'm still skeptical of adding a special > > case particularly for application_name. > > I think a fair argument could be made that you'd want to have > application_name logg

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-22 Thread Andres Freund
On 2018-06-22 12:16:18 -0400, Robert Haas wrote: > On Fri, Jun 22, 2018 at 11:17 AM, Stephen Frost wrote: > > The issue here is exactly that at the point where we emit the > > 'connection authorized' message, we haven't processed generic GUCs from > > the startup packet yet and therefore applicati

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-22 Thread Stephen Frost
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Jun 22, 2018 at 11:17 AM, Stephen Frost wrote: > > The issue here is exactly that at the point where we emit the > > 'connection authorized' message, we haven't processed generic GUCs from > > the startup packet yet and therefore a

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-22 Thread Robert Haas
On Fri, Jun 22, 2018 at 11:17 AM, Stephen Frost wrote: > The issue here is exactly that at the point where we emit the > 'connection authorized' message, we haven't processed generic GUCs from > the startup packet yet and therefore application_name isn't set as a > GUC and, as a result, isn't incl

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-22 Thread Stephen Frost
Greetings, * Robert Haas (robertmh...@gmail.com) wrote: > On Wed, Jun 20, 2018 at 3:45 PM, Stephen Frost wrote: > > * Don Seiler (d...@seiler.us) wrote: > >> In trying to troubleshoot the source of a recent connection storm, I was > >> frustrated to find that the app name was not included in the

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-22 Thread Robert Haas
On Wed, Jun 20, 2018 at 3:45 PM, Stephen Frost wrote: > * Don Seiler (d...@seiler.us) wrote: >> In trying to troubleshoot the source of a recent connection storm, I was >> frustrated to find that the app name was not included in the connection >> messages. It is there in the log_line_prefix on the

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-21 Thread Don Seiler
On Wed, Jun 20, 2018 at 2:45 PM, Stephen Frost wrote: > > diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h > index 7698cd1f88..088ef346a8 100644 > --- a/src/include/libpq/libpq-be.h > +++ b/src/include/libpq/libpq-be.h > @@ -135,6 +135,7 @@ typedef struct Port > *

Re: [PATCH] Include application_name in "connection authorized" log message

2018-06-20 Thread Stephen Frost
Greetings, * Don Seiler (d...@seiler.us) wrote: > In trying to troubleshoot the source of a recent connection storm, I was > frustrated to find that the app name was not included in the connection > messages. It is there in the log_line_prefix on the disconnection messages > but I would prefer it

[PATCH] Include application_name in "connection authorized" log message

2018-06-20 Thread Don Seiler
Good afternoon, all. In trying to troubleshoot the source of a recent connection storm, I was frustrated to find that the app name was not included in the connection messages. It is there in the log_line_prefix on the disconnection messages but I would prefer it be directly visible with the connec