Merge pull request #7194 from Turbo87/desc-license-from-manifest

publish: Use `description`, `license` and `license_file` fields from embedded `Cargo.toml` file

Author: Turbo87

src/controllers/krate/publish.rs

@@ -51,25 +51,6 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
     request_log.add("crate_name", &*metadata.name);
     request_log.add("crate_version", &*metadata.vers);
 
-    // Make sure required fields are provided
-    fn empty(s: Option<&String>) -> bool {
-        s.map_or(true, String::is_empty)
-    }
-
-    // It can have up to three elements per below conditions.
-    let mut missing = Vec::with_capacity(3);
-
-    if empty(metadata.description.as_ref()) {
-        missing.push("description");
-    }
-    if empty(metadata.license.as_ref()) && empty(metadata.license_file.as_ref()) {
-        missing.push("license");
-    }
-    if !missing.is_empty() {
-        let message = missing_metadata_error_message(&missing);
-        return Err(cargo_err(&message));
-    }
-
     conduit_compat(move || {
         let conn = &mut *app.db_write()?;
 
@@ -110,6 +91,51 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
         app.rate_limiter
             .check_rate_limit(user.id, rate_limit_action, conn)?;
 
+        let content_length = tarball_bytes.len() as u64;
+
+        let maximums = Maximums::new(
+            existing_crate.as_ref().and_then(|c| c.max_upload_size),
+            app.config.max_upload_size,
+            app.config.max_unpack_size,
+        );
+
+        if content_length > maximums.max_upload_size {
+            return Err(cargo_err(&format_args!(
+                "max upload size is: {}",
+                maximums.max_upload_size
+            )));
+        }
+
+        let pkg_name = format!("{}-{}", &*metadata.name, &*metadata.vers);
+        let tarball_info = process_tarball(&pkg_name, &*tarball_bytes, maximums.max_unpack_size)?;
+
+        // `unwrap()` is safe here since `process_tarball()` validates that
+        // we only accept manifests with a `package` section and without
+        // inheritance.
+        let package = tarball_info.manifest.package.unwrap();
+
+        let description = package.description.map(|it| it.as_local().unwrap());
+        let license = package.license.map(|it| it.as_local().unwrap());
+        let license_file = package.license_file.map(|it| it.as_local().unwrap());
+
+        // Make sure required fields are provided
+        fn empty(s: Option<&String>) -> bool {
+            s.map_or(true, String::is_empty)
+        }
+
+        // It can have up to three elements per below conditions.
+        let mut missing = Vec::with_capacity(3);
+        if empty(description.as_ref()) {
+            missing.push("description");
+        }
+        if empty(license.as_ref()) && empty(license_file.as_ref()) {
+            missing.push("license");
+        }
+        if !missing.is_empty() {
+            let message = missing_metadata_error_message(&missing);
+            return Err(cargo_err(&message));
+        }
+
         // Create a transaction on the database, if there are no errors,
         // commit the transactions to record a new or updated crate.
         conn.transaction(|conn| {
@@ -136,15 +162,15 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
             // Persist the new crate, if it doesn't already exist
             let persist = NewCrate {
                 name: &name,
-                description: metadata.description.as_deref(),
+                description: description.as_deref(),
                 homepage: metadata.homepage.as_deref(),
                 documentation: metadata.documentation.as_deref(),
                 readme: metadata.readme.as_deref(),
                 repository: repo.as_deref(),
                 max_upload_size: None,
             };
 
-            let license_file = metadata.license_file.as_deref();
+            let license_file = license_file.as_deref();
 
             validate_url(persist.homepage, "homepage")?;
             validate_url(persist.documentation, "documentation")?;
@@ -182,40 +208,17 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
                 }
             }
 
-            let content_length = tarball_bytes.len() as u64;
-
-            let maximums = Maximums::new(
-                krate.max_upload_size,
-                app.config.max_upload_size,
-                app.config.max_unpack_size,
-            );
-
-            if content_length > maximums.max_upload_size {
-                return Err(cargo_err(&format_args!(
-                    "max upload size is: {}",
-                    maximums.max_upload_size
-                )));
-            }
-
             // Read tarball from request
             let hex_cksum: String = Sha256::digest(&tarball_bytes).encode_hex();
 
-            let pkg_name = format!("{}-{}", krate.name, vers);
-            let tarball_info =
-                process_tarball(&pkg_name, &*tarball_bytes, maximums.max_unpack_size)?;
-
-            // `unwrap()` is safe here since `process_tarball()` validates that
-            // we only accept manifests with a `package` section and without
-            // inheritance.
-            let package = tarball_info.manifest.package.unwrap();
             let rust_version = package.rust_version.map(|rv| rv.as_local().unwrap());
 
             // Persist the new version of this crate
             let version = NewVersion::new(
                 krate.id,
                 vers,
                 &features,
-                metadata.license,
+                license,
                 license_file,
                 // Downcast is okay because the file length must be less than the max upload size
                 // to get here, and max upload sizes are way less than i32 max

src/tests/builders/publish.rs

@@ -142,7 +142,6 @@ impl PublishBuilder {
             vers: u::EncodableCrateVersion(self.version.clone()),
             features: self.features.clone(),
             deps: self.deps.clone(),
-            description: self.desc.clone(),
             homepage: None,
             documentation: self.doc_url.clone(),
             readme: self.readme,
@@ -161,8 +160,6 @@ impl PublishBuilder {
                     .map(u::EncodableCategory)
                     .collect(),
             ),
-            license: self.license.clone(),
-            license_file: self.license_file.clone(),
             repository: None,
             links: None,
         };

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid length 6, expected at most 5 categories per crate at line 1 column 219"
+      "detail": "invalid upload request: invalid length 6, expected at most 5 categories per crate at line 1 column 191"
     }
   ]
 }

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid value: string \"?@?%\", expected a valid keyword specifier at line 1 column 178"
+      "detail": "invalid upload request: invalid value: string \"?@?%\", expected a valid keyword specifier at line 1 column 150"
     }
   ]
 }

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid value: string \"áccênts\", expected a valid keyword specifier at line 1 column 183"
+      "detail": "invalid upload request: invalid value: string \"áccênts\", expected a valid keyword specifier at line 1 column 155"
     }
   ]
 }

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid length 29, expected a keyword with less than 20 characters at line 1 column 203"
+      "detail": "invalid upload request: invalid length 29, expected a keyword with less than 20 characters at line 1 column 175"
     }
   ]
 }

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid upload request: invalid length 6, expected at most 5 keywords per crate at line 1 column 203"
+      "detail": "invalid upload request: invalid length 6, expected at most 5 keywords per crate at line 1 column 175"
     }
   ]
 }

src/views/krate_publish.rs

@@ -18,7 +18,6 @@ pub struct PublishMetadata {
     pub vers: EncodableCrateVersion,
     pub deps: Vec<EncodableCrateDependency>,
     pub features: BTreeMap<EncodableFeatureName, Vec<EncodableFeature>>,
-    pub description: Option<String>,
     pub homepage: Option<String>,
     pub documentation: Option<String>,
     pub readme: Option<String>,
@@ -27,8 +26,6 @@ pub struct PublishMetadata {
     pub keywords: EncodableKeywordList,
     #[serde(default)]
     pub categories: EncodableCategoryList,
-    pub license: Option<String>,
-    pub license_file: Option<String>,
     pub repository: Option<String>,
     #[serde(default)]
     pub links: Option<String>,