Keep credentials out of malformed connection-string errors

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)

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

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)