Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .github/workflows/ci-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,9 @@ jobs:
# Server-version preflight: end-to-end core CDC path (create slot -> stream -> decode INSERT) plus PG17+ gated slot ops (FAILOVER create + ALTER) that must pass the client-side version gate and execute; the gated portion skips gracefully on PG<17.
cargo test --test version_preflight -- --ignored --nocapture --test-threads=1

# quote_literal escaping: proves quote_literal stays injection-safe under standard_conforming_strings off and on (a backslash+quote payload roundtrips byte-for-byte); uses the regular connection and creates no replication slot.
cargo test --test quote_literal_scs_off -- --ignored --nocapture --test-threads=1

integration-ssl:
name: SSL Integration Tests (PG ${{ matrix.pg_version }})
runs-on: ubuntu-latest
Expand Down
4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,10 @@ path = "integration-tests/base_backup_options.rs"
name = "version_preflight"
path = "integration-tests/version_preflight.rs"

[[test]]
name = "quote_literal_scs_off"
path = "integration-tests/quote_literal_scs_off.rs"

[[bench]]
name = "wal_pipeline"
harness = false
Expand Down
87 changes: 87 additions & 0 deletions integration-tests/quote_literal_scs_off.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
#![cfg(any(feature = "libpq", feature = "rustls-tls"))]

//! Live proof that `quote_literal` is injection-safe under either value of the
//! server's `standard_conforming_strings` setting.
//!
//! With `standard_conforming_strings = off` a backslash is an escape character
//! inside a plain `'…'` string, so quote-doubling alone would let a crafted
//! value terminate the literal early. `quote_literal` defends against this by
//! switching to the escape-string form ` E'…'` (backslashes doubled) whenever
//! the input contains a backslash. This test feeds a payload combining a
//! backslash, a single quote, and trailing SQL through `quote_literal` into a
//! `SELECT`, then asserts the value roundtrips byte-for-byte — proving the
//! string never terminated early and the trailing SQL never executed.
//!
//! ## Prerequisites
//!
//! - PostgreSQL (any supported version)
//! - `DATABASE_URL_REGULAR` — a regular (non-replication) connection, e.g.
//! `postgresql://postgres:postgres@localhost:5432/test_walstream`
//! (falls back to `DATABASE_URL` with `replication=database` stripped)
//!
//! ## Running Locally
//!
//! ```bash
//! export DATABASE_URL_REGULAR="postgresql://postgres:postgres@localhost:5432/test_walstream"
//! cargo test --test quote_literal_scs_off -- --ignored --nocapture
//! ```

use pg_walstream::{quote_literal, PgReplicationConnection};

fn regular_conn_string() -> String {
std::env::var("DATABASE_URL_REGULAR").unwrap_or_else(|_| {
std::env::var("DATABASE_URL")
.unwrap_or_else(|_| {
"postgresql://postgres:postgres@localhost:5432/test_walstream".to_string()
})
// Drop the replication flag: this test needs a *regular* (non-replication) connection so `SET` and a plain `SELECT` work.
// Handle it as the first, a later, or the sole query parameter without leaving a dangling `?`/`&`.
.replace("?replication=database&", "?")
.replace("&replication=database", "")
.replace("?replication=database", "")
})
}

fn init_tracing() {
let _ = tracing_subscriber::fmt()
.with_max_level(tracing::Level::INFO)
.try_init();
}

/// `SELECT <quote_literal(payload)>` and return the single scalar the server
/// echoes back.
fn roundtrip(conn: &mut PgReplicationConnection, payload: &str) -> String {
let sql = format!(
"SELECT {} AS v",
quote_literal(payload).expect("no null bytes")
);
conn.exec(&sql)
.expect("SELECT of quoted literal must succeed")
.get_value(0, 0)
.expect("one row, one column")
}

#[test]
#[ignore = "requires live PostgreSQL"]
fn quote_literal_survives_standard_conforming_strings_off() {
init_tracing();

let mut conn =
PgReplicationConnection::connect(&regular_conn_string()).expect("regular connection");

// Combines both literal-breakout vectors: a backslash (the scs=off escape
// character) and a single quote, followed by SQL that must NOT run.
let payload = r"weird \ value ' with; DROP TABLE x; --";

for scs in ["off", "on"] {
conn.exec(&format!("SET standard_conforming_strings = {scs}"))
.unwrap_or_else(|e| panic!("SET standard_conforming_strings = {scs}: {e}"));

let got = roundtrip(&mut conn, payload);
assert_eq!(
got, payload,
"payload must roundtrip intact under standard_conforming_strings = {scs}; \
early string termination would change, truncate, or drop it"
);
}
}
29 changes: 24 additions & 5 deletions src/connection/libpq.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,22 @@ impl PgReplicationConnection {
}
}

// Force UTF-8 client_encoding so quote_literal/quote_ident's ASCII-only escaping is multibyte-safe regardless of the server/locale default , parity with the native backend, which sends client_encoding=UTF8 in its startup packet. Under an ASCII-unsafe encoding (SJIS/GBK/BIG5) a raw lead byte could otherwise fuse with an escaping quote and reopen a literal breakout.
Comment thread
isdaniel marked this conversation as resolved.
if unsafe { PQsetClientEncoding(conn, c"UTF8".as_ptr()) } != 0 {
let error_msg = unsafe {
let error_ptr = PQerrorMessage(conn);
if error_ptr.is_null() {
"Unknown error".to_string()
} else {
CStr::from_ptr(error_ptr).to_string_lossy().into_owned()
}
};
unsafe { PQfinish(conn) };
return Err(ReplicationError::permanent_connection(format!(
"Failed to set client_encoding=UTF8: {error_msg}"
)));
}

// Check server version - logical replication requires PostgreSQL 14+
let server_version = unsafe { PQserverVersion(conn) };
if server_version < 140000 {
Expand Down Expand Up @@ -1199,11 +1215,14 @@ mod tests {
}

#[test]
fn test_sanitize_backslash_and_quote() {
// Backslashes should not be specially treated, only single quotes
let input = "test\\'value";
let sanitized = sanitize_sql_string_value(input);
assert_eq!(sanitized, "test\\''value");
fn test_quote_backslash_and_quote() {
// A backslash switches quote_literal to the escape-string form ` E'…'`
// (safe under standard_conforming_strings off or on); both the backslash
// and the single quote are doubled.
assert_eq!(
quote_sql_string_value("test\\'value"),
r#" E'test\\''value'"#
);
}

#[test]
Expand Down
10 changes: 8 additions & 2 deletions src/connection/native/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1273,8 +1273,14 @@ mod tests {
}

#[test]
fn test_sanitize_backslash_and_quote() {
assert_eq!(sanitize_sql_string_value("test\\'value"), "test\\''value");
fn test_quote_backslash_and_quote() {
// A backslash switches quote_literal to the escape-string form ` E'…'`
// (safe under standard_conforming_strings off or on); both the backslash
// and the single quote are doubled.
assert_eq!(
quote_sql_string_value("test\\'value"),
r#" E'test\\''value'"#
);
}

#[test]
Expand Down
Loading
Loading