controllers/version: Use custom deserializer to validate version field

Author: Turbo87

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<String, D::Error> {
+    let s = String::deserialize(deserializer)?;
+    let _ = semver::Version::parse(&s).map_err(Error::custom)?;
+    Ok(s)
+}

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<ErasedJson> {
-    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?;
 

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<ErasedJson> {
-    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<ErasedJson> {
-    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<VersionUpdateRequest>,
 ) -> AppResult<ErasedJson> {
-    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)?;

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?;

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"}]}"#
     );
 }