node-postgres
node-postgres copied to clipboard
Fix parsing of ssl=false connection string parameter
Cherry-picks https://github.com/iceddev/pg-connection-string/pull/39 to this monorepo.
ssl=false
isn’t supported by libpq, only ssl=true
. Incompatibilities already exist between pg-connection-string and libpq, but I’m against adding new ones.
(sslmode=disable
is the correct version.)
ssl=false
is documented in pg-connection-string’s readme, though. Would you rather like to remove it there?
Alternatively, we could remove support for all ssl
values except true
to align with libpq. After all, it might be a bit surprising to support ssl=1
, ssl=true
, ssl=0
but not ssl=false
.
I agree that since the other non-libpq modes are supported, ssl=false should be supported.
When ssl=false
is set, it actually ends up enabling ssl because it gets set to the string "false", which is truthy.
This is very surprising behavior, especially because the docs actually explicitly state this is allowed.
If feature parity with libpq is more important, it could at least log a warning or throw an exception if the user uses this ssl flag in an unsupported way.
It could possibly throw an uncaught exception without this patch.
Reproduce Steps
- Postgres with SSL
- Connect with
ssl=false
const client = new Client({
connectionString: "postgres://postgres:postgres@localhost:5432/postgres?ssl=false",
});
client.connect();
// Throw uncaught exception
TypeError: Cannot use 'in' operator to search for 'key' in false
https://github.com/brianc/node-postgres/blob/b1a8947738ce0af004cb926f79829bb2abc64aa6/packages/pg/lib/connection.js#L90C7-L90C7
Since my application allows people to input any connection strings freely, throw an uncaught exception is not nice.
Possible solutions
Method 1: Merge this pull request.
Method 2: In case you prefer sslmode=disable
, "ssl=false" should completely ignored and muted, rather than parse it as a string "false".