Fix parsing of ssl=false connection string parameter

[sonicdoe]

Cherry-picks iceddev/pg-connection-string#39 to this monorepo.

[charmander]

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.)

[sonicdoe]

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.

[snowbldr]

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.

[louislam]

It could possibly throw an uncaught exception without this patch.

Reproduce Steps

1. Postgres with SSL
2. 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".

[hjr3]

Method 2: In case you prefer sslmode=disable, "ssl=false" should completely ignored and muted, rather than parse it as a string "false".

I observe that libpq throws an error when ssl=false is passed.

$ psql 'postgres://localhost/bar?ssl=false'
psql: error: invalid URI query parameter: "ssl"

I think this is the behavior we want to match. Muting causes "spooky action at a distance" problems.

[mercmobily]

Interested as this affects knex at the moment

[charmander]

@mercmobily If this affects knex, knex should stop passing ssl=false in a connection string (which is not a real thing) and start passing sslmode=disable.

[mercmobily]

Ah, now, this is something I was missing!