On 2021-01-12 06:53, Ian Lawrence Barwick wrote:
> postgres=# SELECT has_column_privilege('foo', 999::int2, 'SELECT');
> has_column_privilege
> ----------------------
> t
> (1 row)
>
> The comment on the relevant code section in "src/backend/utils/adt/acl.c"
> (related to "column_privilege_check()") indicate that NULL is the intended
> return value in these cases:
>
> Likewise, the variants that take an integer attnum
> return NULL (rather than throwing an error) if there is no such
> pg_attribute entry. All variants return NULL if an attisdropped
> column is selected.
>
> The unexpected "TRUE" value is a result of "column_privilege_check()" returning
> TRUE if the user has table-level privileges. This returns a valid result with
> the function variants where the column name is specified, as the calling
> function will have already performed a check of the column through its call to
> "convert_column_name()". However when the attnum is specified, the status of
> the column never gets checked.
I'm not convinced the current behavior is wrong. Is there some
practical use case that is affected by this behavior?
I was poking around at the function with a view to using it for something and was
curious what it did with bad input.
Providing the column name of a dropped column:
Me: "Hey Postgres, do I have privileges on the dropped column 'bar' of my table 'foo'?"
Pg: "That column doesn't even exist - here, have an error instead."
Me: "Hey Postgres, does some other less-privileged user have privileges on the
dropped column 'bar' of my table 'foo'?
Pg: "That column doesn't even exist - here, have an error instead."
Providing the attnum of a dropped column:
Me: "Hey Postgres, here's the attnum of the dropped column 'bar', does some
other less-privileged user have privileges on that column?"
Pg: "That column doesn't even exist - here, have a NULL".
Me: "Hey Postgres, here's the attnum of the dropped column 'bar' on this table
I own, do I have privileges on that column?"
Pg: "Yup. And feel free to throw any other smallint at me, I'll pretend that
represents a column too even if it never existed.".
Looking at the code, particularly the cited comment, it does seems the intent was
to return NULL in all cases where an invalid attnum was provided, but that gets
short-circuited by the assumption table owner = has privilege on any column.
Not the most urgent or exciting of issues, but seems inconsistent to me.
> The second patch adds a bunch of missing static prototypes to "acl.c",
> on general
> principles.
Why is this necessary?
Not exactly necessary, but seemed odd some functions had prototypes, others not.
I have no idea what the policy is, if any, and not something I would lose sleep over,
just thought I'd note it in passing.
