diff --git a/pgconn/errors.go b/pgconn/errors.go index f5fbd7834..9e3be79a2 100644 --- a/pgconn/errors.go +++ b/pgconn/errors.go @@ -289,7 +289,16 @@ func redactURLPassword(connString string) string { // Userinfo: same lookahead as parseURLSettings. if i := strings.IndexAny(p, "@/"); i >= 0 && p[i] == '@' { user, _, hasPassword := strings.Cut(p[:i], ":") - p = p[i+1:] + end := i + if hasPassword && !uriStructureUnambiguous(connString) { + // The parser ends the userinfo at the first '@', but in a + // malformed URI an unencoded password may itself contain '@' + // (user:sec@ret@host). Stopping at the first one would print + // the rest of the password ("ret") in clear text, so mask up + // to the '@' that most plausibly ends the userinfo instead. + end = brokenUserinfoEnd(p, i) + } + p = p[end+1:] b.WriteString(user) if hasPassword { b.WriteString(":" + mask) @@ -358,12 +367,17 @@ func redactURLPassword(connString string) string { // to be numeric or empty: the parser accepts arbitrary text in the port slot // (postgres://a@u:sec:ret@h parses with port "sec:ret@h"), so a non-numeric // port may be a mislaid password and keeps the string in best-effort -// territory. +// territory. The same holds for a host containing '@': the parser accepts +// postgres://user:sec@ret@h with host "ret@h", but no host name contains '@', +// so the '@' is a mislaid part of the password. func uriStructureUnambiguous(connString string) bool { settings, _, err := parseURLSettings(connString) if err != nil { return false } + if strings.IndexByte(settings["host"], '@') >= 0 { + return false + } for part := range strings.SplitSeq(settings["port"], ",") { for i := 0; i < len(part); i++ { if part[i] < '0' || part[i] > '9' { @@ -374,6 +388,24 @@ func uriStructureUnambiguous(connString string) bool { return true } +// brokenUserinfoEnd returns the index of the '@' that most plausibly +// terminates the userinfo of a malformed connection URI whose first '@' is at +// first: the last '@' before the last '?', so that an '@' inside a query value +// (application_name=me@example.com) is not mistaken for it; or, when no '@' +// follows first before that '?' (the '?' then belongs to the password, as in +// user:pass?x@host), the last '@' in p. Over-masking a malformed host is +// preferable to printing part of a password. +func brokenUserinfoEnd(p string, first int) int { + end := -1 + if q := strings.LastIndexByte(p, '?'); q > first { + end = strings.LastIndexByte(p[:q], '@') + } + if end <= first { + end = strings.LastIndexByte(p, '@') + } + return max(end, first) +} + // uriQueryStart returns the index of the '?' that begins the query component // of p (the post-scheme, post-userinfo part of a connection URI), or -1 if // there is none. It follows parseURLSettings' structure: '[' at the start of diff --git a/pgconn/errors_test.go b/pgconn/errors_test.go index 84d8807c5..760035b7e 100644 --- a/pgconn/errors_test.go +++ b/pgconn/errors_test.go @@ -83,6 +83,31 @@ func TestConfigError(t *testing.T) { err: pgconn.NewParseConfigError("postgres://host/db?password=super secret&sslmode=disable", "msg", nil), expectedMsg: "cannot parse `postgres://host/db?password=xxxxx&sslmode=disable`: msg", }, + { + name: "url with at sign inside the password", + err: pgconn.NewParseConfigError("postgres://user:sec@ret:more/x#y@host1:5432,host2:5432/db?sslmode=disable", "msg", nil), + expectedMsg: "cannot parse `postgres://user:xxxxx@host1:5432,host2:5432/db?sslmode=disable`: msg", + }, + { + name: "url with at sign and invalid escape in the password", + err: pgconn.NewParseConfigError("postgres://user:x%zz@secret@host/db", "msg", nil), + expectedMsg: "cannot parse `postgres://user:xxxxx@host/db`: msg", + }, + { + name: "url with at sign in the password that otherwise parses", + err: pgconn.NewParseConfigError("postgres://user:sec@ret@host/db?sslmode=bogus", "msg", nil), + expectedMsg: "cannot parse `postgres://user:xxxxx@host/db?sslmode=bogus`: msg", + }, + { + name: "url with at sign in the password and in a query value", + err: pgconn.NewParseConfigError("postgres://user:sec@ret@host/db?application_name=me@example.com&password=supersecret", "msg", nil), + expectedMsg: "cannot parse `postgres://user:xxxxx@host/db?application_name=me@example.com&password=xxxxx`: msg", + }, + { + name: "url with question mark and at sign in the password", + err: pgconn.NewParseConfigError("postgres://user:sec@ret?x@host/db", "msg", nil), + expectedMsg: "cannot parse `postgres://user:xxxxx@host/db`: msg", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {