Tom Lane wrote:
> NAK. This changes the behavior of connectDatabase() for *all* users
> of that function, not only vacuumdb. But the proposed behavioral
> change is only appropriate for calling programs in which only a single
> host/port/database target is used per execution. In other contexts,
> reusing the prior password is not just inappropriate but could actually
> create security issues. (It's possible that this behavior would be okay
> for all existing callers, but that doesn't mean we should put in a
> security gotcha for future uses.)
>
> We could make this approach work if connectDatabase() remembered all
> the parameters internally, and only tried to reuse the password when
> they all match. Or maybe it'd be better to alter the API so the caller
> can say whether to try to reuse a saved password or not. But I'm not sure
> whether either of those answers is cleaner than the previous patch.
Thanks for the input. I decided to push what we had because it's less
invasive in terms of API definition. If we want to change in the
direction suggested by Masao-san, we can still do it, but perhaps only
in master -- maybe we would like to have both pg_dump and
src/bin/scripts compile a single source code file instead of having two
copies of essentially the same routine, for instance.
> (BTW, I notice that pg_dumpall.c has a version of connectDatabase in
> which the "static" trick is already being used, sans any documentation.
> That's okay for pg_dumpall, but might be an issue if anyone copies-and-
> pastes that version somewhere else ... and in any case it's fair to ask
> why that version hasn't been merged with common.c.)
We'd have to have a file common to both subdirs that only contains that
routine, I think. We now have pg_dump/common.c as well ... Not
something I want to propose for 9.5.
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
