Convert pagination ISE into a Bad Request response

src/controllers/helpers/pagination.rs

@@ -1,5 +1,5 @@
 use crate::models::helpers::with_count::*;
-use crate::util::errors::{cargo_err, AppResult};
+use crate::util::errors::{bad_request, AppResult};
 use diesel::pg::Pg;
 use diesel::prelude::*;
 use diesel::query_builder::*;
@@ -16,9 +16,9 @@ pub(crate) enum Page {
 impl Page {
     fn new(params: &IndexMap<String, String>) -> AppResult<Self> {
         if let Some(s) = params.get("page") {
-            let numeric_page = s.parse()?;
+            let numeric_page = s.parse().map_err(|e| bad_request(&e))?;
             if numeric_page < 1 {
-                return Err(cargo_err(&format_args!(
+                return Err(bad_request(&format_args!(
                     "page indexing starts from 1, page {} is invalid",
                     numeric_page,
                 )));
@@ -44,11 +44,11 @@ impl PaginationOptions {
 
         let per_page = params
             .get("per_page")
-            .map(|s| s.parse())
+            .map(|s| s.parse().map_err(|e| bad_request(&e)))
             .unwrap_or(Ok(DEFAULT_PER_PAGE))?;
 
         if per_page > MAX_PER_PAGE {
-            return Err(cargo_err(&format_args!(
+            return Err(bad_request(&format_args!(
                 "cannot request more than {} items",
                 MAX_PER_PAGE,
             )));
@@ -182,3 +182,30 @@ where
         Ok(())
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{Page, PaginationOptions};
+    use indexmap::IndexMap;
+
+    #[test]
+    fn page_must_be_a_number() {
+        let mut params = IndexMap::new();
+        params.insert(String::from("page"), String::from("not a number"));
+        let page_error = Page::new(&params).unwrap_err().response().unwrap();
+
+        assert_eq!(page_error.status, (400, "Bad Request"));
+    }
+
+    #[test]
+    fn per_page_must_be_a_number() {
+        let mut params = IndexMap::new();
+        params.insert(String::from("per_page"), String::from("not a number"));
+        let per_page_error = PaginationOptions::new(&params)
+            .unwrap_err()
+            .response()
+            .unwrap();
+
+        assert_eq!(per_page_error.status, (400, "Bad Request"));
+    }
+}

src/tests/krate.rs

@@ -2364,3 +2364,31 @@ fn pagination_links_included_if_applicable() {
     assert_eq!(None, page4.meta.next_page);
     assert_eq!(Some("?page=2&per_page=1".to_string()), page3.meta.prev_page);
 }
+
+#[test]
+fn pagination_parameters_only_accept_integers() {
+    let (app, anon, user) = TestApp::init().with_user();
+    let user = user.as_model();
+
+    app.db(|conn| {
+        CrateBuilder::new("pagination_links_1", user.id).expect_build(conn);
+        CrateBuilder::new("pagination_links_2", user.id).expect_build(conn);
+        CrateBuilder::new("pagination_links_3", user.id).expect_build(conn);
+    });
+
+    let invalid_per_page_json = anon
+        .get_with_query::<()>("/api/v1/crates", "page=1&per_page=100%22%EF%BC%8Cexception")
+        .bad_with_status(400);
+    assert_eq!(
+        invalid_per_page_json.errors[0].detail,
+        "invalid digit found in string"
+    );
+
+    let invalid_page_json = anon
+        .get_with_query::<()>("/api/v1/crates", "page=100%22%EF%BC%8Cexception&per_page=1")
+        .bad_with_status(400);
+    assert_eq!(
+        invalid_page_json.errors[0].detail,
+        "invalid digit found in string"
+    );
+}

src/tests/user.rs

@@ -296,7 +296,7 @@ fn following() {
     assert_eq!(r.meta.more, false);
 
     user.get_with_query::<()>("/api/v1/me/updates", "page=0")
-        .bad_with_status(200); // TODO: Should be 500
+        .bad_with_status(400);
 }
 
 #[test]