Re: [HACKERS] Sloppy SSPI error reporting code
On Fri, Apr 3, 2015 at 08:32:08PM -0400, Noah Misch wrote: On Thu, Apr 02, 2015 at 07:31:52AM -0400, Bruce Momjian wrote: On Thu, Apr 2, 2015 at 01:44:59AM -0400, Noah Misch wrote: On Wed, Apr 01, 2015 at 10:49:01PM -0400, Bruce Momjian wrote: On Sat, Jan 10, 2015 at 02:53:13PM -0500, Tom Lane wrote: While looking at fe-auth.c I noticed quite a few places that weren't bothering to make error messages localizable (ie, missing libpq_gettext calls), and/or were failing to add a trailing newline as expected in libpq error messages. Perhaps these are intentional but I doubt it. Most of the instances seemed to be SSPI-related. I have no intention of fixing these myself, but whoever committed that code should take a second look. I looked through that file and only found two cases; patch attached. Tom mentioned newline omissions, which you'll find in pg_SSPI_error(). Oh, I accidentally saw the backend version of that function, which looked fine. I have attached a patch for that. That patch looks reasonable. Patch applied. Thanks. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Sloppy SSPI error reporting code
On Thu, Apr 02, 2015 at 07:31:52AM -0400, Bruce Momjian wrote: On Thu, Apr 2, 2015 at 01:44:59AM -0400, Noah Misch wrote: On Wed, Apr 01, 2015 at 10:49:01PM -0400, Bruce Momjian wrote: On Sat, Jan 10, 2015 at 02:53:13PM -0500, Tom Lane wrote: While looking at fe-auth.c I noticed quite a few places that weren't bothering to make error messages localizable (ie, missing libpq_gettext calls), and/or were failing to add a trailing newline as expected in libpq error messages. Perhaps these are intentional but I doubt it. Most of the instances seemed to be SSPI-related. I have no intention of fixing these myself, but whoever committed that code should take a second look. I looked through that file and only found two cases; patch attached. Tom mentioned newline omissions, which you'll find in pg_SSPI_error(). Oh, I accidentally saw the backend version of that function, which looked fine. I have attached a patch for that. That patch looks reasonable. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Sloppy SSPI error reporting code
On Thu, Apr 2, 2015 at 01:44:59AM -0400, Noah Misch wrote: On Wed, Apr 01, 2015 at 10:49:01PM -0400, Bruce Momjian wrote: On Sat, Jan 10, 2015 at 02:53:13PM -0500, Tom Lane wrote: While looking at fe-auth.c I noticed quite a few places that weren't bothering to make error messages localizable (ie, missing libpq_gettext calls), and/or were failing to add a trailing newline as expected in libpq error messages. Perhaps these are intentional but I doubt it. Most of the instances seemed to be SSPI-related. I have no intention of fixing these myself, but whoever committed that code should take a second look. I looked through that file and only found two cases; patch attached. Tom mentioned newline omissions, which you'll find in pg_SSPI_error(). Oh, I accidentally saw the backend version of that function, which looked fine. I have attached a patch for that. ! printfPQExpBuffer(conn-errorMessage, libpq_gettext(SSPI returned invalid number of output buffers\n)); !libpq_gettext(fe_sendauth: error sending password authentication\n)); The first message is too internals-focused for a translation to be useful. If it were in the backend, we would use elog() and not translate. The second is a non-actionable message painting over a wide range of specific failures; translating it would just put lipstick on the pig. OK. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c new file mode 100644 index 8927df4..08cc906 *** a/src/interfaces/libpq/fe-auth.c --- b/src/interfaces/libpq/fe-auth.c *** pg_SSPI_error(PGconn *conn, const char * *** 236,245 if (FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, r, 0, sysmsg, sizeof(sysmsg), NULL) == 0) ! printfPQExpBuffer(conn-errorMessage, %s: SSPI error %x, mprefix, (unsigned int) r); else ! printfPQExpBuffer(conn-errorMessage, %s: %s (%x), mprefix, sysmsg, (unsigned int) r); } --- 236,245 if (FormatMessage(FORMAT_MESSAGE_FROM_SYSTEM, NULL, r, 0, sysmsg, sizeof(sysmsg), NULL) == 0) ! printfPQExpBuffer(conn-errorMessage, %s: SSPI error %x\n, mprefix, (unsigned int) r); else ! printfPQExpBuffer(conn-errorMessage, %s: %s (%x)\n, mprefix, sysmsg, (unsigned int) r); } -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Sloppy SSPI error reporting code
On Wed, Apr 01, 2015 at 10:49:01PM -0400, Bruce Momjian wrote: On Sat, Jan 10, 2015 at 02:53:13PM -0500, Tom Lane wrote: While looking at fe-auth.c I noticed quite a few places that weren't bothering to make error messages localizable (ie, missing libpq_gettext calls), and/or were failing to add a trailing newline as expected in libpq error messages. Perhaps these are intentional but I doubt it. Most of the instances seemed to be SSPI-related. I have no intention of fixing these myself, but whoever committed that code should take a second look. I looked through that file and only found two cases; patch attached. Tom mentioned newline omissions, which you'll find in pg_SSPI_error(). ! printfPQExpBuffer(conn-errorMessage, libpq_gettext(SSPI returned invalid number of output buffers\n)); ! libpq_gettext(fe_sendauth: error sending password authentication\n)); The first message is too internals-focused for a translation to be useful. If it were in the backend, we would use elog() and not translate. The second is a non-actionable message painting over a wide range of specific failures; translating it would just put lipstick on the pig. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Sloppy SSPI error reporting code
On Sat, Jan 10, 2015 at 02:53:13PM -0500, Tom Lane wrote: While looking at fe-auth.c I noticed quite a few places that weren't bothering to make error messages localizable (ie, missing libpq_gettext calls), and/or were failing to add a trailing newline as expected in libpq error messages. Perhaps these are intentional but I doubt it. Most of the instances seemed to be SSPI-related. I have no intention of fixing these myself, but whoever committed that code should take a second look. I looked through that file and only found two cases; patch attached. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c new file mode 100644 index 8927df4..c267f72 *** a/src/interfaces/libpq/fe-auth.c --- b/src/interfaces/libpq/fe-auth.c *** pg_SSPI_continue(PGconn *conn) *** 333,339 * authentication. Keep check in case it shows up with other * authentication methods later. */ ! printfPQExpBuffer(conn-errorMessage, SSPI returned invalid number of output buffers\n); return STATUS_ERROR; } --- 333,339 * authentication. Keep check in case it shows up with other * authentication methods later. */ ! printfPQExpBuffer(conn-errorMessage, libpq_gettext(SSPI returned invalid number of output buffers\n)); return STATUS_ERROR; } *** pg_fe_sendauth(AuthRequest areq, PGconn *** 691,697 if (pg_password_sendauth(conn, conn-pgpass, areq) != STATUS_OK) { printfPQExpBuffer(conn-errorMessage, ! fe_sendauth: error sending password authentication\n); return STATUS_ERROR; } break; --- 691,697 if (pg_password_sendauth(conn, conn-pgpass, areq) != STATUS_OK) { printfPQExpBuffer(conn-errorMessage, ! libpq_gettext(fe_sendauth: error sending password authentication\n)); return STATUS_ERROR; } break; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Sloppy SSPI error reporting code
While looking at fe-auth.c I noticed quite a few places that weren't bothering to make error messages localizable (ie, missing libpq_gettext calls), and/or were failing to add a trailing newline as expected in libpq error messages. Perhaps these are intentional but I doubt it. Most of the instances seemed to be SSPI-related. I have no intention of fixing these myself, but whoever committed that code should take a second look. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers