diff --git a/Cargo.lock b/Cargo.lock index 1b00f89d6..2e91011c7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3170,6 +3170,7 @@ dependencies = [ "derive_more", "fnv", "futures", + "hex", "hickory-resolver", "http-body-util", "hyper", diff --git a/pgdog/Cargo.toml b/pgdog/Cargo.toml index 63a81998e..332287350 100644 --- a/pgdog/Cargo.toml +++ b/pgdog/Cargo.toml @@ -78,6 +78,7 @@ azure_core = "0.34.0" crc32c = "0.6.8" bit-vec = "0.8" reqwest = { version = "0.12", default-features = false, features = ["rustls-tls-webpki-roots-no-provider"] } +hex = "0.4" [target.'cfg(unix)'.dependencies] libc = "0.2" diff --git a/pgdog/src/frontend/router/parser/error.rs b/pgdog/src/frontend/router/parser/error.rs index bddf059fc..b0e4c20a1 100644 --- a/pgdog/src/frontend/router/parser/error.rs +++ b/pgdog/src/frontend/router/parser/error.rs @@ -55,6 +55,9 @@ pub enum Error { #[error("missing parameter: ${0}")] MissingParameter(usize), + #[error("expected parameter ${0} to be an integer, got \'{1}\' instead")] + ParameterNotInteger(usize, String), + #[error("column has no associated table")] ColumnNoTable, diff --git a/pgdog/src/frontend/router/parser/limit.rs b/pgdog/src/frontend/router/parser/limit.rs index 447effa8f..18fe7d067 100644 --- a/pgdog/src/frontend/router/parser/limit.rs +++ b/pgdog/src/frontend/router/parser/limit.rs @@ -52,12 +52,13 @@ impl<'a> LimitClause<'a> { if param.is_null() { Ok(None) } else { - Ok(Some( - param - .bigint() - .ok_or(Error::MissingParameter(*number as usize))? - as usize, - )) + match param.bigint() { + Some(param) => Ok(Some(param as usize)), + None => Err(Error::ParameterNotInteger( + *number as usize, + param.text_debug(), + )), + } } } else { Ok(None) diff --git a/pgdog/src/frontend/router/parser/query/test/test_select.rs b/pgdog/src/frontend/router/parser/query/test/test_select.rs index fedc73cae..084df0d7b 100644 --- a/pgdog/src/frontend/router/parser/query/test/test_select.rs +++ b/pgdog/src/frontend/router/parser/query/test/test_select.rs @@ -74,6 +74,45 @@ fn test_limit_offset_with_params() { assert_eq!(route.limit().offset, Some(25)); } +#[test] +fn test_limit_offset_with_bad_params() { + let mut test = QueryParserTest::new(); + + let command = test.try_execute(vec![ + Parse::named("__test_limit", "SELECT * FROM users LIMIT $1 OFFSET $2").into(), + Bind::new_params( + "__test_limit", + &[Parameter::new(b"apples"), Parameter::new(b"25")], + ) + .into(), + Execute::new().into(), + Sync.into(), + ]); + + let err = command.err().expect("limit should fail"); + assert_eq!( + err.to_string(), + "expected parameter $1 to be an integer, got 'apples' instead" + ); + + let command = test.try_execute(vec![ + Parse::named("__test_limit", "SELECT * FROM users LIMIT $1 OFFSET $2").into(), + Bind::new_params( + "__test_limit", + &[Parameter::new(b"25"), Parameter::new(b"oranges")], + ) + .into(), + Execute::new().into(), + Sync.into(), + ]); + + let err = command.err().expect("offset should fail"); + assert_eq!( + err.to_string(), + "expected parameter $2 to be an integer, got 'oranges' instead" + ); +} + #[test] fn test_distinct_row() { let mut test = QueryParserTest::new(); diff --git a/pgdog/src/net/messages/bind.rs b/pgdog/src/net/messages/bind.rs index b35b9f36d..70d868031 100644 --- a/pgdog/src/net/messages/bind.rs +++ b/pgdog/src/net/messages/bind.rs @@ -71,6 +71,15 @@ impl<'a> ParameterWithFormat<'a> { from_utf8(&self.parameter.data).ok() } + /// Get the parameter as a textual value for debugging purposes only. + pub fn text_debug(&self) -> String { + if let Some(text) = self.text() { + text.to_string() + } else { + hex::encode(self.data()) + } + } + /// Get BIGINT if one is encoded in the field. pub fn bigint(&self) -> Option { Self::decode(self)