From be9ea06e3e30ad2ece6f61e52d57f3cf2bd4746e Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Thu, 1 Aug 2024 15:56:06 +0300 Subject: [PATCH 1/3] Fix GET /download missing crate return 405 (Fix #15) --- src/serve.rs | 45 ++++++++++++++++++----- src/serve_frontend.rs | 26 +++++++------ tests/end-to-end.rs | 85 ++++++++++++++++++++++++++++++++++++------- 3 files changed, 120 insertions(+), 36 deletions(-) diff --git a/src/serve.rs b/src/serve.rs index 572f319..3f18297 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -2,7 +2,7 @@ use std::net::SocketAddr; use std::path::Path; use std::sync::Arc; -use anyhow::Context as _; +use anyhow::{anyhow, Context as _}; use anyhow::Error; use anyhow::Result; @@ -12,13 +12,15 @@ use serde::Serialize; use tokio::net::TcpListener; use tokio_stream::wrappers::TcpListenerStream; use tracing::info; - +use tracing_subscriber::fmt::format::FmtSpan; use warp::http::StatusCode; use warp::http::Uri; use warp::reject::Reject; -use warp::Filter; +use warp::{Filter, Reply, reply, trace}; +use warp::fs::File; use warp::Rejection; - +use warp::reply::Response; +use warp::trace::Info; use crate::index::handle_git; use crate::index::Index; use crate::publish::crate_file_name; @@ -142,11 +144,17 @@ pub async fn serve(root: &Path, binding: impl Into, server_addr: body, query, ) - .await, + .await, ) } }, ); + + + // Custom rejection handler that maps rejections into responses. + async fn reply_on_not_found(err: Rejection) -> Result {} + + // Handle sparse index requests at /index/ // let sparse_index = warp::path("index").and(warp::fs::dir(index_folder.clone())); @@ -154,15 +162,32 @@ pub async fn serve(root: &Path, binding: impl Into, server_addr: // downloading the .crate files, to which we redirect from the // download handler below. let crates = warp::path("crates") - .and(warp::fs::dir(crates_folder.to_path_buf())) + .and(warp::fs::dir(crates_folder.to_path_buf()) + // warp::fs::dir uses `and_then` so if file is missing it will try to find another route that match + // until returning Method not allowed... + // so we fix that by returning not found explicitly + .recover( + |err: Rejection| async { + if err.is_not_found() { + return Ok(reply::with_status(reply::json(&RegistryError { + detail: "Not found".to_string(), + }), StatusCode::NOT_FOUND)); + } + + return Err(err); + }) + ) + .with(warp::trace::request()); - let download = warp::get() - .and(warp::path("api")) + let download = warp::path("api") .and(warp::path("v1")) .and(warp::path("crates")) .and(warp::path::param()) .and(warp::path::param()) .and(warp::path("download")) + .and( + warp::get().or(warp::head()).unify() + ) .map(move |name: String, version: String| { let crate_path = crate_path(&name).join(crate_file_name(&name, &version)); let path = format!( @@ -180,11 +205,11 @@ pub async fn serve(root: &Path, binding: impl Into, server_addr: path.parse::().map(warp::redirect).unwrap() }) .with(warp::trace::request()); - let publish = warp::put() - .and(warp::path("api")) + let publish = warp::path("api") .and(warp::path("v1")) .and(warp::path("crates")) .and(warp::path("new")) + .and(warp::put()) .and(warp::path::end()) .and(warp::body::bytes()) // We cap total body size to 20 MiB to have some upper bound. At the diff --git a/src/serve_frontend.rs b/src/serve_frontend.rs index fd198b1..c3b6e1e 100644 --- a/src/serve_frontend.rs +++ b/src/serve_frontend.rs @@ -100,9 +100,9 @@ fn frontend_api( root: &Path, ) -> impl warp::Filter + Clone { let path_for_platforms = root.to_path_buf(); - let available_platforms = warp::get() - .and(warp::path("api")) + let available_platforms = warp::path("api") .and(warp::path("available-platforms")) + .and(warp::get()) .and_then(move || { let path_for_api = path_for_platforms.clone(); async move { @@ -114,9 +114,9 @@ fn frontend_api( }); let path_for_versions = root.to_path_buf(); - let versions_for_channel = warp::get() - .and(warp::path("api")) + let versions_for_channel = warp::path("api") .and(warp::path("versions")) + .and(warp::get()) .and_then(move || { let path_for_version = path_for_versions.clone(); async move { @@ -126,9 +126,9 @@ fn frontend_api( } }); let path_for_loading = root.to_path_buf(); - let load_pack_file = warp::put() - .and(warp::path("api")) + let load_pack_file = warp::path("api") .and(warp::path("load-pack-file")) + .and(warp::put()) .and(warp::body::bytes()) .and(warp::header::optional::("Content-Type")) .and_then(move |data: Bytes, content_type: Option| { @@ -163,12 +163,14 @@ fn frontend_api( pub fn serve_frontend( root: &Path, ) -> impl warp::Filter + Clone { - let home_page = warp::get().and(warp::path::end()).and_then(|| async { - FRONTEND - .get_file("index.html") - .ok_or_else(warp::reject::not_found) - .map(|f| warp::reply::html(f.contents())) - }); + let home_page = warp::path::end() + .and(warp::get()) + .and_then(|| async { + FRONTEND + .get_file("index.html") + .ok_or_else(warp::reject::not_found) + .map(|f| warp::reply::html(f.contents())) + }); let static_files = warp::get() .and(warp::path::tail()) diff --git a/tests/end-to-end.rs b/tests/end-to-end.rs index cc22d2f..dec4ef2 100644 --- a/tests/end-to-end.rs +++ b/tests/end-to-end.rs @@ -10,10 +10,9 @@ use anyhow::anyhow; use anyhow::bail; use anyhow::Context as _; use anyhow::Result; - -use reqwest::Url; +use reqwest::{Method, StatusCode, Url}; +use reqwest::header::USER_AGENT; use tempfile::tempdir; - use tokio::net::TcpListener; use tokio::spawn; use tokio::task::JoinHandle; @@ -40,6 +39,20 @@ async fn get_listener_in_available_port() -> TcpListener { panic!("No port available"); } +async fn send_download_request(addr: SocketAddr, method: Method, pkg_name: &str, ver: &str)-> StatusCode { + let client = reqwest::Client::new(); + + let base_url = format!("http://{}/api/v1/crates", addr); + + client + .request(method, format!("{}/{}/{}/download", base_url, pkg_name, ver)) + .header(USER_AGENT, "cargo-upload") + .send() + .await + .expect("Failed to send request") + .status() +} + /// Append data to a file. fn append(file: &Path, data: B) -> Result<()> where @@ -147,17 +160,17 @@ where async fn serve_registry() -> (JoinHandle<()>, PathBuf, SocketAddr) { let root = tempdir().unwrap(); let path = root.path(); - let listener = get_listener_in_available_port().await; - let addr = listener.local_addr().unwrap(); + let listener = get_listener_in_available_port().await; + let addr = listener.local_addr().unwrap(); - let server = move || { - let path = path.to_owned(); - let addr = addr.clone(); - async move { serve(&path, listener, addr).await.unwrap() } - }; - let handle = spawn(server()); + let server = move || { + let path = path.to_owned(); + let addr = addr.clone(); + async move { serve(&path, listener, addr).await.unwrap() } + }; + let handle = spawn(server()); - (handle, path.to_owned(), addr) + (handle, path.to_owned(), addr) } /// Check that we can publish a crate. @@ -181,8 +194,52 @@ async fn publish() { my_lib.join("Cargo.toml").to_str().unwrap(), ], ) - .await - .unwrap(); + .await + .unwrap(); +} + +#[tokio::test] +async fn publish_and_consume_download_endpoint() { + let (_handle, _reg_root, addr) = serve_registry().await; + + let src_root = tempdir().unwrap(); + let src_root = src_root.path(); + let home = setup_cargo_home(src_root, Locator::Socket(addr)).unwrap(); + + let my_lib = src_root.join("my-lib"); + cargo_init(&home, ["--lib", my_lib.to_str().unwrap()]) + .await + .unwrap(); + + cargo_publish( + &home, + [ + "--manifest-path", + my_lib.join("Cargo.toml").to_str().unwrap(), + ], + ) + .await + .unwrap(); + + // "/crates/my/-l/my-lib-0.1.0.crate" + let existing_crate_and_version_status = send_download_request(addr, Method::GET, "my-lib", "0.1.0").await; + assert_eq!(existing_crate_and_version_status, 200); + + let existing_crate_and_version_status = send_download_request(addr, Method::HEAD, "my-lib", "0.1.0").await; + assert_eq!(existing_crate_and_version_status, 200); + + let existing_crate_and_missing_version_status = send_download_request(addr, Method::GET, "my-lib", "99.99.99").await; + assert_eq!(existing_crate_and_missing_version_status, 404); + + let existing_crate_and_missing_version_status = send_download_request(addr, Method::HEAD, "my-lib", "99.99.99").await; + assert_eq!(existing_crate_and_missing_version_status, 404); + + let missing_crate_and_status = send_download_request(addr, Method::GET, "ba93ba78-f47a-4a37-b25b-1c713e5d11f8", "99.99.99").await; + assert_eq!(missing_crate_and_status, 404); + + let missing_crate_and_status = send_download_request(addr, Method::HEAD, "ba93ba78-f47a-4a37-b25b-1c713e5d11f8", "99.99.99").await; + assert_eq!(missing_crate_and_status, 404); + } async fn test_publish_and_consume(registry_locator: Locator) { From e6f03ef5421db5f7c8cc2b89fefb75f3393a8b1f Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 4 Aug 2024 16:16:57 +0300 Subject: [PATCH 2/3] keep only relevant changes --- src/serve.rs | 10 +--------- tests/end-to-end.rs | 28 +++++++++------------------- 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/src/serve.rs b/src/serve.rs index 3f18297..29ba5fc 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -149,12 +149,6 @@ pub async fn serve(root: &Path, binding: impl Into, server_addr: } }, ); - - - // Custom rejection handler that maps rejections into responses. - async fn reply_on_not_found(err: Rejection) -> Result {} - - // Handle sparse index requests at /index/ // let sparse_index = warp::path("index").and(warp::fs::dir(index_folder.clone())); @@ -185,9 +179,7 @@ pub async fn serve(root: &Path, binding: impl Into, server_addr: .and(warp::path::param()) .and(warp::path::param()) .and(warp::path("download")) - .and( - warp::get().or(warp::head()).unify() - ) + .and(warp::get()) .map(move |name: String, version: String| { let crate_path = crate_path(&name).join(crate_file_name(&name, &version)); let path = format!( diff --git a/tests/end-to-end.rs b/tests/end-to-end.rs index dec4ef2..daa93c8 100644 --- a/tests/end-to-end.rs +++ b/tests/end-to-end.rs @@ -160,17 +160,17 @@ where async fn serve_registry() -> (JoinHandle<()>, PathBuf, SocketAddr) { let root = tempdir().unwrap(); let path = root.path(); - let listener = get_listener_in_available_port().await; - let addr = listener.local_addr().unwrap(); + let listener = get_listener_in_available_port().await; + let addr = listener.local_addr().unwrap(); - let server = move || { - let path = path.to_owned(); - let addr = addr.clone(); - async move { serve(&path, listener, addr).await.unwrap() } - }; - let handle = spawn(server()); + let server = move || { + let path = path.to_owned(); + let addr = addr.clone(); + async move { serve(&path, listener, addr).await.unwrap() } + }; + let handle = spawn(server()); - (handle, path.to_owned(), addr) + (handle, path.to_owned(), addr) } /// Check that we can publish a crate. @@ -225,21 +225,11 @@ async fn publish_and_consume_download_endpoint() { let existing_crate_and_version_status = send_download_request(addr, Method::GET, "my-lib", "0.1.0").await; assert_eq!(existing_crate_and_version_status, 200); - let existing_crate_and_version_status = send_download_request(addr, Method::HEAD, "my-lib", "0.1.0").await; - assert_eq!(existing_crate_and_version_status, 200); - let existing_crate_and_missing_version_status = send_download_request(addr, Method::GET, "my-lib", "99.99.99").await; assert_eq!(existing_crate_and_missing_version_status, 404); - let existing_crate_and_missing_version_status = send_download_request(addr, Method::HEAD, "my-lib", "99.99.99").await; - assert_eq!(existing_crate_and_missing_version_status, 404); - let missing_crate_and_status = send_download_request(addr, Method::GET, "ba93ba78-f47a-4a37-b25b-1c713e5d11f8", "99.99.99").await; assert_eq!(missing_crate_and_status, 404); - - let missing_crate_and_status = send_download_request(addr, Method::HEAD, "ba93ba78-f47a-4a37-b25b-1c713e5d11f8", "99.99.99").await; - assert_eq!(missing_crate_and_status, 404); - } async fn test_publish_and_consume(registry_locator: Locator) { From bc4146ae996ae7b3d47bf64757fc8eb9915aedad Mon Sep 17 00:00:00 2001 From: Raz Luvaton <16746759+rluvaton@users.noreply.github.com> Date: Sun, 4 Aug 2024 16:23:17 +0300 Subject: [PATCH 3/3] keep only relevant --- src/serve.rs | 12 ++++++------ src/serve_frontend.rs | 26 ++++++++++++-------------- 2 files changed, 18 insertions(+), 20 deletions(-) diff --git a/src/serve.rs b/src/serve.rs index 29ba5fc..5e4c0fb 100644 --- a/src/serve.rs +++ b/src/serve.rs @@ -2,7 +2,7 @@ use std::net::SocketAddr; use std::path::Path; use std::sync::Arc; -use anyhow::{anyhow, Context as _}; +use anyhow::Context as _; use anyhow::Error; use anyhow::Result; @@ -12,7 +12,7 @@ use serde::Serialize; use tokio::net::TcpListener; use tokio_stream::wrappers::TcpListenerStream; use tracing::info; -use tracing_subscriber::fmt::format::FmtSpan; + use warp::http::StatusCode; use warp::http::Uri; use warp::reject::Reject; @@ -173,13 +173,13 @@ pub async fn serve(root: &Path, binding: impl Into, server_addr: ) .with(warp::trace::request()); - let download = warp::path("api") + let download = warp::get() + .and(warp::path("api")) .and(warp::path("v1")) .and(warp::path("crates")) .and(warp::path::param()) .and(warp::path::param()) .and(warp::path("download")) - .and(warp::get()) .map(move |name: String, version: String| { let crate_path = crate_path(&name).join(crate_file_name(&name, &version)); let path = format!( @@ -197,11 +197,11 @@ pub async fn serve(root: &Path, binding: impl Into, server_addr: path.parse::().map(warp::redirect).unwrap() }) .with(warp::trace::request()); - let publish = warp::path("api") + let publish = warp::put() + .and(warp::path("api")) .and(warp::path("v1")) .and(warp::path("crates")) .and(warp::path("new")) - .and(warp::put()) .and(warp::path::end()) .and(warp::body::bytes()) // We cap total body size to 20 MiB to have some upper bound. At the diff --git a/src/serve_frontend.rs b/src/serve_frontend.rs index c3b6e1e..fd198b1 100644 --- a/src/serve_frontend.rs +++ b/src/serve_frontend.rs @@ -100,9 +100,9 @@ fn frontend_api( root: &Path, ) -> impl warp::Filter + Clone { let path_for_platforms = root.to_path_buf(); - let available_platforms = warp::path("api") + let available_platforms = warp::get() + .and(warp::path("api")) .and(warp::path("available-platforms")) - .and(warp::get()) .and_then(move || { let path_for_api = path_for_platforms.clone(); async move { @@ -114,9 +114,9 @@ fn frontend_api( }); let path_for_versions = root.to_path_buf(); - let versions_for_channel = warp::path("api") + let versions_for_channel = warp::get() + .and(warp::path("api")) .and(warp::path("versions")) - .and(warp::get()) .and_then(move || { let path_for_version = path_for_versions.clone(); async move { @@ -126,9 +126,9 @@ fn frontend_api( } }); let path_for_loading = root.to_path_buf(); - let load_pack_file = warp::path("api") + let load_pack_file = warp::put() + .and(warp::path("api")) .and(warp::path("load-pack-file")) - .and(warp::put()) .and(warp::body::bytes()) .and(warp::header::optional::("Content-Type")) .and_then(move |data: Bytes, content_type: Option| { @@ -163,14 +163,12 @@ fn frontend_api( pub fn serve_frontend( root: &Path, ) -> impl warp::Filter + Clone { - let home_page = warp::path::end() - .and(warp::get()) - .and_then(|| async { - FRONTEND - .get_file("index.html") - .ok_or_else(warp::reject::not_found) - .map(|f| warp::reply::html(f.contents())) - }); + let home_page = warp::get().and(warp::path::end()).and_then(|| async { + FRONTEND + .get_file("index.html") + .ok_or_else(warp::reject::not_found) + .map(|f| warp::reply::html(f.contents())) + }); let static_files = warp::get() .and(warp::path::tail())