Incorrect error message in InitializeSessionUserId
Hi all,
I have found incorrect error message in InitializeSessionUserId function
if you try to connect to database by role Oid (for example BackgroundWorkerInitializeConnectionByOid).
If role have no permissions to login, you will see error message like this:
FATAL: role "(null)" is not permitted to log in
I changed few lines of code and fixed this.
Patch is attached.
I want to add this patch to commitfest.
Any objections?
Regards,
Dmitriy Sarafannikov
Attachments:
miscinit.patchapplication/x-patch; name="=?UTF-8?B?bWlzY2luaXQucGF0Y2g=?="Download
diff --git a/src/backend/utils/init/miscinit.c b/src/backend/utils/init/miscinit.c
index 603a256..72fddc5 100644
--- a/src/backend/utils/init/miscinit.c
+++ b/src/backend/utils/init/miscinit.c
@@ -474,6 +474,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
{
HeapTuple roleTup;
Form_pg_authid rform;
+ char *rname;
/*
* Don't do scans if we're bootstrapping, none of the system catalogs
@@ -485,16 +486,25 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
AssertState(!OidIsValid(AuthenticatedUserId));
if (rolename != NULL)
+ {
roleTup = SearchSysCache1(AUTHNAME, PointerGetDatum(rolename));
+ if (!HeapTupleIsValid(roleTup))
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+ errmsg("role \"%s\" does not exist", rolename)));
+ }
else
+ {
roleTup = SearchSysCache1(AUTHOID, ObjectIdGetDatum(roleid));
- if (!HeapTupleIsValid(roleTup))
- ereport(FATAL,
- (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
- errmsg("role \"%s\" does not exist", rolename)));
+ if (!HeapTupleIsValid(roleTup))
+ ereport(FATAL,
+ (errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
+ errmsg("role (OID = %u) does not exist", roleid)));
+ }
rform = (Form_pg_authid) GETSTRUCT(roleTup);
roleid = HeapTupleGetOid(roleTup);
+ rname = NameStr(rform->rolname);
AuthenticatedUserId = roleid;
AuthenticatedUserIsSuperuser = rform->rolsuper;
@@ -520,7 +530,7 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
ereport(FATAL,
(errcode(ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION),
errmsg("role \"%s\" is not permitted to log in",
- rolename)));
+ rname)));
/*
* Check connection limit for this role.
@@ -538,11 +548,11 @@ InitializeSessionUserId(const char *rolename, Oid roleid)
ereport(FATAL,
(errcode(ERRCODE_TOO_MANY_CONNECTIONS),
errmsg("too many connections for role \"%s\"",
- rolename)));
+ rname)));
}
/* Record username and superuser status as GUC settings too */
- SetConfigOption("session_authorization", rolename,
+ SetConfigOption("session_authorization", rname,
PGC_BACKEND, PGC_S_OVERRIDE);
SetConfigOption("is_superuser",
AuthenticatedUserIsSuperuser ? "on" : "off",
On Tue, Mar 1, 2016 at 10:21 PM, Dmitriy Sarafannikov
<d.sarafannikov@bk.ru> wrote:
I have found incorrect error message in InitializeSessionUserId function
if you try to connect to database by role Oid (for example
BackgroundWorkerInitializeConnectionByOid).
If role have no permissions to login, you will see error message like this:
FATAL: role "(null)" is not permitted to log inI changed few lines of code and fixed this.
Patch is attached.
I want to add this patch to commitfest.
Any objections?
Not really. That looks like an oversight to me.
--
Michael
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Mar 2, 2016 at 12:21 AM, Dmitriy Sarafannikov
<d.sarafannikov@bk.ru> wrote:
Hi all,
I have found incorrect error message in InitializeSessionUserId function
if you try to connect to database by role Oid (for example
BackgroundWorkerInitializeConnectionByOid).
If role have no permissions to login, you will see error message like this:
FATAL: role "(null)" is not permitted to log inI changed few lines of code and fixed this.
Patch is attached.
I want to add this patch to commitfest.
Any objections?
The patch adds the support of taking the role name from the role tuple
instead of using the provided rolename variable, because it is possible
that rolename variable is NULL if the connection is from a background
worker.
The patch is fine, I didn't find any problems, I marked it as ready for
committer.
IMO this patch may need to backpatch supported branches as it is
a bug fix. Committer can decide.
Regards,
Hari Babu
Fujitsu Australia
--
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, Mar 4, 2016 at 10:45 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
On Wed, Mar 2, 2016 at 12:21 AM, Dmitriy Sarafannikov
<d.sarafannikov@bk.ru> wrote:Hi all,
I have found incorrect error message in InitializeSessionUserId function
if you try to connect to database by role Oid (for example
BackgroundWorkerInitializeConnectionByOid).
If role have no permissions to login, you will see error message like this:
FATAL: role "(null)" is not permitted to log inI changed few lines of code and fixed this.
Patch is attached.
I want to add this patch to commitfest.
Any objections?The patch adds the support of taking the role name from the role tuple
instead of using the provided rolename variable, because it is possible
that rolename variable is NULL if the connection is from a background
worker.The patch is fine, I didn't find any problems, I marked it as ready for
committer.IMO this patch may need to backpatch supported branches as it is
a bug fix. Committer can decide.
+1 for the backpatch. The current error message should the rolename be
undefined in this context is misleading for users.
--
Michael--
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, Mar 4, 2016, 5:23 +03:00 от Michael Paquier < michael.paquier@gmail.com >:
The patch adds the support of taking the role name from the role tuple
instead of using the provided rolename variable, because it is possible
that rolename variable is NULL if the connection is from a background
worker.The patch is fine, I didn't find any problems, I marked it as ready for
committer.IMO this patch may need to backpatch supported branches as it is
a bug fix. Committer can decide.+1 for the backpatch. The current error message should the rolename be undefined in this context is misleading for users. -- Michael
Thanks for feedback.
This patch successfully applies to the 9.5 branch.
In 9.4 and below versions InitializeSessionUserId function has other signature:
void InitializeSessionUserId(const char *rolename)
and it is impossible to pass role Oid to this function.
In this way, the patch is relevant only to the master and 9.5 branches
Regards,
Dmitriy Sarafannikov
--
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, Mar 3, 2016 at 9:23 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
On Fri, Mar 4, 2016 at 10:45 AM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:On Wed, Mar 2, 2016 at 12:21 AM, Dmitriy Sarafannikov
<d.sarafannikov@bk.ru> wrote:Hi all,
I have found incorrect error message in InitializeSessionUserId function
if you try to connect to database by role Oid (for example
BackgroundWorkerInitializeConnectionByOid).
If role have no permissions to login, you will see error message like this:
FATAL: role "(null)" is not permitted to log inI changed few lines of code and fixed this.
Patch is attached.
I want to add this patch to commitfest.
Any objections?The patch adds the support of taking the role name from the role tuple
instead of using the provided rolename variable, because it is possible
that rolename variable is NULL if the connection is from a background
worker.The patch is fine, I didn't find any problems, I marked it as ready for
committer.IMO this patch may need to backpatch supported branches as it is
a bug fix. Committer can decide.+1 for the backpatch. The current error message should the rolename be
undefined in this context is misleading for users.
Back-patched to 9.5. I don't think this is relevant for earlier branches.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers