Re: elog() patch - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: elog() patch
Date
Msg-id 200203030246.g232k5P12522@candle.pha.pa.us
Whole thread Raw
In response to Re: elog() patch  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: elog() patch  (Bruce Momjian <pgman@candle.pha.pa.us>)
Re: elog() patch  (Bruce Momjian <pgman@candle.pha.pa.us>)
List pgsql-hackers
Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > Is this what you were looking for?  I set client_min_messages to the max
> > of debug5 and the output is attached.
>
> If the DBA wants to do that, I don't have a problem with it.  I'm
> wondering what happens if an unprivileged user tries to do it,
> via either PGOPTIONS or Peter's new user/database-local options.
>
> Please note also that I'm wondering about the messages emitted during
> an authorization *failure*, not a successful connection.

You ask a very good question here.  I never tested authentication with
debug sent to the client.  The answer is that it doesn't work without
the attached patch.  Now, I am not about to apply this because it does
change getNotice() to an extern and moves its prototype to libpq-int.h.
This is necessary because I now use getNotice() in fe-connect.c.

The second issue is that this isn't going to work for pre-7.2 clients
because the protocol doesn't expect 'N' messages during the
authentication phase.  I think we can live with a client_min_messages
level of debug* not working on old clients, though we should make a
mention of it in the release notes.

And finally, here is the output from a failed password login with the
patch applied:

    $ psql test
    Password:
    DEBUG:  received password packet with len=12, pw=lkjasdf

    DEBUG:  received password packet with len=12, pw=lkjasdf

    psql: FATAL:  Password authentication failed for user "postgres"

Basically it echoes the failed password back to the user.  Again, this
is only with client_min_messages set to debug1-5.  I don't know how to
fix this because we specifically set things up so the client could see
everything the server logs see.  I wonder if echoing the failed password
into the logs is a good idea either.  I don't think so.

Someone please advise on patch application.  Are there other places that
don't expect a NOTICE in the middle of a protocol handshake?

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 853-3000
  +  If your life is a hard drive,     |  830 Blythe Avenue
  +  Christ can be your backup.        |  Drexel Hill, Pennsylvania 19026
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.182
diff -c -r1.182 fe-connect.c
*** src/interfaces/libpq/fe-connect.c    2 Mar 2002 00:49:22 -0000    1.182
--- src/interfaces/libpq/fe-connect.c    3 Mar 2002 02:33:51 -0000
***************
*** 1296,1301 ****
--- 1296,1310 ----
                      return PGRES_POLLING_READING;
                  }

+                 /* Grab NOTICE/INFO/DEBUG and discard them. */
+                 while (beresp == 'N')
+                 {
+                     if (getNotice(conn))
+                         return PGRES_POLLING_READING;
+                     if (pqGetc(&beresp, conn))
+                         return PGRES_POLLING_READING;
+                 }
+
                  /* Handle errors. */
                  if (beresp == 'E')
                  {
***************
*** 1314,1319 ****
--- 1323,1337 ----
                       */
                      appendPQExpBufferChar(&conn->errorMessage, '\n');
                      goto error_return;
+                 }
+
+                 /* Grab NOTICE/INFO/DEBUG and discard them. */
+                 while (beresp == 'N')
+                 {
+                     if (getNotice(conn))
+                         return PGRES_POLLING_READING;
+                     if (pqGetc(&beresp, conn))
+                         return PGRES_POLLING_READING;
                  }

                  /* Otherwise it should be an authentication request. */
Index: src/interfaces/libpq/fe-exec.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-exec.c,v
retrieving revision 1.113
diff -c -r1.113 fe-exec.c
*** src/interfaces/libpq/fe-exec.c    25 Oct 2001 05:50:13 -0000    1.113
--- src/interfaces/libpq/fe-exec.c    3 Mar 2002 02:33:52 -0000
***************
*** 54,60 ****
  static int    getRowDescriptions(PGconn *conn);
  static int    getAnotherTuple(PGconn *conn, int binary);
  static int    getNotify(PGconn *conn);
- static int    getNotice(PGconn *conn);

  /* ---------------
   * Escaping arbitrary strings to get valid SQL strings/identifiers.
--- 54,59 ----
***************
*** 1379,1385 ****
   * Exit: returns 0 if successfully consumed Notice message.
   *         returns EOF if not enough data.
   */
! static int
  getNotice(PGconn *conn)
  {
      /*
--- 1378,1384 ----
   * Exit: returns 0 if successfully consumed Notice message.
   *         returns EOF if not enough data.
   */
! int
  getNotice(PGconn *conn)
  {
      /*
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.80
diff -c -r1.80 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h    8 Nov 2001 20:37:52 -0000    1.80
--- src/interfaces/libpq/libpq-fe.h    3 Mar 2002 02:33:56 -0000
***************
*** 252,257 ****
--- 252,258 ----
  extern size_t PQescapeString(char *to, const char *from, size_t length);
  extern unsigned char *PQescapeBytea(unsigned char *bintext, size_t binlen,
                size_t *bytealen);
+ extern int    getNotice(PGconn *conn);

  /* Simple synchronous query */
  extern PGresult *PQexec(PGconn *conn, const char *query);

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: elog() patch
Next
From: "Rod Taylor"
Date:
Subject: plpgsql Field of Record issue