mirror of
https://github.com/element-hq/synapse.git
synced 2026-08-14 20:10:26 +00:00
Use libpq's default host when the DSN omits one
`tokio_postgres` and libpq disagree on the default host when a DSN gives no `host=`: tokio-postgres falls back to localhost, whereas libpq (which `psql` and the rest of Synapse use) applies its configurable compiled-in default — typically the Unix socket directory — and honours `PGHOST`. To keep Synapse's existing connection behaviour, `connect()` now runs the DSN through `fixup_default_host`: if the parsed config has neither a host nor a hostaddr, it asks libpq what its default would be (via `PQconnectStart` on an empty conninfo, which applies libpq's defaults/env without opening a socket) and sets that on the tokio-postgres config. The libpq call lives in a small hand-written safe wrapper (`database::postgres::libpq`) over the `pq-sys` crate. pq-sys ships pre-generated bindings and links the system libpq itself, so — unlike a bindgen-based binding crate — this needs no libclang to build. The behaviour is exercised by the Python integration tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
5b734670a3
commit
d7ae4dc645
Generated
+24
@@ -958,6 +958,12 @@ version = "0.2.16"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b"
|
||||
|
||||
[[package]]
|
||||
name = "pkg-config"
|
||||
version = "0.3.33"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "19f132c84eca552bf34cab8ec81f1c1dcc229b811638f9d283dceabe58c5569e"
|
||||
|
||||
[[package]]
|
||||
name = "portable-atomic"
|
||||
version = "1.11.1"
|
||||
@@ -1012,6 +1018,17 @@ dependencies = [
|
||||
"zerocopy",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "pq-sys"
|
||||
version = "0.7.5"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "574ddd6a267294433f140b02a726b0640c43cf7c6f717084684aaa3b285aba61"
|
||||
dependencies = [
|
||||
"libc",
|
||||
"pkg-config",
|
||||
"vcpkg",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
name = "proc-macro2"
|
||||
version = "1.0.95"
|
||||
@@ -1624,6 +1641,7 @@ dependencies = [
|
||||
"mime",
|
||||
"once_cell",
|
||||
"postgres-protocol",
|
||||
"pq-sys",
|
||||
"pyo3",
|
||||
"pyo3-log",
|
||||
"pythonize",
|
||||
@@ -1908,6 +1926,12 @@ version = "1.0.4"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "b6c140620e7ffbb22c2dee59cafe6084a59b5ffc27a8859a5f0d494b5d52b6be"
|
||||
|
||||
[[package]]
|
||||
name = "vcpkg"
|
||||
version = "0.2.15"
|
||||
source = "registry+https://github.com/rust-lang/crates.io-index"
|
||||
checksum = "accd4ea62f7bb7a82fe23066fb0957d48ef677f6eeb8215f372f52e48bb32426"
|
||||
|
||||
[[package]]
|
||||
name = "version_check"
|
||||
version = "0.9.5"
|
||||
|
||||
@@ -66,6 +66,10 @@ tokio-postgres = "0.7"
|
||||
once_cell = "1.18.0"
|
||||
itertools = "0.14.0"
|
||||
postgres-protocol = "0.6.10"
|
||||
# Raw libpq bindings, used by `database::postgres::libpq` to resolve libpq's
|
||||
# default host. Ships pre-generated bindings and links the system libpq itself,
|
||||
# so — unlike a bindgen-based binding — the build needs no libclang.
|
||||
pq-sys = "0.7.5"
|
||||
|
||||
[build-dependencies]
|
||||
blake2 = "0.10.4"
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
//! Resolve libpq's default host for a DSN that omits `host=`.
|
||||
//!
|
||||
//! [`tokio_postgres`] applies its *own* default host (`localhost`) when a DSN
|
||||
//! omits `host=`, whereas Synapse has always used libpq's default — the
|
||||
//! compiled-in default socket directory (which distributions such as Debian
|
||||
//! patch) and environment variables like `PGHOST`. So when a DSN has no host we
|
||||
//! ask the real libpq what it would do and copy that across.
|
||||
//!
|
||||
//! We call libpq through [`pq_sys`], which ships pre-generated bindings and
|
||||
//! links the system libpq itself. Unlike a bindgen-based binding crate that
|
||||
//! generates its FFI at build time, this needs no libclang to build — only a C
|
||||
//! linker and a discoverable libpq (`pg_config` / `pkg-config`).
|
||||
|
||||
use std::ffi::{c_char, CStr, CString};
|
||||
|
||||
use anyhow::{bail, Error};
|
||||
use pq_sys::{ConnStatusType, PGconn, PQconnectStart, PQerrorMessage, PQfinish, PQhost, PQstatus};
|
||||
|
||||
/// Owns a `PGconn` and calls `PQfinish` on drop.
|
||||
struct Connection(*mut PGconn);
|
||||
|
||||
impl Drop for Connection {
|
||||
fn drop(&mut self) {
|
||||
// SAFETY: `self.0` was returned by `PQconnectStart` and is finished
|
||||
// exactly once, here. `PQfinish` accepts a connection that never
|
||||
// completed its (non-blocking) connect.
|
||||
unsafe { PQfinish(self.0) }
|
||||
}
|
||||
}
|
||||
|
||||
/// Ask libpq which host an otherwise-empty connection string would connect to.
|
||||
///
|
||||
/// Resolves libpq's defaults *without opening a socket*: `PQconnectStart`
|
||||
/// begins a non-blocking connection but we never poll it (no `PQconnectPoll`),
|
||||
/// then drop it immediately — so this only parses options and applies defaults.
|
||||
pub fn default_host() -> Result<String, Error> {
|
||||
// An empty conninfo means "use every default".
|
||||
let conninfo = CString::new("")?;
|
||||
|
||||
// SAFETY: `conninfo` is a valid NUL-terminated string that outlives the
|
||||
// call; `PQconnectStart` copies what it needs.
|
||||
let conn = unsafe { PQconnectStart(conninfo.as_ptr()) };
|
||||
if conn.is_null() {
|
||||
bail!("libpq PQconnectStart returned null (out of memory)");
|
||||
}
|
||||
let conn = Connection(conn);
|
||||
|
||||
// SAFETY: `conn.0` is non-null and valid for the rest of this scope.
|
||||
if unsafe { PQstatus(conn.0) } == ConnStatusType::CONNECTION_BAD {
|
||||
// SAFETY: `PQerrorMessage` returns a valid (possibly empty) C string
|
||||
// owned by the connection.
|
||||
let msg = unsafe { cstr_lossy(PQerrorMessage(conn.0)) };
|
||||
bail!("libpq could not parse default connection options: {msg}");
|
||||
}
|
||||
|
||||
// SAFETY: `conn.0` is valid; `PQhost` returns a pointer owned by the
|
||||
// connection (valid until `PQfinish`), or null.
|
||||
let host_ptr = unsafe { PQhost(conn.0) };
|
||||
if host_ptr.is_null() {
|
||||
bail!("libpq PQhost returned null");
|
||||
}
|
||||
|
||||
// SAFETY: non-null, checked just above.
|
||||
let host = unsafe { cstr_lossy(host_ptr) };
|
||||
if host.is_empty() {
|
||||
bail!("libpq reported an empty default host");
|
||||
}
|
||||
|
||||
Ok(host)
|
||||
}
|
||||
|
||||
/// Copy a libpq-owned C string into an owned `String` (lossy on non-UTF-8).
|
||||
///
|
||||
/// # Safety
|
||||
/// `ptr` must be non-null and point to a valid NUL-terminated string.
|
||||
unsafe fn cstr_lossy(ptr: *const c_char) -> String {
|
||||
CStr::from_ptr(ptr).to_string_lossy().into_owned()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn resolves_a_nonempty_default_host() {
|
||||
// We can't assert the exact value (it's host/distro/env dependent),
|
||||
// but libpq must always resolve *some* non-empty default host — the
|
||||
// whole reason we defer to it rather than hard-coding one.
|
||||
let host = default_host().expect("libpq should resolve a default host");
|
||||
assert!(!host.is_empty());
|
||||
}
|
||||
}
|
||||
@@ -1,8 +1,11 @@
|
||||
//! [`tokio_postgres`]-backed `Connection` / `Cursor` types exposed to Python.
|
||||
//!
|
||||
//! The driver itself is async; we drive it from sync Python methods via a
|
||||
//! shared multi-thread tokio runtime (see `super::runtime`).
|
||||
//! The driver itself is async; we drive it from sync Python methods via the
|
||||
//! extension's shared multi-thread tokio runtime (see [`crate::tokio_runtime`]).
|
||||
//! [`connect`] takes the runtime's handle from the reactor once and hands it to
|
||||
//! the [`Connection`], which carries it for the life of the connection.
|
||||
|
||||
use anyhow::Error;
|
||||
use log::warn;
|
||||
use pyo3::exceptions::PyRuntimeError;
|
||||
use pyo3::prelude::*;
|
||||
@@ -14,6 +17,7 @@ use crate::tokio_runtime::runtime_handle;
|
||||
mod connection;
|
||||
mod cursor_state;
|
||||
mod helpers;
|
||||
mod libpq;
|
||||
mod value;
|
||||
|
||||
/// Register the `postgres` submodule (the `Connection` / `Cursor` classes and
|
||||
@@ -59,9 +63,8 @@ fn connect<'py>(
|
||||
) -> PyResult<Bound<'py, connection::Connection>> {
|
||||
let handle = runtime_handle(reactor)?;
|
||||
|
||||
let config = dsn
|
||||
.parse::<tokio_postgres::Config>()
|
||||
.map_err(|e| PyRuntimeError::new_err(format!("Failed to parse DSN: {e}")))?;
|
||||
let config = fixup_default_host(dsn)
|
||||
.map_err(|e| PyRuntimeError::new_err(format!("Failed to prepare DSN: {e}")))?;
|
||||
|
||||
// TLS is not yet supported: unlike libpq (whose default is
|
||||
// `sslmode=prefer`), we never negotiate TLS regardless of the DSN's
|
||||
@@ -81,3 +84,29 @@ fn connect<'py>(
|
||||
|
||||
Bound::new(py, conn)
|
||||
}
|
||||
|
||||
/// Fix up a DSN to ensure it has a host, using libpq's default host if
|
||||
/// necessary.
|
||||
///
|
||||
/// [`tokio_postgres`] has a different default host than libpq, which is what
|
||||
/// Synapse previously used (and is what e.g. `psql` uses). libpq's default host
|
||||
/// is configurable, so when the DSN omits a host we ask libpq what its default
|
||||
/// would be and use that instead (see [`libpq::default_host`]).
|
||||
fn fixup_default_host(dsn: &str) -> Result<tokio_postgres::Config, Error> {
|
||||
let mut config = dsn.parse::<tokio_postgres::Config>()?;
|
||||
|
||||
// `tokio_postgres` parses only the DSN string (it does not consult `PGHOST`
|
||||
// or the compiled-in default), so an empty host list means the DSN really
|
||||
// omitted the host. A DSN that gives a `hostaddr` instead of a `host` is
|
||||
// still connectable as-is, so leave it alone too — injecting a default host
|
||||
// there would just confuse TLS/SNI.
|
||||
if !config.get_hosts().is_empty() || !config.get_hostaddrs().is_empty() {
|
||||
return Ok(config);
|
||||
}
|
||||
|
||||
// Resolve libpq's default host without connecting (see `libpq::default_host`).
|
||||
let host = libpq::default_host()?;
|
||||
config.host(&host);
|
||||
|
||||
Ok(config)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user