Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use and respect the passfile connection parameter #1129

Merged
merged 2 commits into from
Jul 7, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Use and respect the passfile connection parameter
The postgres documentation[1] regarding the password file, states
that:

password file to use can be specified using the connection parameter
passfile or the environment variable PGPASSFILE.

The current implementation of lib/pq only respects the environment
variable PGPASSFILE. This is not correct, but also limiting, as
the PGPASSFILE is global and we might want to use different files
for different clients in the same program.

Fixing that is easy, by just checking the parameter passfile first,
and if not, pull the value from PGPASSFILE.

[1] https://www.postgresql.org/docs/current/libpq-pgpass.html
  • Loading branch information
keymon committed Jun 9, 2023
commit 29f3a400f54725dfcc8a36253f4b14b938ce4135
6 changes: 5 additions & 1 deletion conn.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,11 @@ func (cn *conn) handlePgpass(o values) {
if _, ok := o["password"]; ok {
return
}
filename := os.Getenv("PGPASSFILE")
// Get passfile from the options, if empty, get it from envvar
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the correct place to handle the env var is here:

pq/conn.go

Line 2009 in 381d253

func parseEnviron(env []string) (out map[string]string) {

then the logic after parseEnviron is called will automatically take care of making the connection string parameter override the env var:

for k, v := range parseEnviron(os.Environ()) {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you are right, it makes sense. I didn't see that I focused in that function. Pushed a new commit.

filename := o["passfile"]
if filename == "" {
filename = os.Getenv("PGPASSFILE")
}
if filename == "" {
// XXX this code doesn't work on Windows where the default filename is
// XXX %APPDATA%\postgresql\pgpass.conf
Expand Down
6 changes: 5 additions & 1 deletion conn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ localhost:*:*:*:pass_C
if err != nil {
t.Fatalf("Unexpected error writing pgpass file %#v", err)
}
defer os.Remove(pgpassFile)
pgpass.Close()

assertPassword := func(extra values, expected string) {
Expand Down Expand Up @@ -221,8 +222,11 @@ localhost:*:*:*:pass_C
// localhost also matches the default "" and UNIX sockets
assertPassword(values{"host": "", "user": "some_user"}, "pass_C")
assertPassword(values{"host": "/tmp", "user": "some_user"}, "pass_C")
// passfile connection parameter takes precedence
os.Setenv("PGPASSFILE", "/tmp")
assertPassword(values{"host": "server", "dbname": "some_db", "user": "some_user", "passfile": pgpassFile}, "pass_A")

// cleanup
os.Remove(pgpassFile)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we still keep the cleanup step?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is in a defer next to where the file is created

os.Setenv("PGPASSFILE", "")
}

Expand Down
Loading