From ada86ca4a1cc5f0168a44baa52f02858a7ad7240 Mon Sep 17 00:00:00 2001 From: Jacob Quinn Date: Sun, 4 Oct 2026 10:08:37 -0600 Subject: [PATCH] Keep credentials out of connection string parse errors Validate keyword names before emitting syntax diagnostics and handle URI syntax/decoding separately from known-option validation. Raise the replacement after leaving catch so the original input-bearing exception cannot remain in the displayed chain. Cover public parsing, connection and pool entry points. AI disclosure: This work was prepared with assistance from OpenAI Codex. --- src/connection_string.jl | 34 ++++++++++++++++----- test/connection_string_errors.jl | 52 ++++++++++++++++++++++++++++++++ test/runtests.jl | 2 ++ 3 files changed, 80 insertions(+), 8 deletions(-) create mode 100644 test/connection_string_errors.jl diff --git a/src/connection_string.jl b/src/connection_string.jl index ddff7fb..f9f952f 100644 --- a/src/connection_string.jl +++ b/src/connection_string.jl @@ -165,10 +165,15 @@ end # dangerous: "ssl_mode=verify-full" would leave sslmode unset and fall back to # an unverified connection while the caller believes otherwise. libpq errors # on unknown keywords for the same reason. +function check_known_param(key::String) + (key in KNOWN_PARAMS || key in IGNORED_PARAMS) || + throw(ArgumentError("unrecognized connection parameter; recognized parameters are $(join(sort!(collect(KNOWN_PARAMS)), ", "))")) + return nothing +end + function check_known_params(values::Dict{String, String}) for (key, value) in values - (key in KNOWN_PARAMS || key in IGNORED_PARAMS) || - throw(ArgumentError("unrecognized connection parameter \"$key\"; recognized parameters are $(join(sort!(collect(KNOWN_PARAMS)), ", "))")) + check_known_param(key) key in IGNORED_PARAMS && check_ignored_param(key, value) end return values @@ -231,6 +236,8 @@ function parse_keyword_dsn(dsn::String) key_end = prevind(dsn, i) key = key_end < key_start ? "" : lowercase(String(dsn[key_start:key_end])) isempty(key) && throw(ArgumentError("empty connection parameter name")) + # A malformed DSN can put password text in the apparent key. + check_known_param(key) while i <= lastindex(dsn) && isspace(dsn[i]) i = nextind(dsn, i) end @@ -296,6 +303,9 @@ PostgreSQL URI (`"postgresql://user:pass@host:5432/dbname"`) into to the `PGHOST`, `PGPORT`, `PGUSER`, `PGPASSWORD`, `PGDATABASE`, `PGAPPNAME`, `PGCONNECT_TIMEOUT`, `PGSSL*`, `PGGSSENCMODE`, `PGKRBSRVNAME`, and `PGGSSDELEGATION` environment variables, then to defaults. + +Invalid URI syntax is reported as `ArgumentError` without retaining the +underlying parser's input-bearing exception. """ function parse_dsn(dsn::String) lowered = lowercase(dsn) @@ -304,6 +314,18 @@ function parse_dsn(dsn::String) end function parse_uri(uri::String) + values = try + parse_uri_values(uri) + catch err + (err isa URIs.ParseError || err isa ArgumentError || err isa EOFError) || rethrow() + nothing + end + # Throw outside the catch so the exception chain cannot expose credentials. + values === nothing && throw(ArgumentError("invalid PostgreSQL URI; check syntax and percent-encoding")) + return params_from_values(values) +end + +function parse_uri_values(uri::String) parsed = URIs.URI(uri) scheme = lowercase(String(parsed.scheme)) (scheme == "postgres" || scheme == "postgresql") || throw(ArgumentError("invalid PostgreSQL URI scheme: $scheme")) @@ -333,14 +355,10 @@ function parse_uri(uri::String) if !isempty(query) params = URIs.queryparams(query) for (key, value) in params - (key in KNOWN_PARAMS || key in IGNORED_PARAMS) || - throw(ArgumentError("unrecognized connection parameter \"$key\" in URI; recognized parameters are $(join(sort!(collect(KNOWN_PARAMS)), ", "))")) - key in IGNORED_PARAMS && check_ignored_param(key, value) - # keys we accept but don't implement must not reach params_from_values - key in KNOWN_PARAMS && (values[key] = value) + values[key] = value end end - return params_from_values(values) + return values end function parse_dsn(dsn::Nothing) diff --git a/test/connection_string_errors.jl b/test/connection_string_errors.jl new file mode 100644 index 0000000..b20786b --- /dev/null +++ b/test/connection_string_errors.jl @@ -0,0 +1,52 @@ +@testset "Credential-safe connection string errors" begin + malformed = ( + ("postgresq://u:s3cr3t@h/db", "s3cr3t"), + ("postgresq://u:s3cr3t@h/db?sslmode=disable", "s3cr3t"), + (" postgresq://u:s3cr3t@h/db", "s3cr3t"), + ("postgresql://u:s3cr%zzt@h/db", "s3cr"), + ("postgresql://u:s3cr/t@h/db", "s3cr"), + ("postgresql://u:s3cr?t@h/db", "s3cr"), + ("postgresql://u@h/db?password=bad%qz", "qz"), + ("postgresql://u@h/db?password=bad%", "bad"), + ("postgresql://u@h/db?s3cr3t=value", "s3cr3t"), + ("host=h password=first s3cr3t", "s3cr3t"), + ("host=h password=first s3cr3t='unfinished", "s3cr3t"), + ) + for (dsn, secret) in malformed + for parse in (Postgres.parse_dsn, + s -> DBInterface.connect(Postgres.Connection, s), + Postgres.ConnectionPool) + err, chain = try + parse(dsn) + (nothing, "") + catch e + (e, sprint(showerror, Base.current_exceptions())) + end + @test err isa ArgumentError + if err !== nothing + @test !occursin(secret, sprint(showerror, err)) + @test !occursin(secret, chain) + end + end + end + + # Known option errors still identify the option without exposing a password. + for (dsn, key) in (("host=h password=s3cr3t port=abc", "port"), + ("postgresql://u:s3cr3t@h/db?reconnect=ture", "reconnect"), + ("postgresql://u@h/db?sslpassword=s3cr3t", "sslpassword")) + err = try + Postgres.parse_dsn(dsn) + catch e + e + end + @test err isa ArgumentError + @test occursin(key, sprint(showerror, err)) + @test !occursin("s3cr3t", sprint(showerror, err)) + end + + @test Postgres.parse_dsn("host=h password='x https://example.com'").password == "x https://example.com" + @test Postgres.parse_dsn("host=h password='s3cr/t'").password == "s3cr/t" + for scheme in ("postgres", "postgresql") + @test Postgres.parse_dsn("$scheme://u:s3cr%2Ft@h/db").password == "s3cr/t" + end +end diff --git a/test/runtests.jl b/test/runtests.jl index 1ba3543..71a4f5a 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -718,6 +718,8 @@ include("notification_deadlines.jl") end end + include("connection_string_errors.jl") + @testset "API Type Parsers" begin registry = Dict(Postgres.API.DEFAULT_TYPE_REGISTRY)