From 1d25f4ab3fe1e7d6b4670fb498694e98bb784ef2 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Sun, 15 Dec 2024 14:21:36 +0100 Subject: [PATCH] controllers/version: Use custom deserializer to validate `version` field --- src/controllers/version.rs | 9 +++++++++ src/controllers/version/downloads.rs | 6 +----- src/controllers/version/metadata.rs | 14 +------------- src/controllers/version/yank.rs | 6 +----- src/tests/routes/crates/downloads.rs | 4 ++-- 5 files changed, 14 insertions(+), 25 deletions(-) diff --git a/src/controllers/version.rs b/src/controllers/version.rs index 26c257651ed..e6b3d06b231 100644 --- a/src/controllers/version.rs +++ b/src/controllers/version.rs @@ -4,6 +4,8 @@ pub mod yank; use axum::extract::{FromRequestParts, Path}; use diesel_async::AsyncPgConnection; +use serde::de::Error; +use serde::{Deserialize, Deserializer}; use utoipa::IntoParams; use crate::models::{Crate, Version}; @@ -17,6 +19,7 @@ pub struct CrateVersionPath { pub name: String, /// Version number #[param(example = "1.0.0")] + #[serde(deserialize_with = "deserialize_version")] pub version: String, } @@ -51,3 +54,9 @@ async fn version_and_crate( Ok((version, krate)) } + +fn deserialize_version<'de, D: Deserializer<'de>>(deserializer: D) -> Result { + let s = String::deserialize(deserializer)?; + let _ = semver::Version::parse(&s).map_err(Error::custom)?; + Ok(s) +} diff --git a/src/controllers/version/downloads.rs b/src/controllers/version/downloads.rs index 0d075c5fa8e..cc2fab59407 100644 --- a/src/controllers/version/downloads.rs +++ b/src/controllers/version/downloads.rs @@ -6,7 +6,7 @@ use super::CrateVersionPath; use crate::app::AppState; use crate::models::VersionDownload; use crate::schema::*; -use crate::util::errors::{version_not_found, AppResult}; +use crate::util::errors::AppResult; use crate::util::{redirect, RequestUtils}; use crate::views::EncodableVersionDownload; use axum::response::{IntoResponse, Response}; @@ -56,10 +56,6 @@ pub async fn get_version_downloads( path: CrateVersionPath, req: Parts, ) -> AppResult { - if semver::Version::parse(&path.version).is_err() { - return Err(version_not_found(&path.name, &path.version)); - } - let mut conn = app.db_read().await?; let version = path.load_version(&mut conn).await?; diff --git a/src/controllers/version/metadata.rs b/src/controllers/version/metadata.rs index f13b5df63a9..9cd541ec8a8 100644 --- a/src/controllers/version/metadata.rs +++ b/src/controllers/version/metadata.rs @@ -23,7 +23,7 @@ use crate::models::{ }; use crate::rate_limiter::LimitedAction; use crate::schema::versions; -use crate::util::errors::{bad_request, custom, version_not_found, AppResult}; +use crate::util::errors::{bad_request, custom, AppResult}; use crate::views::{EncodableDependency, EncodableVersion}; use crate::worker::jobs::{SyncToGitIndex, SyncToSparseIndex, UpdateDefaultVersion}; @@ -57,10 +57,6 @@ pub async fn get_version_dependencies( state: AppState, path: CrateVersionPath, ) -> AppResult { - if semver::Version::parse(&path.version).is_err() { - return Err(version_not_found(&path.name, &path.version)); - } - let mut conn = state.db_read().await?; let version = path.load_version(&mut conn).await?; @@ -105,10 +101,6 @@ pub async fn get_version_authors() -> ErasedJson { responses((status = 200, description = "Successful Response")), )] pub async fn find_version(state: AppState, path: CrateVersionPath) -> AppResult { - if semver::Version::parse(&path.version).is_err() { - return Err(version_not_found(&path.name, &path.version)); - } - let mut conn = state.db_read().await?; let (version, krate) = path.load_version_and_crate(&mut conn).await?; let published_by = version.published_by(&mut conn).await?; @@ -134,10 +126,6 @@ pub async fn update_version( req: Parts, Json(update_request): Json, ) -> AppResult { - if semver::Version::parse(&path.version).is_err() { - return Err(version_not_found(&path.name, &path.version)); - } - let mut conn = state.db_write().await?; let (mut version, krate) = path.load_version_and_crate(&mut conn).await?; validate_yank_update(&update_request.version, &version)?; diff --git a/src/controllers/version/yank.rs b/src/controllers/version/yank.rs index 666a544693c..4e69ef458bb 100644 --- a/src/controllers/version/yank.rs +++ b/src/controllers/version/yank.rs @@ -5,7 +5,7 @@ use super::CrateVersionPath; use crate::app::AppState; use crate::controllers::helpers::ok_true; use crate::rate_limiter::LimitedAction; -use crate::util::errors::{version_not_found, AppResult}; +use crate::util::errors::AppResult; use axum::response::Response; use http::request::Parts; @@ -61,10 +61,6 @@ async fn modify_yank( // FIXME: Should reject bad requests before authentication, but can't due to // lifetime issues with `req`. - if semver::Version::parse(&path.version).is_err() { - return Err(version_not_found(&path.name, &path.version)); - } - let mut conn = state.db_write().await?; let (mut version, krate) = path.load_version_and_crate(&mut conn).await?; let auth = authenticate(&req, &mut conn, &krate.name).await?; diff --git a/src/tests/routes/crates/downloads.rs b/src/tests/routes/crates/downloads.rs index 057f551c675..b7b9bdc6a2a 100644 --- a/src/tests/routes/crates/downloads.rs +++ b/src/tests/routes/crates/downloads.rs @@ -209,9 +209,9 @@ async fn test_version_downloads() { let response = anon .get::<()>("/api/v1/crates/foo/invalid-version/downloads") .await; - assert_eq!(response.status(), StatusCode::NOT_FOUND); + assert_eq!(response.status(), StatusCode::BAD_REQUEST); assert_snapshot!( response.text(), - @r#"{"errors":[{"detail":"crate `foo` does not have a version `invalid-version`"}]}"# + @r#"{"errors":[{"detail":"Invalid URL: unexpected character 'i' while parsing major version number"}]}"# ); }