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.

Sign up for GitHub

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

Jump to bottom

Use and respect the passfile connection parameter #1129

Merged
merged 2 commits into from
Jul 7, 2023
Merged

Use and respect the passfile connection parameter #1129

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
29f3a40
Use and respect the passfile connection parameter
keymon Jun 9, 2023
eb3a56e
Move parse of PGPASSFILE to parseEnviron
keymon Jun 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion conn.go
View file
Open in desktop
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:

pq/connector.go

Line 61 in 381d253

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
View file
Open in desktop
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