diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ed209b..d1929e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,17 @@ via the same trait. Both TLS branches now present the client cert via `.with_client_auth_cert(...)` when `ssl_cert`/`ssl_key` are set, and `build_tls_connector` errors clearly if only one of the pair is provided. +- Pool cache key ignored every TLS param (`ssl_mode`/`ssl_ca`/`ssl_cert`/ + `ssl_key`) — `connection_key` matched only on + `host:port:database:user:startup_script`, so two connections to the same + target differing only in TLS configuration (e.g. `require` vs. + `verify-full`, or different client certs) could incorrectly share a + cached pool and its already-negotiated TLS setup. This wasn't inherited + from the builtin driver: the builtin's `build_connection_key` already + keyed on `ssl_mode`/`ssl_ca` before this plugin's `client.rs` was even + staged, so this was a parity miss during extraction, not a later + upstream change. Folded all four TLS params into `connection_key`, + matching the builtin's TLS-param keying. ## [1.0.0-beta.7] - 2026-08-17 diff --git a/CLAUDE.md b/CLAUDE.md index ba15986..25a822e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -53,7 +53,8 @@ migration, now applied across the repo boundary. - Handlers live in `src/handlers/`, organized by domain (connection, metadata, crud, ddl, blob, query). - PostgreSQL access goes through `src/client.rs` (deadpool-postgres pool, - cached by `host:port:database:user:startup_script`). + cached by `host:port:database:user:startup_script` plus every TLS param — + `ssl_mode`/`ssl_ca`/`ssl_cert`/`ssl_key`). - Value binding for INSERT/UPDATE lives in `src/binding.rs` — a strict ordered cascade (DEFAULT sentinel, BLOB wire format, enum CAST, boolean, numeric, temporal, UUID shape, PG array literal, TEXT fallback). Order diff --git a/src/client.rs b/src/client.rs index 39758ee..c6f73f7 100644 --- a/src/client.rs +++ b/src/client.rs @@ -13,10 +13,10 @@ //! transient connection hiccup on one call (e.g. a setup step in a test) is //! silently swallowed by the caller and never retried, unlike a persistent //! pool where a single connection failure doesn't affect already-established -//! connections. Caching by `host:port:database:user` (matches the builtin's -//! `build_connection_key` pattern in `src-tauri/src/pool_manager.rs`, minus -//! the TLS/connection_id refinements that plugin doesn't need yet) closes -//! that gap. +//! connections. Caching by `host:port:database:user:startup_script` plus +//! every TLS param (matches the builtin's `build_connection_key` pattern in +//! `src-tauri/src/pool_manager.rs`, minus the per-connection_id refinement +//! that plugin doesn't need yet) closes that gap. use std::collections::HashMap; use std::str::FromStr; @@ -214,17 +214,24 @@ pub async fn build_pool_pub(params: &ConnectionParams) -> Result { } /// Identifies a connection target for pool-cache purposes. -/// Matches on host:port:database:user (plus the startup script, so editing -/// it forces a fresh pool) — sufficient for this plugin's scope (no -/// per-connection TLS-mode/connection_id refinement, unlike the builtin). +/// Matches on host:port:database:user:startup_script, plus every TLS param +/// (ssl_mode/ssl_ca/ssl_cert/ssl_key) — otherwise two connections differing +/// only in TLS configuration would incorrectly share a pool and its +/// already-negotiated TLS setup. Matches the builtin's `build_connection_key` +/// TLS-param keying in `src-tauri/src/pool_manager.rs`, minus the +/// per-connection_id refinement that plugin doesn't need yet. fn connection_key(params: &ConnectionParams) -> String { format!( - "{}:{}:{}:{}:{}", + "{}:{}:{}:{}:{}:{}:{}:{}:{}", params.host.as_deref().unwrap_or(""), params.port.unwrap_or(5432), params.database.as_deref().unwrap_or(""), params.username.as_deref().unwrap_or(""), params.startup_script.as_deref().unwrap_or(""), + params.ssl_mode.as_deref().unwrap_or(""), + params.ssl_ca.as_deref().unwrap_or(""), + params.ssl_cert.as_deref().unwrap_or(""), + params.ssl_key.as_deref().unwrap_or(""), ) } diff --git a/src/client_tests.rs b/src/client_tests.rs index 940039c..198fd7f 100644 --- a/src/client_tests.rs +++ b/src/client_tests.rs @@ -70,6 +70,65 @@ fn connection_key_is_stable_for_identical_params() { assert_eq!(a, b); } +#[test] +fn connection_key_differs_by_ssl_mode() { + let mut a = params("localhost", 5432, "db", "postgres"); + a.ssl_mode = Some("require".to_string()); + let mut b = params("localhost", 5432, "db", "postgres"); + b.ssl_mode = Some("verify-full".to_string()); + assert_ne!( + connection_key(&a), + connection_key(&b), + "different ssl_mode values must not share a cache key" + ); +} + +#[test] +fn connection_key_differs_by_ssl_ca() { + let mut a = params("localhost", 5432, "db", "postgres"); + a.ssl_mode = Some("verify-ca".to_string()); + let mut b = params("localhost", 5432, "db", "postgres"); + b.ssl_mode = Some("verify-ca".to_string()); + b.ssl_ca = Some("/tmp/ca.pem".to_string()); + assert_ne!( + connection_key(&a), + connection_key(&b), + "different ssl_ca values must not share a cache key" + ); +} + +#[test] +fn connection_key_differs_by_ssl_cert() { + let mut a = params("localhost", 5432, "db", "postgres"); + a.ssl_mode = Some("require".to_string()); + let mut b = params("localhost", 5432, "db", "postgres"); + b.ssl_mode = Some("require".to_string()); + b.ssl_cert = Some("/tmp/client-cert.pem".to_string()); + b.ssl_key = Some("/tmp/client-key.pem".to_string()); + assert_ne!( + connection_key(&a), + connection_key(&b), + "different ssl_cert/ssl_key values must not share a cache key" + ); +} + +#[test] +fn connection_key_differs_by_ssl_key_alone() { + let mut a = params("localhost", 5432, "db", "postgres"); + a.ssl_mode = Some("require".to_string()); + a.ssl_cert = Some("/tmp/client-cert.pem".to_string()); + a.ssl_key = Some("/tmp/client-key-a.pem".to_string()); + let mut b = params("localhost", 5432, "db", "postgres"); + b.ssl_mode = Some("require".to_string()); + b.ssl_cert = Some("/tmp/client-cert.pem".to_string()); + b.ssl_key = Some("/tmp/client-key-b.pem".to_string()); + assert_ne!( + connection_key(&a), + connection_key(&b), + "different ssl_key values must not share a cache key even with an identical ssl_cert" + ); +} + #[tokio::test] async fn get_or_create_pool_reuses_cached_entry_for_identical_params() { // deadpool's Pool::new is lazy (no connection attempt at creation