Skip to content

Commit

Permalink
controllers/version: Use custom deserializer to validate version field
Browse files Browse the repository at this point in the history
  • Loading branch information
Turbo87 committed Dec 15, 2024
1 parent c6f6a3a commit 1d25f4a
Show file tree
Hide file tree
Showing 5 changed files with 14 additions and 25 deletions.
9 changes: 9 additions & 0 deletions src/controllers/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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,
}

Expand Down Expand Up @@ -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)
}
6 changes: 1 addition & 5 deletions src/controllers/version/downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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?;

Expand Down
14 changes: 1 addition & 13 deletions src/controllers/version/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -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?;
Expand All @@ -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)?;
Expand Down
6 changes: 1 addition & 5 deletions src/controllers/version/yank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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?;
Expand Down
4 changes: 2 additions & 2 deletions src/tests/routes/crates/downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"}]}"#
);
}

0 comments on commit 1d25f4a

Please sign in to comment.