Merge pull request #210 from jgallagher/check-integer-out-of-range

Check integer ranges in FromSql.

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rusqlite"
-version = "0.9.1"
+version = "0.9.2"
 authors = ["John Gallagher <jgallagher@bignerdranch.com>"]
 description = "Ergonomic wrapper for SQLite"
 repository = "https://github.com/jgallagher/rusqlite"

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

src/error.rs

@@ -25,6 +25,11 @@ pub enum Error {
     /// the requested Rust type.
     FromSqlConversionFailure(usize, Type, Box<error::Error + Send + Sync>),
 
+    /// 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 |

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)
             }

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<I: RowIndex, T: FromSql>(&self, idx: I) -> T {
         self.get_checked(idx).unwrap()
     }
@@ -1129,6 +1131,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)
             }

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
+    /// Error when an SQLite value is requested, but the type of the result cannot be converted to
-    /// requested Rust type.
+    /// 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<Error + Send + Sync>),
 }
@@ -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(),
         }
     }
@@ -46,11 +53,28 @@ pub trait FromSql: Sized {
     fn column_result(value: ValueRef) -> FromSqlResult<Self>;
 }
 
-impl FromSql for i32 {
+macro_rules! from_sql_integral(
+    ($t:ident) => (
+        impl FromSql for $t {
             fn column_result(value: ValueRef) -> FromSqlResult<Self> {
-        i64::column_result(value).map(|i| i as i32)
+                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<Self> {
@@ -103,3 +127,45 @@ impl FromSql for Value {
         Ok(value.into())
     }
 }
+
+#[cfg(test)]
+mod test {
+    use {Connection, Error};
+    use super::FromSql;
+
+    fn checked_memory_handle() -> Connection {
+        Connection::open_in_memory().unwrap()
+    }
+
+    #[test]
+    fn test_integral_ranges() {
+        let db = checked_memory_handle();
+
+        fn check_ranges<T>(db: &Connection, out_of_range: &[i64], in_range: &[i64])
+            where T: Into<i64> + 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());
+            }
+        }
+
+        check_ranges::<i8>(&db, &[-129, 128], &[-128, 0, 1, 127]);
+        check_ranges::<i16>(&db, &[-32769, 32768], &[-32768, -1, 0, 1, 32767]);
+        check_ranges::<i32>(&db,
+                            &[-2147483649, 2147483648],
+                            &[-2147483648, -1, 0, 1, 2147483647]);
+        check_ranges::<u8>(&db, &[-2, -1, 256], &[0, 1, 255]);
+        check_ranges::<u16>(&db, &[-2, -1, 65536], &[0, 1, 65535]);
+        check_ranges::<u32>(&db, &[-2, -1, 4294967296], &[0, 1, 4294967295]);
+    }
+}

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<T: ToSql> ToSql for Option<T> {
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::ToSql;
+
+    fn is_to_sql<T: ToSql>() {}
+
+    #[test]
+    fn test_integral_types() {
+        is_to_sql::<i8>();
+        is_to_sql::<i16>();
+        is_to_sql::<i32>();
+        is_to_sql::<i64>();
+        is_to_sql::<u8>();
+        is_to_sql::<u16>();
+        is_to_sql::<u32>();
+    }
+}

src/types/value.rs

@@ -30,11 +30,22 @@ impl From<bool> for Value {
     }
 }
 
-impl From<i32> for Value {
+macro_rules! from_i64(
-    fn from(i: i32) -> Value {
+    ($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<i64> for Value {
     fn from(i: i64) -> Value {