Re: BUG #15182: Canceling authentication due to timeout aka Denial of Service Attack

On Fri, Jul 27, 2018 at 12:37:02PM +0900, Kyotaro HORIGUCHI wrote:
> I intended the same as Bossart said. It is not reasonable that
> VACUUM and VACUUM FULL behaves differently on rejection for the
> same reason. The objective of the first (or early) check is
> rejecting (non-superuser's) unprivileged VACUUM FULL. Any
> possible modifications on a syscache tuple before taking a lock
> doesn't seem to harm in the point of view. Anyway the lock
> acquired in expand_vacuum_rel is not held until vacuum_rel.

Thinking about it harder, it is possible to pass down a status pointer
that the callback of RangeVarGetRelidExtended could fill if we pass it
down using the argument list. This would need special handling for the
case where expand_vacuum_rel() is NIL but that would be workable. If
vacuum_check_rel() does not need to work with elevel >= ERROR, then the
set of log messages remains the same.

> Apart from the discussion above, it is uneasy for me that the
> messages seem heavily affected by the callers context.

Sure, the patch for vacuum is still heavily WIP. I am not much happy
about that either.

>> Regarding those patches, I am pretty happy how things turn out for
>> TRUNCATE and REINDEX, way less for VACUUM, so getting 0001 and 0003
>> committed first makes the most sense to me as their logic is rather
>> straight-forward (well way of speaking ;p ).
>
> About 0001, I'm not sure what the "session-level" means but it
> looks fine and works as expected. 0003 looks fine overall.

What I mean here is checking the interactions with other backends,
truncate_check_session is the best name I could come up with.
truncate_check_activity was another. I am not attached to a single
name, if you have an ideaof course please feel free.

> I think I heard that gender-free wording is appreciated but it
> might be only about documentation.

Sure.
--
Michael