From 1974ee573cee38a67c686f76e3c584fbcd265322 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sun, 22 Jan 2017 19:26:19 -0500 Subject: [PATCH 1/6] Add range checks for i32's FromSql impl. --- src/error.rs | 12 ++++++++++ src/lib.rs | 1 + src/types/from_sql.rs | 52 ++++++++++++++++++++++++++++++++++++++++--- 3 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/error.rs b/src/error.rs index 1b41935..f0ee5b3 100644 --- a/src/error.rs +++ b/src/error.rs @@ -25,6 +25,11 @@ pub enum Error { /// the requested Rust type. FromSqlConversionFailure(usize, Type, Box), + /// Error when SQLite gives us an integral value outside the range of the requested type (e.g., + /// trying to get the value 1000 into a `u8`). The associated `c_int` is the column index, and + /// the associated `i64` is the value returned by SQLite. + IntegralValueOutOfRange(c_int, i64), + /// Error converting a string to UTF-8. Utf8Error(str::Utf8Error), @@ -99,6 +104,9 @@ impl fmt::Display for Error { i, err) } + Error::IntegralValueOutOfRange(col, val) => { + write!(f, "Integer {} out of range at index {}", val, col) + } Error::Utf8Error(ref err) => err.fmt(f), Error::NulError(ref err) => err.fmt(f), Error::InvalidParameterName(ref name) => write!(f, "Invalid parameter name: {}", name), @@ -133,6 +141,9 @@ impl error::Error for Error { "SQLite was compiled or configured for single-threaded use only" } Error::FromSqlConversionFailure(_, _, ref err) => err.description(), + Error::IntegralValueOutOfRange(_, _) => { + "integral value out of range of requested type" + } Error::Utf8Error(ref err) => err.description(), Error::InvalidParameterName(_) => "invalid parameter name", Error::NulError(ref err) => err.description(), @@ -160,6 +171,7 @@ impl error::Error for Error { Error::Utf8Error(ref err) => Some(err), Error::NulError(ref err) => Some(err), + Error::IntegralValueOutOfRange(_, _) | Error::SqliteSingleThreadedMode | Error::InvalidParameterName(_) | Error::ExecuteReturnedResults | diff --git a/src/lib.rs b/src/lib.rs index b23d19c..d7016e1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1129,6 +1129,7 @@ impl<'a, 'stmt> Row<'a, 'stmt> { let value = unsafe { ValueRef::new(&self.stmt.stmt, idx) }; FromSql::column_result(value).map_err(|err| match err { FromSqlError::InvalidType => Error::InvalidColumnType(idx, value.data_type()), + FromSqlError::OutOfRange(i) => Error::IntegralValueOutOfRange(idx, i), FromSqlError::Other(err) => { Error::FromSqlConversionFailure(idx as usize, value.data_type(), err) } diff --git a/src/types/from_sql.rs b/src/types/from_sql.rs index 3c78d7c..da4588f 100644 --- a/src/types/from_sql.rs +++ b/src/types/from_sql.rs @@ -5,9 +5,13 @@ use std::fmt; /// Enum listing possible errors from `FromSql` trait. #[derive(Debug)] pub enum FromSqlError { - /// Error when an SQLite value is requested, but the type of the result cannot be converted to the - /// requested Rust type. + /// Error when an SQLite value is requested, but the type of the result cannot be converted to + /// the requested Rust type. InvalidType, + + /// Error when the i64 value returned by SQLite cannot be stored into the requested type. + OutOfRange(i64), + /// An error case available for implementors of the `FromSql` trait. Other(Box), } @@ -16,6 +20,7 @@ impl fmt::Display for FromSqlError { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { FromSqlError::InvalidType => write!(f, "Invalid type"), + FromSqlError::OutOfRange(i) => write!(f, "Value {} out of range", i), FromSqlError::Other(ref err) => err.fmt(f), } } @@ -25,6 +30,7 @@ impl Error for FromSqlError { fn description(&self) -> &str { match *self { FromSqlError::InvalidType => "invalid type", + FromSqlError::OutOfRange(_) => "value out of range", FromSqlError::Other(ref err) => err.description(), } } @@ -33,6 +39,7 @@ impl Error for FromSqlError { fn cause(&self) -> Option<&Error> { match *self { FromSqlError::InvalidType => None, + FromSqlError::OutOfRange(_) => None, FromSqlError::Other(ref err) => err.cause(), } } @@ -48,7 +55,13 @@ pub trait FromSql: Sized { impl FromSql for i32 { fn column_result(value: ValueRef) -> FromSqlResult { - i64::column_result(value).map(|i| i as i32) + i64::column_result(value).and_then(|i| { + if i < i32::min_value() as i64 || i > i32::max_value() as i64 { + Err(FromSqlError::OutOfRange(i)) + } else { + Ok(i as i32) + } + }) } } @@ -103,3 +116,36 @@ impl FromSql for Value { Ok(value.into()) } } + +#[cfg(test)] +mod test { + use {Connection, Error}; + + fn checked_memory_handle() -> Connection { + Connection::open_in_memory().unwrap() + } + + #[test] + fn test_integral_ranges() { + let db = checked_memory_handle(); + + fn assert_out_of_range_error(err: Error, value: i64) { + match err { + Error::IntegralValueOutOfRange(_, bad) => assert_eq!(bad, value), + _ => panic!("unexpected error {}", err), + } + } + + // i32 + for bad in &[-2147483649, 2147483648] { + let err = db.query_row("SELECT ?", &[bad], |r| r.get_checked::<_, i32>(0)) + .unwrap() + .unwrap_err(); + assert_out_of_range_error(err, *bad); + } + for good in &[-2147483648, 2147483647] { + assert_eq!(*good, + db.query_row("SELECT ?", &[good], |r| r.get::<_, i32>(0)).unwrap()); + } + } +} From 846a59695c820fa9de8f1611e2590fb884544057 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sun, 22 Jan 2017 19:40:48 -0500 Subject: [PATCH 2/6] Add range-checked FromSql impls for i8, i16, u8, u16, u32. --- src/types/from_sql.rs | 70 +++++++++++++++++++++++++++---------------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/src/types/from_sql.rs b/src/types/from_sql.rs index da4588f..32ad3a3 100644 --- a/src/types/from_sql.rs +++ b/src/types/from_sql.rs @@ -53,17 +53,28 @@ pub trait FromSql: Sized { fn column_result(value: ValueRef) -> FromSqlResult; } -impl FromSql for i32 { - fn column_result(value: ValueRef) -> FromSqlResult { - i64::column_result(value).and_then(|i| { - if i < i32::min_value() as i64 || i > i32::max_value() as i64 { - Err(FromSqlError::OutOfRange(i)) - } else { - Ok(i as i32) +macro_rules! from_sql_integral( + ($t:ident) => ( + impl FromSql for $t { + fn column_result(value: ValueRef) -> FromSqlResult { + i64::column_result(value).and_then(|i| { + if i < $t::min_value() as i64 || i > $t::max_value() as i64 { + Err(FromSqlError::OutOfRange(i)) + } else { + Ok(i as $t) + } + }) } - }) - } -} + } + ) +); + +from_sql_integral!(i8); +from_sql_integral!(i16); +from_sql_integral!(i32); +from_sql_integral!(u8); +from_sql_integral!(u16); +from_sql_integral!(u32); impl FromSql for i64 { fn column_result(value: ValueRef) -> FromSqlResult { @@ -120,6 +131,7 @@ impl FromSql for Value { #[cfg(test)] mod test { use {Connection, Error}; + use super::FromSql; fn checked_memory_handle() -> Connection { Connection::open_in_memory().unwrap() @@ -129,23 +141,31 @@ mod test { fn test_integral_ranges() { let db = checked_memory_handle(); - fn assert_out_of_range_error(err: Error, value: i64) { - match err { - Error::IntegralValueOutOfRange(_, bad) => assert_eq!(bad, value), - _ => panic!("unexpected error {}", err), + fn check_ranges(db: &Connection, out_of_range: &[i64], in_range: &[i64]) + where T: Into + FromSql + ::std::fmt::Debug + { + for n in out_of_range { + let err = db.query_row("SELECT ?", &[n], |r| r.get_checked::<_, T>(0)) + .unwrap() + .unwrap_err(); + match err { + Error::IntegralValueOutOfRange(_, value) => assert_eq!(*n, value), + _ => panic!("unexpected error: {}", err), + } + } + for n in in_range { + assert_eq!(*n, + db.query_row("SELECT ?", &[n], |r| r.get::<_, T>(0)).unwrap().into()); } } - // i32 - for bad in &[-2147483649, 2147483648] { - let err = db.query_row("SELECT ?", &[bad], |r| r.get_checked::<_, i32>(0)) - .unwrap() - .unwrap_err(); - assert_out_of_range_error(err, *bad); - } - for good in &[-2147483648, 2147483647] { - assert_eq!(*good, - db.query_row("SELECT ?", &[good], |r| r.get::<_, i32>(0)).unwrap()); - } + check_ranges::(&db, &[-129, 128], &[-128, 0, 1, 127]); + check_ranges::(&db, &[-32769, 32768], &[-32768, -1, 0, 1, 32767]); + check_ranges::(&db, + &[-2147483649, 2147483648], + &[-2147483648, -1, 0, 1, 2147483647]); + check_ranges::(&db, &[-2, -1, 256], &[0, 1, 255]); + check_ranges::(&db, &[-2, -1, 65536], &[0, 1, 65535]); + check_ranges::(&db, &[-2, -1, 4294967296], &[0, 1, 4294967295]); } } From 68ae7de1d54dd13bfe0da1957fbd63e74d004f03 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sun, 22 Jan 2017 19:42:51 -0500 Subject: [PATCH 3/6] Update docs on Row::get --- src/lib.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index d7016e1..924d301 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1105,9 +1105,11 @@ impl<'a, 'stmt> Row<'a, 'stmt> { /// /// ## Failure /// - /// Panics if the underlying SQLite column type is not a valid type as a source for `T`. + /// Panics if calling `row.get_checked(idx)` would return an error, including: /// - /// Panics if `idx` is outside the range of columns in the returned query. + /// * If the underlying SQLite column type is not a valid type as a source for `T` + /// * If the underlying SQLite integral value is outside the range representable by `T` + /// * If `idx` is outside the range of columns in the returned query pub fn get(&self, idx: I) -> T { self.get_checked(idx).unwrap() } From 7c072bf55e55fd7e476067f011db1b7d9031a33b Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sun, 22 Jan 2017 19:55:49 -0500 Subject: [PATCH 4/6] impl ToSql for i8,i16,u8,u16,u32 --- src/types/to_sql.rs | 23 +++++++++++++++++++++++ src/types/value.rs | 21 ++++++++++++++++----- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/types/to_sql.rs b/src/types/to_sql.rs index d6b3daf..3738217 100644 --- a/src/types/to_sql.rs +++ b/src/types/to_sql.rs @@ -57,8 +57,13 @@ macro_rules! to_sql_self( to_sql_self!(Null); to_sql_self!(bool); +to_sql_self!(i8); +to_sql_self!(i16); to_sql_self!(i32); to_sql_self!(i64); +to_sql_self!(u8); +to_sql_self!(u16); +to_sql_self!(u32); to_sql_self!(f64); impl<'a, T: ?Sized> ToSql for &'a T @@ -95,3 +100,21 @@ impl ToSql for Option { } } } + +#[cfg(test)] +mod test { + use super::ToSql; + + fn is_to_sql() {} + + #[test] + fn test_integral_types() { + is_to_sql::(); + is_to_sql::(); + is_to_sql::(); + is_to_sql::(); + is_to_sql::(); + is_to_sql::(); + is_to_sql::(); + } +} diff --git a/src/types/value.rs b/src/types/value.rs index 3628546..e192826 100644 --- a/src/types/value.rs +++ b/src/types/value.rs @@ -30,11 +30,22 @@ impl From for Value { } } -impl From for Value { - fn from(i: i32) -> Value { - Value::Integer(i as i64) - } -} +macro_rules! from_i64( + ($t:ty) => ( + impl From<$t> for Value { + fn from(i: $t) -> Value { + Value::Integer(i as i64) + } + } + ) +); + +from_i64!(i8); +from_i64!(i16); +from_i64!(i32); +from_i64!(u8); +from_i64!(u16); +from_i64!(u32); impl From for Value { fn from(i: i64) -> Value { From 7b32713313834fab4d8b354d1b35e9ecbd83a95d Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sun, 22 Jan 2017 19:59:13 -0500 Subject: [PATCH 5/6] Bump to 0.9.2 --- Cargo.toml | 2 +- Changelog.md | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index c27b41a..f66daac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rusqlite" -version = "0.9.1" +version = "0.9.2" authors = ["John Gallagher "] description = "Ergonomic wrapper for SQLite" repository = "https://github.com/jgallagher/rusqlite" diff --git a/Changelog.md b/Changelog.md index 63631e3..1450f0e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,3 +1,12 @@ +# Version 0.9.2 (2017-01-22) + +* Bugfix: The `FromSql` impl for `i32` now returns an error instead of + truncating if the underlying SQLite value is out of `i32`'s range. +* Added `FromSql` and `ToSql` impls for `i8`, `i16`, `u8`, `u16`, and `u32`. + `i32` and `i64` already had impls. `u64` is omitted because their range + cannot be represented by `i64`, which is the type we use to communicate with + SQLite. + # Version 0.9.1 (2017-01-20) * BREAKING CHANGE: `Connection::close()` now returns a `Result<(), (Connection, Error)>` instead From e180ab15ce3da1247662f2d7240dc14cfdd3d909 Mon Sep 17 00:00:00 2001 From: John Gallagher Date: Sun, 22 Jan 2017 20:05:06 -0500 Subject: [PATCH 6/6] Fix incomplete match under `functions` feature --- src/functions.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/functions.rs b/src/functions.rs index 9d19137..0ab2e1c 100644 --- a/src/functions.rs +++ b/src/functions.rs @@ -198,6 +198,7 @@ impl<'a> Context<'a> { FromSqlError::InvalidType => { Error::InvalidFunctionParameterType(idx, value.data_type()) } + FromSqlError::OutOfRange(i) => Error::IntegralValueOutOfRange(idx as c_int, i), FromSqlError::Other(err) => { Error::FromSqlConversionFailure(idx, value.data_type(), err) }