diff --git a/src/controllers/helpers/pagination.rs b/src/controllers/helpers/pagination.rs index 9fd7a4a7a92..536ecdf0f66 100644 --- a/src/controllers/helpers/pagination.rs +++ b/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) -> AppResult { 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(¶ms).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(¶ms) + .unwrap_err() + .response() + .unwrap(); + + assert_eq!(per_page_error.status, (400, "Bad Request")); + } +} diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 3b710eaae24..a83c89f4b2d 100644 --- a/src/tests/krate.rs +++ b/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" + ); +} diff --git a/src/tests/user.rs b/src/tests/user.rs index 23129cf4878..ee1e77bc7b4 100644 --- a/src/tests/user.rs +++ b/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]