On Thu, Dec 24, 2015 at 4:36 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> As written, this would leak password strings, and it even seems possible
> that it would leave savedPassword pointing at dangling memory, since the
> free(password) inside the loop might free what savedPassword is pointing
> at, and then in principle we might fail to overwrite savedPassword
> afterwards. This probably can't happen in practice because it'd require
> successive connection attempts to come to different conclusions about
> PQconnectionNeedsPassword/PQconnectionUsedPassword.
Yes, that's what I was assuming.
> But it seems pretty
> fragile in the face of future changes to this code. I modified it further
> so that "password" and "savedPassword" never share storage, and pushed it.
OK, thanks for fixing the issue!
> A larger concern is that I suspect we need to abandon this whole approach
> of passing a different representation of the connection parameters than
> we were given the first time through. If dbname is a connection URI
> (or keyword=value connection string), it might well contain more
> information than just host + port + username + password. We're losing
> any such details during the workers' reconnections. But that looks like
> it would be a rather wide-ranging rewrite, so I just committed what we
> had for now; at least we fixed the reported bug symptom.
vacuumdb suffers the same symptoms I think...
--
Michael
