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 <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

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

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 <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ Everyone has their own god. +

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

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 <bruce@momjian.us> http://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