home directory lookup can fail

[kr]

Method setupSSLClientCertificates uses uses getpwuid_r (via user.Current) to find the user's home directory.

        // Automatically load certificates from ~/.postgresql.
        user, err := user.Current()
        if err != nil {
            // user.Current() might fail when cross-compiling.  We have to
            // ignore the error and continue without client certificates, since
            // we wouldn't know where to load them from.
            return
        }

It would be better to use the environment variable HOME (or, on windows, a combination of HOMEDRIVE and HOMEPATH). That way the user can control where it looks.

Avoiding os/user would also prevent having to call into cgo, which would make cross compiling a bit easier and behave more consistently. For example, the comment above about how the lookup "might fail" would no longer be necessary.

If this sounds ok, I'll send a patch.

[erykwalder]

+1

[kr]

Note also that we've experienced some unreliability with this cgo call. (This is not OOM, it appears to be some other failure.)

fatal error: C malloc failed

goroutine 2720 [running]:
runtime.throw(0x953900, 0xf)
    /usr/local/go/src/runtime/panic.go:527 +0x90 fp=0xcf8cf6f700 sp=0xcf8cf6f6e8
runtime.cmalloc(0x400, 0xcd223b6020)
    /usr/local/go/src/runtime/cgocall.go:148 +0x68 fp=0xcf8cf6f730 sp=0xcf8cf6f700
os/user._Cfunc__CMalloc(0x400, 0x400)
    ??:0 +0x21 fp=0xcf8cf6f748 sp=0xcf8cf6f730
os/user.lookupUnix(0x3e8, 0x0, 0x0, 0xc85554ef00, 0x0, 0x0, 0x0)
    /usr/local/go/src/os/user/lookup_unix.go:70 +0xaf fp=0xcf8cf6f8b8 sp=0xcf8cf6f748
os/user.current(0xcf8cf6f928, 0x0, 0x0)
    /usr/local/go/src/os/user/lookup_unix.go:39 +0x42 fp=0xcf8cf6f8f8 sp=0xcf8cf6f8b8
os/user.Current(0x8e6840, 0x0, 0x0)
    /usr/local/go/src/os/user/lookup.go:9 +0x24 fp=0xcf8cf6f918 sp=0xcf8cf6f8f8
chain/vendor/github.com/lib/pq.(*conn).setupSSLClientCertificates(0xc835ddef00, 0xcb1200a640, 0xcf8cf70248)
    /tmp/src/chain/vendor/github.com/lib/pq/conn.go:981 +0x478 fp=0xcf8cf6fbb0 sp=0xcf8cf6f918
chain/vendor/github.com/lib/pq.(*conn).ssl(0xc835ddef00, 0xcf8cf70248)
    /tmp/src/chain/vendor/github.com/lib/pq/conn.go:914 +0x1b4 fp=0xcf8cf6fda0 sp=0xcf8cf6fbb0
chain/vendor/github.com/lib/pq.DialOpen(0x7f61dac664b8, 0xd03ef0, 0xc83858f680, 0x93, 0x0, 0x0, 0x0, 0x0)
    /tmp/src/chain/vendor/github.com/lib/pq/conn.go:242 +0xdcf fp=0xcf8cf70448 sp=0xcf8cf6fda0
chain/vendor/github.com/lib/pq.Open(0xc82000e007, 0x7f, 0x0, 0x0, 0x0, 0x0)
    /tmp/src/chain/vendor/github.com/lib/pq/conn.go:155 +0x9e fp=0xcf8cf70490

It would be wonderful to avoid this crash in the future. :)

Another way we might dramatically reduce the incidence of this failure would be to store the results of setupSSLClientCertificates in memory, so lib/pq only does the work once, when the first connection is opened. I'm also happy to send a patch for that in addition to (or instead of) the env var suggestion above.

[kr]

For now, we are working around this with CGO_ENABLED=0.

[johto]

Note also that we've experienced some unreliability with this cgo call. (This is not OOM, it appears to be some other failure.)

fatal error: C malloc failed

It would be wonderful to avoid this crash in the future. :)

That looks like something that ought to be fixed elsewhere.

Another way we might dramatically reduce the incidence of this failure would be to store the results of setupSSLClientCertificates in memory, so lib/pq only does the work once, when the first connection is opened.

That sounds superficially reasonable.

I'm also happy to send a patch for that in addition to (or instead of) the env var suggestion above.

I think at least on POSIX we only need to look at $HOME. On Windows, though, I'm not so sure; does HOMEPATH work the same way as user.Current().HomeDir?

[kr]

Cool, two patches coming up!

I think at least on POSIX we only need to look at $HOME. On Windows, though, I'm not so sure; does HOMEPATH work the same way as user.Current().HomeDir?

Yes. As mentioned above, on unix we should use HOME, and on windows we should use the concatenation of HOMEDRIVE and HOMEPATH. See https://support.microsoft.com/en-us/kb/101507.

The default directory the Command Prompt displays is the equivalent of HOMEDRIVE plus HOMEPATH.

[kr]

Just opened #396. Env var patch still coming up.

[ahmetb]

@kr you can use %USERPROFILE% on windows (it's in WinXP+), no need to concat %HOMEDRIVE%+%HOMEPATH%. https://github.com/docker/docker/blob/master/pkg/homedir/homedir.go

[kostya-sh]

FYI, user.Current most likely will work without cgo in Go 1.7: golang/go@795e712

[kr]

FYI, user.Current most likely will work without cgo in Go 1.7

…and it uses os.Getenv("HOME") 😉