Skip to content

Commit 39598b8

Browse files
committed
fix(pgwire): decode binary-format extended-query parameters
Previously the bind layer decoded every portal parameter as UTF-8 text regardless of wire format, so clients that declare parameter types via prepare_typed (which causes tokio-postgres to send binary-encoded values) had their BOOL/INT2/INT4/INT8/FLOAT4/FLOAT8 parameters rejected with a bogus "invalid UTF-8" error. Binary scalars are now decoded via their postgres_types::FromSql encodings; NUMERIC/TIMESTAMP/TIMESTAMPTZ and other unmodeled binary types are explicitly rejected rather than silently misread as text. Extracts the parameter conversion logic into its own param_bind module and adds regression coverage for binary-format decoding across the scalar types.
1 parent 0f06e64 commit 39598b8

8 files changed

Lines changed: 695 additions & 338 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ pco = "1"
242242
pgwire = { version = "0.38", default-features = false, features = [
243243
"server-api-aws-lc-rs",
244244
] }
245+
postgres-types = "0.2"
245246
tokio-postgres = "0.7"
246247
async-trait = "0.1"
247248
futures = "0.3"

nodedb/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ reqwest = { workspace = true }
125125

126126
# PostgreSQL wire protocol
127127
pgwire = { workspace = true }
128+
postgres-types = { workspace = true }
128129
futures = { workspace = true }
129130
async-stream = { workspace = true }
130131

nodedb/src/control/server/pgwire/handler/prepared/execute.rs

Lines changed: 2 additions & 325 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,18 @@
88
99
use std::fmt::Debug;
1010

11-
use bytes::Bytes;
1211
use futures::sink::Sink;
1312
use pgwire::api::portal::Portal;
1413
use pgwire::api::results::Response;
15-
use pgwire::api::{ClientInfo, ClientPortalStore, Type};
14+
use pgwire::api::{ClientInfo, ClientPortalStore};
1615
use pgwire::error::{ErrorInfo, PgWireError, PgWireResult};
1716
use pgwire::messages::PgWireBackendMessage;
1817

1918
use crate::control::server::response_shape::schema::{OutputColumn, OutputSchema};
2019

2120
use super::super::core::NodeDbPgHandler;
2221
use super::super::routing::result_shaping::ResultShaping;
22+
use super::param_bind::convert_portal_params;
2323
use super::result_format::{pg_type_to_ddl_col_type, resolve_result_formats};
2424
use super::statement::ParsedStatement;
2525

