Merge pull request #7195 from Turbo87/license-validation

models/version: Move license validation into publish endpoint

Author: Turbo87

src/controllers/krate/publish.rs

@@ -19,6 +19,7 @@ use crate::models::{
 
 use crate::middleware::log_request::RequestLogExt;
 use crate::models::token::EndpointScope;
+use crate::models::version::validate_license_expr;
 use crate::rate_limiter::LimitedAction;
 use crate::schema::*;
 use crate::sql::canon_crate_name;
@@ -115,7 +116,7 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
         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 mut 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
@@ -136,6 +137,15 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
             return Err(cargo_err(&message));
         }
 
+        if let Some(ref license) = license {
+            validate_license_expr(license)?;
+        } else if license_file.is_some() {
+            // If no license is given, but a license file is given, flag this
+            // crate as having a nonstandard license. Note that we don't
+            // actually do anything else with license_file currently.
+            license = Some(String::from("non-standard"));
+        }
+
         // Create a transaction on the database, if there are no errors,
         // commit the transactions to record a new or updated crate.
         conn.transaction(|conn| {
@@ -170,8 +180,6 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
                 max_upload_size: None,
             };
 
-            let license_file = license_file.as_deref();
-
             validate_url(persist.homepage, "homepage")?;
             validate_url(persist.documentation, "documentation")?;
             validate_url(persist.repository, "repository")?;
@@ -219,7 +227,6 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
                 vers,
                 &features,
                 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
                 content_length as i32,

src/downloads_counter.rs

@@ -455,7 +455,6 @@ mod tests {
                 &Version::parse(&format!("{}.0.0", self.next_version)).unwrap(),
                 &BTreeMap::new(),
                 None,
-                None,
                 0,
                 self.user.id,
                 "0000000000000000000000000000000000000000000000000000000000000000".to_string(),

src/models.rs

@@ -30,4 +30,4 @@ mod rights;
 mod team;
 pub mod token;
 pub mod user;
-mod version;
+pub mod version;

src/models/version.rs

@@ -139,7 +139,6 @@ impl NewVersion {
         num: &semver::Version,
         features: &BTreeMap<String, Vec<String>>,
         license: Option<String>,
-        license_file: Option<&str>,
         crate_size: i32,
         published_by: i32,
         checksum: String,
@@ -148,7 +147,7 @@ impl NewVersion {
     ) -> AppResult<Self> {
         let features = serde_json::to_value(features)?;
 
-        let mut new_version = NewVersion {
+        Ok(NewVersion {
             crate_id,
             num: num.to_string(),
             features,
@@ -158,11 +157,7 @@ impl NewVersion {
             checksum,
             links,
             rust_version,
-        };
-
-        new_version.validate_license(license_file)?;
-
-        Ok(new_version)
+        })
     }
 
     pub fn save(&self, conn: &mut PgConnection, published_by_email: &str) -> AppResult<Version> {
@@ -195,21 +190,9 @@ impl NewVersion {
             Ok(version)
         })
     }
-
-    fn validate_license(&mut self, license_file: Option<&str>) -> AppResult<()> {
-        if let Some(ref license) = self.license {
-            validate_license_expr(license)?;
-        } else if license_file.is_some() {
-            // If no license is given, but a license file is given, flag this
-            // crate as having a nonstandard license. Note that we don't
-            // actually do anything else with license_file currently.
-            self.license = Some(String::from("non-standard"));
-        }
-        Ok(())
-    }
 }
 
-fn validate_license_expr(s: &str) -> AppResult<()> {
+pub fn validate_license_expr(s: &str) -> AppResult<()> {
     pub const PARSE_MODE: spdx::ParseMode = spdx::ParseMode {
         allow_lower_case_operators: false,
         allow_slash_as_or_operator: true,

src/tests/builders/version.rs

@@ -14,7 +14,6 @@ pub struct VersionBuilder<'a> {
     dependencies: Vec<(i32, Option<&'static str>)>,
     features: BTreeMap<String, Vec<String>>,
     license: Option<&'a str>,
-    license_file: Option<&'a str>,
     num: semver::Version,
     size: i32,
     yanked: bool,
@@ -40,7 +39,6 @@ impl<'a> VersionBuilder<'a> {
             dependencies: Vec::new(),
             features: BTreeMap::new(),
             license: None,
-            license_file: None,
             num,
             size: 0,
             yanked: false,
@@ -106,7 +104,6 @@ impl<'a> VersionBuilder<'a> {
             &self.num,
             &self.features,
             license,
-            self.license_file,
             self.size,
             published_by,
             self.checksum,

src/worker/update_downloads.rs

@@ -105,7 +105,6 @@ mod test {
             &semver::Version::parse("1.0.0").unwrap(),
             &BTreeMap::new(),
             None,
-            None,
             0,
             user_id,
             "0000000000000000000000000000000000000000000000000000000000000000".to_string(),