publish: Move crate name and version validation to publish controller (#7237)

Author: Turbo87

src/controllers/krate/publish.rs

@@ -55,9 +55,31 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
     let metadata: PublishMetadata = serde_json::from_slice(&json_bytes)
         .map_err(|e| cargo_err(&format_args!("invalid upload request: {e}")))?;
 
+    if !Crate::valid_name(&metadata.name) {
+        return Err(cargo_err(&format_args!(
+            "\"{}\" is an invalid crate name (crate names must start with a \
+            letter, contain only letters, numbers, hyphens, or underscores and \
+            have at most {MAX_NAME_LENGTH} characters)",
+            metadata.name
+        )));
+    }
+
+    let version = match semver::Version::parse(&metadata.vers) {
+        Ok(parsed) => parsed,
+        Err(_) => {
+            return Err(cargo_err(&format_args!(
+                "\"{}\" is an invalid semver version",
+                metadata.vers
+            )))
+        }
+    };
+
+    // Convert the version back to a string to deal with any inconsistencies
+    let version_string = version.to_string();
+
     let request_log = req.request_log();
     request_log.add("crate_name", &*metadata.name);
-    request_log.add("crate_version", &*metadata.vers);
+    request_log.add("crate_version", &version_string);
 
     conduit_compat(move || {
         let conn = &mut *app.db_write()?;
@@ -114,7 +136,7 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
             )));
         }
 
-        let pkg_name = format!("{}-{}", &*metadata.name, &*metadata.vers);
+        let pkg_name = format!("{}-{}", &*metadata.name, &version_string);
         let tarball_info = process_tarball(&pkg_name, &*tarball_bytes, maximums.max_unpack_size)?;
 
         // `unwrap()` is safe here since `process_tarball()` validates that
@@ -208,7 +230,6 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
         // commit the transactions to record a new or updated crate.
         conn.transaction(|conn| {
             let name = metadata.name;
-            let vers = &*metadata.vers;
             let keywords = keywords.iter().map(|s| s.as_str()).collect::<Vec<_>>();
             let categories = categories.iter().map(|s| s.as_str()).collect::<Vec<_>>();
 
@@ -263,7 +284,7 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
             // Persist the new version of this crate
             let version = NewVersion::new(
                 krate.id,
-                vers,
+                &version,
                 &features,
                 license,
                 // Downcast is okay because the file length must be less than the max upload size
@@ -321,7 +342,7 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
             Handle::current()
                 .block_on(app.storage.upload_crate_file(
                     &krate.name,
-                    &vers.to_string(),
+                    &version_string,
                     tarball_bytes,
                 ))
                 .map_err(|e| internal(format!("failed to upload crate: {e}")))?;

src/tests/builders/publish.rs

@@ -134,8 +134,8 @@ impl PublishBuilder {
 
     pub fn build(self) -> (String, Vec<u8>) {
         let metadata = u::PublishMetadata {
-            name: u::EncodableCrateName(self.krate_name.clone()),
-            vers: u::EncodableCrateVersion(self.version.clone()),
+            name: self.krate_name.clone(),
+            vers: self.version.to_string(),
             deps: self.deps.clone(),
             readme: self.readme,
             readme_file: None,

src/tests/krate/publish/snapshots/all__krate__publish__validation__invalid_names-2.snap

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid value: string \"foo bar\", expected a valid crate name to start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters at line 1 column 17"
+      "detail": "\"foo bar\" is an invalid crate name (crate names must start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters)"
     }
   ]
 }

src/tests/krate/publish/snapshots/all__krate__publish__validation__invalid_names-3.snap

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid value: string \"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\", expected a valid crate name to start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters at line 1 column 75"
+      "detail": "\"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa\" is an invalid crate name (crate names must start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters)"
     }
   ]
 }

src/tests/krate/publish/snapshots/all__krate__publish__validation__invalid_names-4.snap

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid value: string \"snow☃\", expected a valid crate name to start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters at line 1 column 17"
+      "detail": "\"snow☃\" is an invalid crate name (crate names must start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters)"
     }
   ]
 }

src/tests/krate/publish/snapshots/all__krate__publish__validation__invalid_names-5.snap

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid value: string \"áccênts\", expected a valid crate name to start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters at line 1 column 19"
+      "detail": "\"áccênts\" is an invalid crate name (crate names must start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters)"
     }
   ]
 }

src/tests/krate/publish/snapshots/all__krate__publish__validation__invalid_names.snap

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid value: string \"\", expected a valid crate name to start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters at line 1 column 10"
+      "detail": "\"\" is an invalid crate name (crate names must start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters)"
     }
   ]
 }

src/tests/krate/publish/snapshots/all__krate__publish__validation__invalid_version.snap

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid value: string \"broken\", expected a valid semver at line 1 column 29"
+      "detail": "\"broken\" is an invalid semver version"
     }
   ]
 }

src/views/krate_publish.rs

@@ -3,17 +3,14 @@
 //! to and from structs. The serializing is only utilised in
 //! integration tests.
 
-use serde::{de, Deserialize, Deserializer, Serialize};
+use serde::{Deserialize, Serialize};
 
-use crate::models::krate::MAX_NAME_LENGTH;
-
-use crate::models::Crate;
 use crate::models::DependencyKind;
 
 #[derive(Deserialize, Serialize, Debug)]
 pub struct PublishMetadata {
-    pub name: EncodableCrateName,
-    pub vers: EncodableCrateVersion,
+    pub name: String,
+    pub vers: String,
     pub deps: Vec<EncodableCrateDependency>,
     pub readme: Option<String>,
     pub readme_file: Option<String>,
@@ -31,39 +28,3 @@ pub struct EncodableCrateDependency {
     pub explicit_name_in_toml: Option<String>,
     pub registry: Option<String>,
 }
-
-#[derive(PartialEq, Eq, Hash, Serialize, Clone, Debug, Deref)]
-pub struct EncodableCrateName(pub String);
-
-impl<'de> Deserialize<'de> for EncodableCrateName {
-    fn deserialize<D: Deserializer<'de>>(d: D) -> Result<EncodableCrateName, D::Error> {
-        let s = String::deserialize(d)?;
-        if !Crate::valid_name(&s) {
-            let value = de::Unexpected::Str(&s);
-            let expected = format!(
-                "a valid crate name to start with a letter, contain only letters, \
-                 numbers, hyphens, or underscores and have at most {MAX_NAME_LENGTH} characters"
-            );
-            Err(de::Error::invalid_value(value, &expected.as_ref()))
-        } else {
-            Ok(EncodableCrateName(s))
-        }
-    }
-}
-
-#[derive(Serialize, Debug, Deref)]
-pub struct EncodableCrateVersion(pub semver::Version);
-
-impl<'de> Deserialize<'de> for EncodableCrateVersion {
-    fn deserialize<D: Deserializer<'de>>(d: D) -> Result<EncodableCrateVersion, D::Error> {
-        let s = String::deserialize(d)?;
-        match semver::Version::parse(&s) {
-            Ok(v) => Ok(EncodableCrateVersion(v)),
-            Err(..) => {
-                let value = de::Unexpected::Str(&s);
-                let expected = "a valid semver";
-                Err(de::Error::invalid_value(value, &expected))
-            }
-        }
-    }
-}