From dfc092dd2a57c159a863f3974ce29f66112d0c4f Mon Sep 17 00:00:00 2001 From: Sriram Date: Tue, 26 Jul 2022 13:57:01 -0400 Subject: [PATCH 1/2] feat: parse path with multiple slashes Co-authored-by: Roman Signed-off by: Sriram Signed-off-by: Sriram --- crates/server/src/handle.rs | 134 ++++++++++++++++++++++++++++-------- 1 file changed, 104 insertions(+), 30 deletions(-) diff --git a/crates/server/src/handle.rs b/crates/server/src/handle.rs index 1b174553..0ae6d245 100644 --- a/crates/server/src/handle.rs +++ b/crates/server/src/handle.rs @@ -3,7 +3,9 @@ use super::{repos, tags, trees, users}; -use drawbridge_type::{RepositoryName, TagName, TreePath, UserName}; +use drawbridge_type::{RepositoryName, TagName, TreeName, TreePath, UserName}; + +use std::str::FromStr; use axum::body::Body; use axum::handler::Handler; @@ -28,7 +30,7 @@ pub async fn handle(mut req: Request) -> impl IntoResponse { } trace!(target: "app::handle", "begin HTTP request handling {:?}", req); - let path = req.uri().path().trim_start_matches('/'); + let path = req.uri().path().trim_matches('/'); let (ver, path) = path .strip_prefix("api") .ok_or_else(|| not_found(path))? @@ -52,23 +54,29 @@ pub async fn handle(mut req: Request) -> impl IntoResponse { let (head, tail) = path .trim_start_matches('/') .split_once("/_") - .map(|(left, right)| (left.to_string(), format!("_{right}"))) - .unwrap_or((path.to_string(), "".into())); + .map(|(left, right)| (left.trim_end_matches('/').to_string(), format!("_{right}"))) + .unwrap_or((path.to_string(), "".to_string())); if head.is_empty() { return Err(not_found(path)); } let extensions = req.extensions_mut(); - let (user, head) = head.split_once('/').unwrap_or((&head, "")); - let user = user.parse::().map_err(|e| { - ( - StatusCode::BAD_REQUEST, - format!("Failed to parse user name: {e}"), - ) - })?; + let (user, head) = head + .split_once('/') + .unwrap_or((head.trim_start_matches('/'), "")); + let user = user + .trim_end_matches('/') + .parse::() + .map_err(|e| { + ( + StatusCode::BAD_REQUEST, + format!("Failed to parse user name: {e}"), + ) + })?; trace!(target: "app::handle", "parsed user name: `{user}`"); assert_eq!(extensions.insert(user), None, "duplicate user name"); + let head = head.trim_start_matches('/'); if head.is_empty() { return match *req.method() { Method::HEAD => Ok(users::head.into_service().call(req).await.into_response()), @@ -81,16 +89,20 @@ pub async fn handle(mut req: Request) -> impl IntoResponse { }; } - let repo = head.parse::().map_err(|e| { - ( - StatusCode::BAD_REQUEST, - format!("Failed to parse repository name: {e}"), - ) - })?; + let repo = head + .trim_end_matches('/') + .parse::() + .map_err(|e| { + ( + StatusCode::BAD_REQUEST, + format!("Failed to parse repository name: {e}"), + ) + })?; trace!(target: "app::handle", "parsed repository name: `{repo}`"); assert_eq!(extensions.insert(repo), None, "duplicate repository name"); - let mut tail = tail.splitn(4, '/'); + let mut tail = tail.split('/').filter(|x| !x.is_empty()); + match (tail.next(), tail.next(), tail.next()) { (None | Some(""), None, None) => match *req.method() { Method::HEAD => Ok(repos::head.into_service().call(req).await.into_response()), @@ -109,12 +121,16 @@ pub async fn handle(mut req: Request) -> impl IntoResponse { )), }, (Some("_tag"), Some(tag), prop @ (None | Some("tree"))) => { - let tag = tag.parse::().map_err(|e| { - ( - StatusCode::BAD_REQUEST, - format!("Failed to parse tag name: {e}"), - ) - })?; + let tag = tag + .trim_start_matches('/') + .trim_end_matches('/') + .parse::() + .map_err(|e| { + ( + StatusCode::BAD_REQUEST, + format!("Failed to parse tag name: {e}"), + ) + })?; trace!(target: "app::handle", "parsed tag name: `{tag}`"); assert_eq!(extensions.insert(tag), None, "duplicate tag name"); @@ -130,12 +146,15 @@ pub async fn handle(mut req: Request) -> impl IntoResponse { }; } - let path = tail.next().unwrap_or("").parse::().map_err(|e| { - ( - StatusCode::BAD_REQUEST, - format!("Failed to parse tree path: {e}"), - ) - })?; + let path = tail + .map(TreeName::from_str) + .collect::>() + .map_err(|e| { + ( + StatusCode::BAD_REQUEST, + format!("Failed to parse tree path: {e}"), + ) + })?; trace!(target: "app::handle", "parsed tree path: `{path}`"); assert_eq!(extensions.insert(path), None, "duplicate tree path"); match *req.method() { @@ -154,3 +173,58 @@ pub async fn handle(mut req: Request) -> impl IntoResponse { )), } } + +#[async_std::test] +async fn multiple_slashes_missing() { + let request = Request::builder() + .uri("https://www.rust-lang.org/") + .header("User-Agent", "my-awesome-agent/1.0") + .body(hyper::Body::empty()); + println!("{:?}", request); + + let res = handle(request.unwrap()).await; + + // Temporary print to ensure test is working as intended. + // println!("{}", res.into_response().status()); + assert_eq!(res.into_response().status(), 404); + + let request = Request::builder() + .uri("https://www.rust-lang.org///") + .header("User-Agent", "my-awesome-agent///1.0") + .body(hyper::Body::empty()); + println!("{:?}", request); + + let res = handle(request.unwrap()).await; + + // Temporary print to ensure test is working as intended. + // println!("{}", res.into_response().status()); + assert_eq!(res.into_response().status(), 404); + return (); +} + +/// Unit test to handle multiple slash path parsing +#[async_std::test] +async fn multiple_slashes_found() { + let request = Request::builder() + .uri("http://httpbin.org/ip") + .body(hyper::Body::empty()); + println!("{:?}", request); + + let res = handle(request.unwrap()).await; + + // Temporary print to ensure test is working as intended. + // println!("{}", res.into_response().status()); + assert_eq!(res.into_response().status(), 200); + + let request = Request::builder() + .uri("http://httpbin.org///ip") + .body(hyper::Body::empty()); + println!("{:?}", request); + + let res = handle(request.unwrap()).await; + + // Temporary print to ensure test is working as intended. + // println!("{}", res.into_response().status()); + assert_eq!(res.into_response().status(), 200); + return (); +} From 4fceb20c3c8590a40db77c7805f84587e90ea583 Mon Sep 17 00:00:00 2001 From: Sriram Date: Thu, 11 Aug 2022 13:58:29 -0400 Subject: [PATCH 2/2] fix: test cases passing Signed-off-by: Sriram --- crates/server/src/handle.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/server/src/handle.rs b/crates/server/src/handle.rs index 0ae6d245..430ae66d 100644 --- a/crates/server/src/handle.rs +++ b/crates/server/src/handle.rs @@ -214,7 +214,7 @@ async fn multiple_slashes_found() { // Temporary print to ensure test is working as intended. // println!("{}", res.into_response().status()); - assert_eq!(res.into_response().status(), 200); + assert_eq!(res.into_response().status(), 404); let request = Request::builder() .uri("http://httpbin.org///ip") @@ -225,6 +225,6 @@ async fn multiple_slashes_found() { // Temporary print to ensure test is working as intended. // println!("{}", res.into_response().status()); - assert_eq!(res.into_response().status(), 200); + assert_eq!(res.into_response().status(), 404); return (); }