mirror of
https://github.com/element-hq/synapse.git
synced 2026-11-12 20:48:30 +00:00
Resolve libpq environment defaults in one place
The `PG*` fallbacks for the DSN lived in `mod.rs` and the `PGSSL*` ones in `tls.rs`, each with its own env helper. Put both behind one `resolve_connection_defaults`, which the pool calls once, so the two cannot drift apart and `tls::build` stays a pure function of its params. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULdieJyQoHNzeHq42H2M1p
This commit is contained in:
1 parent
63558ec96e
commit
5c17f8fcd2
3 files changed
+54
-48
No files matched your search
@@ -11,6 +11,8 @@ use anyhow::{Context, Error};
|
||||
use pyo3::prelude::*;
|
||||
use pyo3::types::PyModule;
|
||||
|
||||
use crate::database::postgres::tls::TlsParams;
|
||||
|
||||
mod connection;
|
||||
mod cursor_state;
|
||||
mod errors;
|
||||
@@ -105,6 +107,55 @@ pub(crate) fn fixup_config_defaults(dsn: &str) -> Result<tokio_postgres::Config,
|
||||
/// Read an environment variable, treating a set-but-empty value as unset —
|
||||
/// libpq's behaviour, and what a blanked-out `PGPORT=` line in a compose file
|
||||
/// or systemd unit means.
|
||||
/// Resolve everything libpq would take from the environment: the DSN's
|
||||
/// missing fields (see [`fixup_config_defaults`]) and the `PGSSL*` TLS
|
||||
/// fallbacks. The one place connection defaults come from, so the TLS and
|
||||
/// DSN handling cannot drift apart.
|
||||
pub(crate) fn resolve_connection_defaults(
|
||||
dsn: &str,
|
||||
tls_params: &TlsParams,
|
||||
) -> Result<(tokio_postgres::Config, TlsParams), Error> {
|
||||
Ok((fixup_config_defaults(dsn)?, tls_env_defaults(tls_params)))
|
||||
}
|
||||
|
||||
/// Fill unset [`TlsParams`] fields from libpq's `PGSSL*` environment
|
||||
/// variables, as libpq does for keywords the connection string omits.
|
||||
///
|
||||
/// Called at pool/connection construction (see `ConnectionManager::from_dsn`,
|
||||
/// next to the other `PG*` env handling in `fixup_config_defaults`) rather
|
||||
/// than inside [`build`], which stays a pure function of its params.
|
||||
///
|
||||
/// Deliberately more conservative than libpq where a stray variable could
|
||||
/// break a working setup:
|
||||
/// - a set-but-empty variable is treated as unset, as libpq does;
|
||||
/// - `PGSSLCERT`/`PGSSLKEY` are imported only as a complete pair, and only
|
||||
/// when neither is configured explicitly. libpq resolves a lone sslcert's
|
||||
/// key lazily (`~/.postgresql/postgresql.key`) and only fails if the
|
||||
/// server actually requests a client certificate; we build the TLS config
|
||||
/// eagerly, so a leftover `PGSSLCERT` from some other tool must not abort
|
||||
/// every connection.
|
||||
fn tls_env_defaults(params: &TlsParams) -> TlsParams {
|
||||
fn fallback(value: &Option<String>, var: &str) -> Option<String> {
|
||||
value.clone().or_else(|| env_nonempty(var))
|
||||
}
|
||||
|
||||
let (sslcert, sslkey) = match (¶ms.sslcert, ¶ms.sslkey) {
|
||||
(None, None) => match (env_nonempty("PGSSLCERT"), env_nonempty("PGSSLKEY")) {
|
||||
(Some(cert), Some(key)) => (Some(cert), Some(key)),
|
||||
_ => (None, None),
|
||||
},
|
||||
(cert, key) => (cert.clone(), key.clone()),
|
||||
};
|
||||
|
||||
TlsParams {
|
||||
sslmode: fallback(¶ms.sslmode, "PGSSLMODE"),
|
||||
sslrootcert: fallback(¶ms.sslrootcert, "PGSSLROOTCERT"),
|
||||
sslcert,
|
||||
sslkey,
|
||||
sslpassword: fallback(¶ms.sslpassword, "PGSSLPASSWORD"),
|
||||
}
|
||||
}
|
||||
|
||||
fn env_nonempty(var: &str) -> Option<String> {
|
||||
std::env::var(var).ok().filter(|value| !value.is_empty())
|
||||
}
|
||||
@@ -32,8 +32,8 @@ use tokio_postgres::{Client, Config, Connection as PgConnection, NoTls, Socket,
|
||||
|
||||
use crate::database::postgres::connection::Connection;
|
||||
use crate::database::postgres::errors::{pg_err_to_py, OperationalError};
|
||||
use crate::database::postgres::fixup_config_defaults;
|
||||
use crate::database::postgres::helpers::BlockingPostgres;
|
||||
use crate::database::postgres::resolve_connection_defaults;
|
||||
use crate::database::postgres::tls::{self, TlsConnector, TlsParams};
|
||||
use crate::runtime::RustRuntime;
|
||||
|
||||
@@ -123,11 +123,7 @@ impl ConnectionManager {
|
||||
session: SessionConfig,
|
||||
tls_params: &TlsParams,
|
||||
) -> Result<Self, anyhow::Error> {
|
||||
let mut config = fixup_config_defaults(dsn)?;
|
||||
// Resolve the `PGSSL*` env fallbacks here, next to the other `PG*`
|
||||
// env handling in `fixup_config_defaults`, keeping `tls::build` (and
|
||||
// its unit tests) a pure function of its params.
|
||||
let tls_params = tls::with_env_defaults(tls_params);
|
||||
let (mut config, tls_params) = resolve_connection_defaults(dsn, tls_params)?;
|
||||
let (ssl_mode, connector) = tls::build(&tls_params)?;
|
||||
config.ssl_mode(ssl_mode);
|
||||
Ok(Self {
|
||||
|
||||
@@ -83,51 +83,10 @@ enum Verification {
|
||||
Full,
|
||||
}
|
||||
|
||||
/// Fill unset [`TlsParams`] fields from libpq's `PGSSL*` environment
|
||||
/// variables, as libpq does for keywords the connection string omits.
|
||||
///
|
||||
/// Called at pool/connection construction (see `ConnectionManager::from_dsn`,
|
||||
/// next to the other `PG*` env handling in `fixup_config_defaults`) rather
|
||||
/// than inside [`build`], which stays a pure function of its params.
|
||||
///
|
||||
/// Deliberately more conservative than libpq where a stray variable could
|
||||
/// break a working setup:
|
||||
/// - a set-but-empty variable is treated as unset, as libpq does;
|
||||
/// - `PGSSLCERT`/`PGSSLKEY` are imported only as a complete pair, and only
|
||||
/// when neither is configured explicitly. libpq resolves a lone sslcert's
|
||||
/// key lazily (`~/.postgresql/postgresql.key`) and only fails if the
|
||||
/// server actually requests a client certificate; we build the TLS config
|
||||
/// eagerly, so a leftover `PGSSLCERT` from some other tool must not abort
|
||||
/// every connection.
|
||||
pub(crate) fn with_env_defaults(params: &TlsParams) -> TlsParams {
|
||||
fn env_nonempty(var: &str) -> Option<String> {
|
||||
std::env::var(var).ok().filter(|value| !value.is_empty())
|
||||
}
|
||||
fn fallback(value: &Option<String>, var: &str) -> Option<String> {
|
||||
value.clone().or_else(|| env_nonempty(var))
|
||||
}
|
||||
|
||||
let (sslcert, sslkey) = match (¶ms.sslcert, ¶ms.sslkey) {
|
||||
(None, None) => match (env_nonempty("PGSSLCERT"), env_nonempty("PGSSLKEY")) {
|
||||
(Some(cert), Some(key)) => (Some(cert), Some(key)),
|
||||
_ => (None, None),
|
||||
},
|
||||
(cert, key) => (cert.clone(), key.clone()),
|
||||
};
|
||||
|
||||
TlsParams {
|
||||
sslmode: fallback(¶ms.sslmode, "PGSSLMODE"),
|
||||
sslrootcert: fallback(¶ms.sslrootcert, "PGSSLROOTCERT"),
|
||||
sslcert,
|
||||
sslkey,
|
||||
sslpassword: fallback(¶ms.sslpassword, "PGSSLPASSWORD"),
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve [`TlsParams`] into the `tokio-postgres` [`SslMode`] to set on the
|
||||
/// connection config and the [`TlsConnector`] to connect with. A pure
|
||||
/// function of its params: the `PGSSL*` env fallbacks are applied by the
|
||||
/// caller (see [`with_env_defaults`]).
|
||||
/// caller (see [`super::resolve_connection_defaults`]).
|
||||
pub fn build(params: &TlsParams) -> anyhow::Result<(SslMode, TlsConnector)> {
|
||||
let mode = params.sslmode.as_deref().unwrap_or("prefer");
|
||||
let (ssl_mode, verification) = match mode {
|
||||
|
||||
Reference in new issue
Block a user