Re: improving user.c error messages
Nathan Bossart <nathandbossart@gmail.com>
From: Nathan Bossart <nathandbossart@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2023-01-27T23:15:07Z
Lists: pgsql-hackers
Attachments
- v4-0001-Improve-user.c-error-messages.patch (text/x-diff)
On Fri, Jan 27, 2023 at 07:31:19PM +0100, Alvaro Herrera wrote:
> On 2023-Jan-26, Nathan Bossart wrote:
>> ereport(ERROR,
>> (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
>> - errmsg("permission denied: bootstrap user must be superuser")));
>> + errmsg("permission denied to alter role"),
>> + errdetail("The bootstrap user must be superuser.")));
>
> I think this one isn't using the right errcode; this is not a case of
> insufficient privileges. There's no priv you can acquire that lets you
> do it. So I'd change it to unsupported operation.
І fixed this in v4. I've also attached a second patch in which I've
adjusted the messages that Peter mentioned upthread.
One thing that feels a bit odd is how some of the DETAILs mention the
operation being attempted while others do not. For example, we have
ERROR: permission denied to drop role
DETAIL: You must have SUPERUSER privilege to drop roles with SUPERUSER.
In this case, the DETAIL explains the action that is prohibited. In other
cases, we have something like
ERROR: permission denied to alter role
DETAIL: You must have CREATEROLE privilege and ADMIN OPTION on role "myrole".
which does not. I think this is okay because adding "to alter the role" to
the end of the DETAIL seems kind of awkward. But in other cases, such as
ERROR: permission denied to use replication slots
DETAIL: You must have REPLICATION privilege.
adding the operation to the end seems less awkward (i.e., "You must have
REPLICATION privilege to use replication slots."). I don't think there's
any information lost by omitting the action in the DETAIL, so perhaps this
is just a stylistic choice. I think I'm inclined to add the action to the
DETAIL whenever it doesn't make the message lengthy and awkward, and leave
it out otherwise. Thoughts?
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
Commits
Same data as JSON:
GET /api/v1/messages/:b64id/commits
the thread's linked commits as JSON, with link sources.
API reference →
-
Improve several permission-related error messages.
- de4d456b406b 16.0 landed
-
Integrate superuser check into has_rolreplication()
- 442f8700656b 16.0 landed
-
Small code simplification
- 3b7cd8c690f2 16.0 landed
-
Adjust interaction of CREATEROLE with role properties.
- f1358ca52dd7 16.0 landed
-
Add new GUC reserved_connections.
- 6e2775e4d4e4 16.0 landed
-
Rename ReservedBackends variable to SuperuserReservedConnections.
- fe00fec1f5d7 16.0 landed
-
Update docs and error message for superuser_reserved_connections.
- 6c1d5ba48678 16.0 landed
-
Add new GUC createrole_self_grant.
- e5b8a4c098ad 16.0 cited
-
Restrict the privileges of CREATEROLE users.
- cf5eb37c5ee0 16.0 cited
-
Add a SET option to the GRANT command.
- 3d14e171e9e2 16.0 cited