Tom Lane wrote:
> Bruce Momjian <br...@momjian.us> writes:
> > Alvaro Herrera wrote:
> >> This bit seems strange ...  I think we just do strcmp() to compare 
> >> sqlstates?
> 
> > Uh, the PQresultErrorField is a string, while
> > ERRCODE_INVALID_PASSWORD_SPECIFICATION is a 4-byte value in base-64.  
> > Yea, it's true.  For excitement, see the MAKE_SQLSTATE macro.
> 
> I don't particularly believe in doing things this way: I think that
> libpq should just hardwire the string the same as it's doing for the
> other specific sqlstate it's checking for.  If we do this at all,
> then we are effectively embedding that sqlstate value in the protocol.
> We are not going to be able to change it later.  Doing it like this
> opens us up to randomly using errcodes in the frontend without realizing
> that we've thereby lost the freedom to change them.
> 
> Even if you insist on including errcodes.h into the frontend code
> despite that, this is an ugly brute-force way of solving the problem.
> The intended way of solving the problem was to redefine MAKE_SQLSTATE
> before including the file, in .c files where you need a different
> representation.

OK, just defined it as a constant string. I am not a big fan of
redefining macros, especially ones that are referenced in later include
files.

Is this going to cause problems for client applications that only test
for ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION?  I doubt many of them
are testing for just the first two digits.

Is there anywhere else we should be testing for this new sqlstate value?

Updated patch attached.

-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
Index: doc/src/sgml/errcodes.sgml
===================================================================
RCS file: /cvsroot/pgsql/doc/src/sgml/errcodes.sgml,v
retrieving revision 1.28
diff -c -c -r1.28 errcodes.sgml
*** doc/src/sgml/errcodes.sgml	7 Dec 2009 05:22:21 -0000	1.28
--- doc/src/sgml/errcodes.sgml	11 Mar 2010 22:40:24 -0000
***************
*** 761,766 ****
--- 761,772 ----
  <entry>invalid_authorization_specification</entry>
  </row>
  
+ <row>
+ <entry><literal>28001</literal></entry>
+ <entry>INVALID PASSWORD SPECIFICATION</entry>
+ <entry>invalid_password_specification</entry>
+ </row>
+ 
  
  <row>
  <entry spanname="span13"><emphasis role="bold">Class 2B &mdash; Dependent Privilege Descriptors Still Exist</></entry>
Index: src/backend/libpq/auth.c
===================================================================
RCS file: /cvsroot/pgsql/src/backend/libpq/auth.c,v
retrieving revision 1.195
diff -c -c -r1.195 auth.c
*** src/backend/libpq/auth.c	26 Feb 2010 02:00:42 -0000	1.195
--- src/backend/libpq/auth.c	11 Mar 2010 22:40:24 -0000
***************
*** 232,238 ****
  auth_failed(Port *port, int status)
  {
  	const char *errstr;
! 
  	/*
  	 * If we failed due to EOF from client, just quit; there's no point in
  	 * trying to send a message to the client, and not much point in logging
--- 232,239 ----
  auth_failed(Port *port, int status)
  {
  	const char *errstr;
! 	int		errcode_return = ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION;
! 	
  	/*
  	 * If we failed due to EOF from client, just quit; there's no point in
  	 * trying to send a message to the client, and not much point in logging
***************
*** 269,274 ****
--- 270,277 ----
  		case uaMD5:
  		case uaPassword:
  			errstr = gettext_noop("password authentication failed for user \"%s\"");
+ 			/* We use it to indicate if a .pgpass password failed. */
+ 			errcode_return = ERRCODE_INVALID_PASSWORD_SPECIFICATION;
  			break;
  		case uaPAM:
  			errstr = gettext_noop("PAM authentication failed for user \"%s\"");
***************
*** 285,291 ****
  	}
  
  	ereport(FATAL,
! 			(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
  			 errmsg(errstr, port->user_name)));
  	/* doesn't return */
  }
--- 288,294 ----
  	}
  
  	ereport(FATAL,
! 			(errcode(errcode_return),
  			 errmsg(errstr, port->user_name)));
  	/* doesn't return */
  }
Index: src/include/utils/errcodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/include/utils/errcodes.h,v
retrieving revision 1.31
diff -c -c -r1.31 errcodes.h
*** src/include/utils/errcodes.h	2 Jan 2010 16:58:10 -0000	1.31
--- src/include/utils/errcodes.h	11 Mar 2010 22:40:25 -0000
***************
*** 194,199 ****
--- 194,200 ----
  
  /* Class 28 - Invalid Authorization Specification */
  #define ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION MAKE_SQLSTATE('2','8', '0','0','0')
+ #define ERRCODE_INVALID_PASSWORD_SPECIFICATION MAKE_SQLSTATE('2','8', '0','0','1')
  
  /* Class 2B - Dependent Privilege Descriptors Still Exist */
  #define ERRCODE_DEPENDENT_PRIVILEGE_DESCRIPTORS_STILL_EXIST		MAKE_SQLSTATE('2','B', '0','0','0')
Index: src/interfaces/libpq/fe-connect.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-connect.c,v
retrieving revision 1.389
diff -c -c -r1.389 fe-connect.c
*** src/interfaces/libpq/fe-connect.c	3 Mar 2010 20:31:09 -0000	1.389
--- src/interfaces/libpq/fe-connect.c	11 Mar 2010 22:40:25 -0000
***************
*** 91,96 ****
--- 91,99 ----
   */
  #define ERRCODE_APPNAME_UNKNOWN "42704"
  
+ /* This is part of the protocol so just define it */
+ #define ERRCODE_INVALID_PASSWORD_SPECIFICATION "28001"
+ 
  /*
   * fall back options if they are not specified by arguments or defined
   * by environment variables
***************
*** 284,289 ****
--- 287,293 ----
  static char *pwdfMatchesString(char *buf, char *token);
  static char *PasswordFromFile(char *hostname, char *port, char *dbname,
  				 char *username);
+ static void dot_pg_pass_warning(PGconn *conn);
  static void default_threadlock(int acquire);
  
  
***************
*** 652,657 ****
--- 656,663 ----
  										conn->dbName, conn->pguser);
  		if (conn->pgpass == NULL)
  			conn->pgpass = strdup(DefaultPassword);
+ 		else
+ 			conn->dot_pgpass_used = true;
  	}
  
  	/*
***************
*** 2133,2138 ****
--- 2139,2146 ----
  
  error_return:
  
+ 	dot_pg_pass_warning(conn);
+ 	
  	/*
  	 * We used to close the socket at this point, but that makes it awkward
  	 * for those above us if they wish to remove this socket from their own
***************
*** 2191,2196 ****
--- 2199,2205 ----
  	conn->verbosity = PQERRORS_DEFAULT;
  	conn->sock = -1;
  	conn->password_needed = false;
+ 	conn->dot_pgpass_used = false;
  #ifdef USE_SSL
  	conn->allow_ssl_try = true;
  	conn->wait_ssl_try = false;
***************
*** 4426,4431 ****
--- 4435,4459 ----
  #undef LINELEN
  }
  
+ 
+ /*
+  *	If the connection failed, we should mention if
+  *	we got the password from .pgpass in case that
+  *	password is wrong.
+  */
+ static void
+ dot_pg_pass_warning(PGconn *conn)
+ {
+ 	/* If it was 'invalid authorization', add .pgpass mention */
+ 	if (conn->dot_pgpass_used && conn->password_needed && conn->result &&
+ 		/* only works with >= 9.0 servers */
+ 		strcmp(PQresultErrorField(conn->result, PG_DIAG_SQLSTATE),
+ 			ERRCODE_INVALID_PASSWORD_SPECIFICATION) == 0)
+ 		appendPQExpBufferStr(&conn->errorMessage,
+ 			libpq_gettext("password retrieved from .pgpass\n"));
+ }
+ 
+ 	
  /*
   * Obtain user's home directory, return in given buffer
   *
Index: src/interfaces/libpq/libpq-int.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/libpq-int.h,v
retrieving revision 1.149
diff -c -c -r1.149 libpq-int.h
*** src/interfaces/libpq/libpq-int.h	26 Feb 2010 02:01:33 -0000	1.149
--- src/interfaces/libpq/libpq-int.h	11 Mar 2010 22:40:25 -0000
***************
*** 343,348 ****
--- 343,349 ----
  	ProtocolVersion pversion;	/* FE/BE protocol version in use */
  	int			sversion;		/* server version, e.g. 70401 for 7.4.1 */
  	bool		password_needed;	/* true if server demanded a password */
+ 	bool		dot_pgpass_used;	/* true if used .pgpass */
  	bool		sigpipe_so;		/* have we masked SIGPIPE via SO_NOSIGPIPE? */
  	bool		sigpipe_flag;	/* can we mask SIGPIPE via MSG_NOSIGNAL? */
  
Index: src/pl/plpgsql/src/plerrcodes.h
===================================================================
RCS file: /cvsroot/pgsql/src/pl/plpgsql/src/plerrcodes.h,v
retrieving revision 1.20
diff -c -c -r1.20 plerrcodes.h
*** src/pl/plpgsql/src/plerrcodes.h	2 Jan 2010 16:58:13 -0000	1.20
--- src/pl/plpgsql/src/plerrcodes.h	11 Mar 2010 22:40:25 -0000
***************
*** 368,373 ****
--- 368,377 ----
  },
  
  {
+ 	"invalid_password_specification", ERRCODE_INVALID_PASSWORD_SPECIFICATION
+ },
+ 
+ {
  	"dependent_privilege_descriptors_still_exist", ERRCODE_DEPENDENT_PRIVILEGE_DESCRIPTORS_STILL_EXIST
  },
  
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to