@@ -149,326 +149,3 @@ impl NodeDbPgHandler {
149149
Ok(results.pop().unwrap_or(Response::EmptyQuery))
150150
}
151151
}
152-
153-
/// Convert pgwire portal parameters to typed `ParamValue` for AST-level binding.
154-
///
155-
/// Uses per-parameter format codes from the pgwire 0.38 `Format` API to determine
156-
/// whether each parameter was sent in text or binary format.
157-
///
158-
/// Binary-format NUMERIC, TIMESTAMP, and TIMESTAMPTZ parameters are explicitly
159-
/// rejected with SQLSTATE 0A000 — their binary encodings are client-library-specific
160-
/// structs that would produce corrupt values if decoded naively. Clients must use
161-
/// text format for these types.
162-
fn convert_portal_params(
163-
params: &[Option<Bytes>],
164-
param_types: &[Option<Type>],
165-
param_format: &pgwire::api::portal::Format,
166-
) -> PgWireResult<Vec<nodedb_sql::ParamValue>> {
167-
let mut result = Vec::with_capacity(params.len());
168-
for (i, param) in params.iter().enumerate() {
169-
let pg_type = param_types
170-
.get(i)
171-
.and_then(|t| t.as_ref())
172-
.unwrap_or(&Type::UNKNOWN);
173-
174-
let pv = match param {
175-
None => nodedb_sql::ParamValue::Null,
176-
Some(bytes) => {
177-
// Reject binary format for types whose binary encoding is
178-
// client-library-specific and cannot be decoded portably.
179-
if param_format.is_binary(i) {
180-
let type_name = if *pg_type == Type::NUMERIC {
181-
Some("NUMERIC")
182-
} else if *pg_type == Type::TIMESTAMP {
183-
Some("TIMESTAMP")
184-
} else if *pg_type == Type::TIMESTAMPTZ {
185-
Some("TIMESTAMPTZ")
186-
} else {
187-
None
188-
};
189-
if let Some(name) = type_name {
190-
return Err(PgWireError::UserError(Box::new(ErrorInfo::new(
191-
"ERROR".to_owned(),
192-
"0A000".to_owned(),
193-
format!(
194-
"binary {name} parameter format is not supported for \
195-
parameter ${n}; use text format",
196-
n = i + 1
197-
),
198-
))));
199-
}
200-
}
201-
202-
let text = std::str::from_utf8(bytes).map_err(|_| {
203-
PgWireError::UserError(Box::new(ErrorInfo::new(
204-
"ERROR".to_owned(),
205-
"22021".to_owned(),
206-
format!("invalid UTF-8 in parameter ${}", i + 1),
207-
)))
208-
})?;
209-
210-
pgwire_text_to_param(text, pg_type)
211-
}
212-
};
213-
result.push(pv);
214-
}
215-
Ok(result)
216-
}
217-
218-
/// Convert a pgwire text parameter + declared type to a typed
219-
/// `ParamValue` for AST/DSL binding.
220-
///
221-
/// # Type coverage
222-
///
223-
/// Natively decoded: `BOOL`, `INT2`/`INT4`/`INT8`, `FLOAT4`/`FLOAT8`/
224-
/// `NUMERIC`, `TIMESTAMP`, `TIMESTAMPTZ`, `TEXT`/`VARCHAR` (implicit via
225-
/// fall-through), and `UNKNOWN` (the untyped-driver path).
226-
///
227-
/// # TIMESTAMP / TIMESTAMPTZ
228-
///
229-
/// Text-format TIMESTAMP and TIMESTAMPTZ parameters are parsed directly to
230-
/// `ParamValue::Timestamp` / `ParamValue::Timestamptz`. This produces the
231-
/// correct typed `SqlValue` variant (Timestamp vs Timestamptz) through the
232-
/// resolver, ensuring the planner and engine see the right column type rather
233-
/// than a generic string that must be coerced.
234-
///
235-
/// If parsing fails the text is passed through as `ParamValue::Text` so the
236-
/// engine's string-coercion path can attempt a best-effort conversion — the
237-
/// same as all other text-passthrough types.
238-
///
239-
/// # Fallback policy (catch-all arm)
240-
///
241-
/// Types the bind layer does not decode natively — `DATE`, `TIME`, `BYTEA`,
242-
/// `UUID`, `JSON`, `JSONB`, `INTERVAL`, array types, and user-defined types —
243-
/// fall through to `ParamValue::Text(text)`. The pgwire text representation of
244-
/// these types is well-defined and the AST bind emits it as a
245-
/// `SingleQuotedString`. Downstream, the planner/engine type-coerces the text
246-
/// via the same path used for literal strings in simple-query SQL.
247-
///
248-
/// Binary-format parameters are handled at a layer above this function
249-
/// (see `convert_portal_params`); they never reach this function.
250-
///
251-
/// # Why not error on unknown types
252-
///
253-
/// Postgres itself accepts text representations of every built-in type through
254-
/// the extended-query protocol; refusing here would break drivers that
255-
/// legitimately send dates/UUIDs/etc. as text.
256-
fn pgwire_text_to_param(text: &str, pg_type: &Type) -> nodedb_sql::ParamValue {
257-
match *pg_type {
258-
Type::BOOL => {
259-
let lower = text.to_lowercase();
260-
if lower == "t" || lower == "true" || lower == "1" {
261-
return nodedb_sql::ParamValue::Bool(true);
262-
}
263-
if lower == "f" || lower == "false" || lower == "0" {
264-
return nodedb_sql::ParamValue::Bool(false);
265-
}
266-
nodedb_sql::ParamValue::Text(text.to_string())
267-
}
268-
Type::INT2 | Type::INT4 | Type::INT8 => {
269-
if let Ok(n) = text.parse::<i64>() {
270-
return nodedb_sql::ParamValue::Int64(n);
271-
}
272-
nodedb_sql::ParamValue::Text(text.to_string())
273-
}
274-
Type::FLOAT4 | Type::FLOAT8 => {
275-
if let Ok(f) = text.parse::<f64>() {
276-
return nodedb_sql::ParamValue::Float64(f);
277-
}
278-
nodedb_sql::ParamValue::Text(text.to_string())
279-
}
280-
Type::NUMERIC => {
281-
// Parse NUMERIC as exact Decimal, not lossy f64.
282-
if let Ok(d) = rust_decimal::Decimal::from_str_exact(text) {
283-
return nodedb_sql::ParamValue::Decimal(d);
284-
}
285-
// If parsing fails, return typed error — do not fall back to Float
286-
// since that would silently lose precision.
287-
nodedb_sql::ParamValue::Text(text.to_string())
288-
}
289-
Type::TIMESTAMP => {
290-
// Parse ISO 8601 / PostgreSQL timestamp text to a typed NaiveDateTime.
291-
if let Some(dt) = nodedb_types::datetime::NdbDateTime::parse(text) {
292-
return nodedb_sql::ParamValue::Timestamp(dt);
293-
}
294-
nodedb_sql::ParamValue::Text(text.to_string())
295-
}
296-
Type::TIMESTAMPTZ => {
297-
// Parse ISO 8601 / PostgreSQL timestamptz text to a typed DateTime (UTC).
298-
if let Some(dt) = nodedb_types::datetime::NdbDateTime::parse(text) {
299-
return nodedb_sql::ParamValue::Timestamptz(dt);
300-
}
301-
nodedb_sql::ParamValue::Text(text.to_string())
302-
}
303-
// Text-passthrough types: wire-format text is already the
304-
// canonical representation. Engine performs type coercion.
305-
_ => nodedb_sql::ParamValue::Text(text.to_string()),
306-
}
307-
}
308-
309-
#[cfg(test)]
310-
mod tests {
311-
use pgwire::api::portal::Format;
312-
313-
use super::*;
314-
315-
fn text_format() -> Format {
316-
Format::UnifiedText
317-
}
318-
319-
fn binary_format() -> Format {
320-
Format::UnifiedBinary
321-
}
322-
323-
#[test]
324-
fn convert_null_param() {
325-
let params = vec![None];
326-
let types = vec![Some(Type::INT8)];
327-
let result = convert_portal_params(&params, &types, &text_format()).unwrap();
328-
assert_eq!(result.len(), 1);
329-
assert!(matches!(result[0], nodedb_sql::ParamValue::Null));
330-
}
331-
332-
#[test]
333-
fn convert_typed_params() {
334-
let params = vec![
335-
Some(Bytes::from_static(b"42")),
336-
Some(Bytes::from_static(b"hello")),
337-
Some(Bytes::from_static(b"true")),
338-
];
339-
let types = vec![Some(Type::INT8), Some(Type::TEXT), Some(Type::BOOL)];
340-
let result = convert_portal_params(&params, &types, &text_format()).unwrap();
341-
assert!(matches!(result[0], nodedb_sql::ParamValue::Int64(42)));
342-
assert!(matches!(&result[1], nodedb_sql::ParamValue::Text(s) if s == "hello"));
343-
assert!(matches!(result[2], nodedb_sql::ParamValue::Bool(true)));
344-
}
345-
346-
#[test]
347-
fn convert_float_param() {
348-
let params = vec![Some(Bytes::from_static(b"2.78"))];
349-
let types = vec![Some(Type::FLOAT8)];
350-
let result = convert_portal_params(&params, &types, &text_format()).unwrap();
351-
assert!(
352-
matches!(result[0], nodedb_sql::ParamValue::Float64(f) if (f - 2.78).abs() < f64::EPSILON)
353-
);
354-
}
355-
356-
#[test]
357-
fn convert_numeric_text_to_decimal() {
358-
let params = vec![Some(Bytes::from_static(b"123.45"))];
359-
let types = vec![Some(Type::NUMERIC)];
360-
let result = convert_portal_params(&params, &types, &text_format()).unwrap();
361-
match &result[0] {
362-
nodedb_sql::ParamValue::Decimal(decimal) => assert_eq!(decimal.to_string(), "123.45"),
363-
other => panic!("expected Decimal, got {other:?}"),
364-
}
365-
}
366-
367-
fn assert_binary_type_rejected(ty: Type, bytes: &'static [u8], name: &str) {
368-
let params = vec![Some(Bytes::from_static(bytes))];
369-
let types = vec![Some(ty)];
370-
let error = convert_portal_params(&params, &types, &binary_format()).unwrap_err();
371-
let message = error.to_string();
372-
assert!(message.contains(name) || message.contains("0A000"));
373-
}
374-
375-
#[test]
376-
fn convert_numeric_binary_returns_error() {
377-
assert_binary_type_rejected(Type::NUMERIC, &[0x00, 0x03, 0x00, 0x02], "NUMERIC");
378-
}
379-
380-
#[test]
381-
fn convert_timestamp_binary_returns_error() {
382-
assert_binary_type_rejected(Type::TIMESTAMP, &[0; 8], "TIMESTAMP");
383-
}
384-
385-
#[test]
386-
fn convert_timestamptz_binary_returns_error() {
387-
assert_binary_type_rejected(Type::TIMESTAMPTZ, &[0; 8], "TIMESTAMPTZ");
388-
}
389-
390-
fn assert_text_param(
391-
input: &'static [u8],
392-
ty: Type,
393-
expected: fn(&nodedb_sql::ParamValue) -> bool,
394-
) {
395-
let params = vec![Some(Bytes::from_static(input))];
396-
let types = vec![Some(ty)];
397-
let result = convert_portal_params(&params, &types, &text_format()).unwrap();
398-
assert!(expected(&result[0]));
399-
}
400-
401-
#[test]
402-
fn convert_timestamp_text_to_typed() {
403-
assert_text_param(b"2024-01-01 00:00:00", Type::TIMESTAMP, |value| {
404-
matches!(value, nodedb_sql::ParamValue::Timestamp(_))
405-
});
406-
}
407-
408-
#[test]
409-
fn convert_timestamptz_text_to_typed() {
410-
assert_text_param(b"2024-01-01 00:00:00+00", Type::TIMESTAMPTZ, |value| {
411-
matches!(value, nodedb_sql::ParamValue::Timestamptz(_))
412-
});
413-
}
414-
415-
#[test]
416-
fn convert_bool_variants() {
417-
for (input, expected) in [("t", true), ("f", false), ("1", true), ("0", false)] {
418-
let params = vec![Some(Bytes::from(input))];
419-
let types = vec![Some(Type::BOOL)];
420-
let result = convert_portal_params(&params, &types, &text_format()).unwrap();
421-
assert!(matches!(result[0], nodedb_sql::ParamValue::Bool(value) if value == expected));
422-
}
423-
}
424-
425-
#[test]
426-
fn passthrough_date_text() {
427-
let value = pgwire_text_to_param("2026-04-19", &Type::DATE);
428-
assert!(matches!(&value, nodedb_sql::ParamValue::Text(text) if text == "2026-04-19"));
429-
}
430-
431-
#[test]
432-
fn timestamp_text_parses_to_typed() {
433-
let value = pgwire_text_to_param("2026-04-19 12:00:00", &Type::TIMESTAMP);
434-
assert!(matches!(value, nodedb_sql::ParamValue::Timestamp(_)));
435-
}
436-
437-
#[test]
438-
fn timestamptz_text_parses_to_typed() {
439-
let value = pgwire_text_to_param("2026-04-19 12:00:00+00", &Type::TIMESTAMPTZ);
440-
assert!(matches!(value, nodedb_sql::ParamValue::Timestamptz(_)));
441-
}
442-
443-
#[test]
444-
fn passthrough_uuid_text() {
445-
let uuid = "550e8400-e29b-41d4-a716-446655440000";
446-
let value = pgwire_text_to_param(uuid, &Type::UUID);
447-
assert!(matches!(&value, nodedb_sql::ParamValue::Text(text) if text == uuid));
448-
}
449-
450-
#[test]
451-
fn passthrough_jsonb_text() {
452-
let json = r#"{"a":1}"#;
453-
let value = pgwire_text_to_param(json, &Type::JSONB);
454-
assert!(matches!(&value, nodedb_sql::ParamValue::Text(text) if text == json));
455-
}
456-
457-
#[test]
458-
fn passthrough_bytea_hex_text() {
459-
let value = pgwire_text_to_param("\\xDEADBEEF", &Type::BYTEA);
460-
assert!(matches!(&value, nodedb_sql::ParamValue::Text(text) if text == "\\xDEADBEEF"));
461-
}
462-
463-
#[test]
464-
fn int_parse_failure_falls_back_to_text() {
465-
let value = pgwire_text_to_param("abc", &Type::INT8);
466-
assert!(matches!(&value, nodedb_sql::ParamValue::Text(text) if text == "abc"));
467-
}
468-
469-
#[test]
470-
fn unknown_type_routes_to_text() {
471-
let value = pgwire_text_to_param("42", &Type::UNKNOWN);
472-
assert!(matches!(&value, nodedb_sql::ParamValue::Text(text) if text == "42"));
473-
}
474-
}

nodedb/src/control/server/pgwire/handler/prepared/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
pub mod describe;
44
pub mod execute;
5+
pub mod param_bind;
56
pub mod parser;
67
pub mod result_format;
78
pub mod statement;

0 commit comments

Comments
 (0)