Thread: Time to get rid of PQnoPasswordSupplied?
Hi all I frequently see users confused by one of our more common and less clear error messages: fe_sendauth: no password supplied What this really means is that the server requested a password for md5 or cleartext authentication but no password was supplied to the client and it cannot prompt for one in this context. I'd like to get rid of it. It's clear others have wanted to in the past, since it's a backward compat #define in libpq-fe.h : /* Error when no password was given. */ /* Note: depending on this is deprecated; use PQconnectionNeedsPassword(). */ #define PQnoPasswordSupplied "fe_sendauth: no password supplied\n" but given the git blame for it: 4f9bf7fc (Tom Lane 2007-12-09 19:01:40 +0000 493) /* Note: depending on this is deprecated; use PQconnectionNeedsPassword(). */ 88fd162e (Bruce Momjian 2004-10-16 03:10:17 +0000 494) #define PQnoPasswordSupplied "fe_sendauth: no password supplied\n" I'm wondering if it's time for this to go away, so we can have a decent error message for this common error. It's been deprecated for eight years. How about: "The server requested a password but no password was supplied to the client" ? I'm sure it can be better than that, but ... well, it's not "fe_sendauth: no password supplied". -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On Sun, Jun 14, 2015 at 11:34 PM, Craig Ringer <craig@2ndquadrant.com> wrote: > I frequently see users confused by one of our more common and less > clear error messages: > > fe_sendauth: no password supplied I've never seen this error message, but I'm not opposed to improving it in some way. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Jun 14, 2015 at 11:34 PM, Craig Ringer <craig@2ndquadrant.com> wrote:
>> I frequently see users confused by one of our more common and less
>> clear error messages:
>>
>> fe_sendauth: no password supplied
> I've never seen this error message, but I'm not opposed to improving
> it in some way.
The reason you don't see it is that psql and other clients take special
action (ie, prompt you for a password) instead of actually showing you
that error string when it's returned by libpq.
The reason the string is considered part of libpq's external API is that
once upon a time, strcmp() to that particular string value was the only
way for clients to realize that prompt-for-a-password was the appropriate
response. Nowadays you're supposed to use PQconnectionNeedsPassword()
instead.
Apparently, Craig is responsible for some client-side code that has never
heard of either convention and just fails instead of prompting for a
password. So improving that might be something to put on somebody's
to-do list.
However, the question for us is whether there are any clients that still
use strcmp() to the PQnoPasswordSupplied literal, and so would be broken
if we change it. My guess is yes, but I'd still be on board with
replacing the string with something more message-style-guide-compliant
(and localizable). PQnoPasswordSupplied has been marked as deprecated
since 2007.
On the other hand, you could argue that improving the string is going
to break clients that do the right thing (even if klugily) in order
to help clients that are doing the wrong thing (ie, failing without
offering the opportunity to enter a password). Ideally no client app
would ever show this message to users and so its readability would not
matter.
regards, tom lane
On 19 June 2015 at 22:35, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Sun, Jun 14, 2015 at 11:34 PM, Craig Ringer <craig@2ndquadrant.com> wrote: >>> I frequently see users confused by one of our more common and less >>> clear error messages: >>> >>> fe_sendauth: no password supplied > >> I've never seen this error message, but I'm not opposed to improving >> it in some way. > > The reason you don't see it is that psql and other clients take special > action (ie, prompt you for a password) instead of actually showing you > that error string when it's returned by libpq. > Apparently, Craig is responsible for some client-side code that has never > heard of either convention and just fails instead of prompting for a > password. So improving that might be something to put on somebody's > to-do list. I'm not responsible for clients that do this, I just interact with enough communities outside -hackers that I see new users confused by such clients a *lot*. It's very common for most libpq based client libraries, like Python's psycopg2, PHP's pg_connect, etc. Have a look at https://www.google.com.au/?q=%22fe_sendauth%3A%20no%20password%20supplied%22 to get an idea of the confusion. Unfortunately, after some more thought I don't think we can remove it until the next protocol rev. Sure, the symbol can be removed from the libpq headers since (as below) there's almost nobody using it, but we can't actually change the string sent by the server so it does no good. So it's really a matter of fixing individual clients to actually use PQconnectionNeedsPassword(...) . The main reason the string the server sends can't be changed is that there will be old copies of libpq floating around for ages, and a code search suggests that there are also lots and lots of copies of libpq in various random projects' source trees. For anyone else looking at this later, I've had a look at users of the API. PgAdmin-III still uses the obsolete API and should be patched. It's the only reasonably relevant user of PQnoPasswordSupplied that I found. Newer versions of Ruby's 'Pg' gem seem to use PQconnectionNeedsPassword and make no reference to PQnoPasswordSupplied or a hardcoded string equivalent. Older versions didn't reference either and just passed the raw message to the user. RPostgreSQL also uses PQconnectionNeedsPassword . psycopg2, PHP's pg_connect and PHP's PDO driver for pg, libdbi, and Perl's DBD::Pg do no password-required detection and just spit "fe_sendauth: no password supplied" out at the user. There's no reference to PQnoPasswordSupplied, PQconnectionNeedsPassword or fe_sendauth in their source trees. None of these clients will care about the change. Some of them should be taught to use PQconnectionNeedsPassword, of course, but that's separate. A search of http://code.openhub.net/search?s=%22PQnoPasswordSupplied%22 suggests that few users of PQnoPasswordSupplied exist, and most of those are copies of libpq. > On the other hand, you could argue that improving the string is going > to break clients that do the right thing (even if klugily) in order > to help clients that are doing the wrong thing (ie, failing without > offering the opportunity to enter a password). Ideally no client app > would ever show this message to users and so its readability would not > matter. Yes. I think it's basically part of the v3 protocol and we're stuck with it until the next protocol rev. I'll poke client driver writers instead. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
On 6/19/15 10:35 AM, Tom Lane wrote: > On the other hand, you could argue that improving the string is going > to break clients that do the right thing (even if klugily) in order > to help clients that are doing the wrong thing (ie, failing without > offering the opportunity to enter a password). Ideally no client app > would ever show this message to users and so its readability would not > matter. Could we return a HINT? Or is that part of the same string? I agree that it's probably not worth breaking people that are doing the right thing. Perhaps this could be better documented, though I don't know where we'd put it (I doubt users would go looking in the libpq api docs to find it...) -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Data in Trouble? Get it in Treble! http://BlueTreble.com
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> On 6/19/15 10:35 AM, Tom Lane wrote:
>> On the other hand, you could argue that improving the string is going
>> to break clients that do the right thing (even if klugily) in order
>> to help clients that are doing the wrong thing (ie, failing without
>> offering the opportunity to enter a password). Ideally no client app
>> would ever show this message to users and so its readability would not
>> matter.
> Could we return a HINT? Or is that part of the same string?
Unfortunately no, there's no out-of-band additions possible here.
It strikes me that my argument above is too myopic, because I was only
thinking about cases where the client can plausibly do an interactive
prompt for password. If it cannot (eg, psql --no-password, or perhaps
the process has no controlling terminal) then what you're going to see
is whatever message libpq returns. So if people feel that this message
is not clear enough, maybe it's time to break compatibility and change it.
I do not follow Craig's argument that this is somehow connected to the
wire protocol version. It's not; it's part of the libpq-to-client API.
If there ever is a protocol version 4, it will almost certainly not
trigger significant changes in that API --- there might be additions,
but not incompatible changes. So if we think we can't change that
message now, then face it, we never will.
regards, tom lane
On 6/22/15 9:00 AM, Tom Lane wrote: > Jim Nasby <Jim.Nasby@BlueTreble.com> writes: >> On 6/19/15 10:35 AM, Tom Lane wrote: >>> On the other hand, you could argue that improving the string is going >>> to break clients that do the right thing (even if klugily) in order >>> to help clients that are doing the wrong thing (ie, failing without >>> offering the opportunity to enter a password). Ideally no client app >>> would ever show this message to users and so its readability would not >>> matter. > >> Could we return a HINT? Or is that part of the same string? > > Unfortunately no, there's no out-of-band additions possible here. > > It strikes me that my argument above is too myopic, because I was only > thinking about cases where the client can plausibly do an interactive > prompt for password. If it cannot (eg, psql --no-password, or perhaps > the process has no controlling terminal) then what you're going to see > is whatever message libpq returns. So if people feel that this message > is not clear enough, maybe it's time to break compatibility and change it. > > I do not follow Craig's argument that this is somehow connected to the > wire protocol version. It's not; it's part of the libpq-to-client API. > If there ever is a protocol version 4, it will almost certainly not > trigger significant changes in that API --- there might be additions, > but not incompatible changes. So if we think we can't change that > message now, then face it, we never will. Yeah, looking at Craig's extensive review, it seems most of those places wouldn't actually prompt for a password, so I think it's best that we just change this. -- Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX Data in Trouble? Get it in Treble! http://BlueTreble.com
On 22 June 2015 at 22:00, Tom Lane <tgl@sss.pgh.pa.us> wrote: > I do not follow Craig's argument that this is somehow connected to the > wire protocol version. Upon revisiting it, neither do I. You know when you read code and think "what idiot wrote this" ... then "git blame" says it was you? I, at least, know that feeling... and that's what reading that part of that email was like. Tom's right, of course. It's libpq-to-client. The string "fe_sendauth: no password supplied" never goes over the wire; it's a response libpq generates in response to an auth failure ErrorResponse message from the server. It's only relevant for libpq-based clients. Lets change it. PgAdmin-III will need a patch, but that's about the only client I found that would care. -- Craig Ringer http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services