Fix crate and dependency name validation

Signed-off-by: hi-rustin <[email protected]>

Author: 0xPoe

src/controllers/krate/publish.rs

@@ -52,7 +52,7 @@ 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}")))?;
 
-    Crate::validate_crate_name(&metadata.name).map_err(cargo_err)?;
+    Crate::validate_crate_name("crate", &metadata.name).map_err(cargo_err)?;
 
     let version = match semver::Version::parse(&metadata.vers) {
         Ok(parsed) => parsed,
@@ -586,7 +586,7 @@ fn convert_dependency(
 }
 
 pub fn validate_dependency(dep: &EncodableCrateDependency) -> AppResult<()> {
-    Crate::validate_crate_name(&dep.name).map_err(cargo_err)?;
+    Crate::validate_crate_name("dependency", &dep.name).map_err(cargo_err)?;
 
     for feature in &dep.features {
         Crate::validate_feature(feature).map_err(cargo_err)?;

src/models/krate.rs

@@ -192,35 +192,55 @@ impl Crate {
             })
     }
 
-    pub fn validate_crate_name(name: &str) -> Result<(), InvalidCrateName> {
+    // Validates the name is a valid crate name.
+    // This is also used for validating the name of dependencies.
+    // So the `for_what` parameter is used to indicate what the name is used for.
+    // It can be "crate" or "dependency".
+    pub fn validate_crate_name(for_what: &str, name: &str) -> Result<(), InvalidCrateName> {
         if name.chars().count() > MAX_NAME_LENGTH {
-            return Err(InvalidCrateName::TooLong(name.into()));
+            return Err(InvalidCrateName::TooLong {
+                what: for_what.into(),
+                name: name.into(),
+            });
         }
-        Crate::validate_create_ident(name)
+        Crate::validate_create_ident(for_what, name)
     }
 
     // Checks that the name is a valid crate name.
     // 1. The name must be non-empty.
     // 2. The first character must be an ASCII character.
     // 3. The remaining characters must be ASCII alphanumerics or `-` or `_`.
     // Note: This differs from `valid_dependency_name`, which allows `_` as the first character.
-    fn validate_create_ident(name: &str) -> Result<(), InvalidCrateName> {
+    fn validate_create_ident(for_what: &str, name: &str) -> Result<(), InvalidCrateName> {
         if name.is_empty() {
-            return Err(InvalidCrateName::Empty);
+            return Err(InvalidCrateName::Empty {
+                what: for_what.into(),
+            });
         }
         let mut chars = name.chars();
         if let Some(ch) = chars.next() {
             if ch.is_ascii_digit() {
-                return Err(InvalidCrateName::StartWithDigit(name.into()));
+                return Err(InvalidCrateName::StartWithDigit {
+                    what: for_what.into(),
+                    name: name.into(),
+                });
             }
             if !ch.is_ascii_alphabetic() {
-                return Err(InvalidCrateName::Start(ch, name.into()));
+                return Err(InvalidCrateName::Start {
+                    first_char: ch,
+                    what: for_what.into(),
+                    name: name.into(),
+                });
             }
         }
 
         for ch in chars {
             if !(ch.is_ascii_alphanumeric() || ch == '-' || ch == '_') {
-                return Err(InvalidCrateName::Char(ch, name.into()));
+                return Err(InvalidCrateName::Char {
+                    ch,
+                    what: for_what.into(),
+                    name: name.into(),
+                });
             }
         }
 
@@ -559,25 +579,33 @@ pub enum InvalidFeature {
 
 #[derive(Debug, Eq, PartialEq, thiserror::Error)]
 pub enum InvalidCrateName {
-    #[error("the crate name `{0}` is too long (max {MAX_NAME_LENGTH} characters)")]
-    TooLong(String),
-    #[error("crate name cannot be empty")]
-    Empty,
+    #[error("the {what} name `{name}` is too long (max {MAX_NAME_LENGTH} characters)")]
+    TooLong { what: String, name: String },
+    #[error("{what} name cannot be empty")]
+    Empty { what: String },
     #[error(
-        "the name `{0}` cannot be used as a crate name, \
+        "the name `{name}` cannot be used as a {what} name, \
         the name cannot start with a digit"
     )]
-    StartWithDigit(String),
+    StartWithDigit { what: String, name: String },
     #[error(
-        "invalid character `{0}` in crate name: `{1}`, \
+        "invalid character `{first_char}` in {what} name: `{name}`, \
         the first character must be an ASCII character"
     )]
-    Start(char, String),
+    Start {
+        first_char: char,
+        what: String,
+        name: String,
+    },
     #[error(
-        "invalid character `{0}` in crate name: `{1}`, \
+        "invalid character `{ch}` in {what} name: `{name}`, \
         characters must be an ASCII alphanumeric characters, `-`, or `_`"
     )]
-    Char(char, String),
+    Char {
+        ch: char,
+        what: String,
+        name: String,
+    },
 }
 
 #[derive(Debug, Eq, PartialEq, thiserror::Error)]
@@ -611,38 +639,68 @@ mod tests {
     fn validate_crate_name() {
         use super::{InvalidCrateName, MAX_NAME_LENGTH};
 
-        assert_ok!(Crate::validate_crate_name("foo"));
+        assert_ok!(Crate::validate_crate_name("crate", "foo"));
         assert_err_eq!(
-            Crate::validate_crate_name("京"),
-            InvalidCrateName::Start('京', "京".into())
+            Crate::validate_crate_name("crate", "京"),
+            InvalidCrateName::Start {
+                first_char: '京',
+                what: "crate".into(),
+                name: "京".into()
+            }
         );
-        assert_err_eq!(Crate::validate_crate_name(""), InvalidCrateName::Empty);
         assert_err_eq!(
-            Crate::validate_crate_name("💝"),
-            InvalidCrateName::Start('💝', "💝".into())
+            Crate::validate_crate_name("crate", ""),
+            InvalidCrateName::Empty {
+                what: "crate".into()
+            }
         );
-        assert_ok!(Crate::validate_crate_name("foo_underscore"));
-        assert_ok!(Crate::validate_crate_name("foo-dash"));
         assert_err_eq!(
-            Crate::validate_crate_name("foo+plus"),
-            InvalidCrateName::Char('+', "foo+plus".into())
+            Crate::validate_crate_name("crate", "💝"),
+            InvalidCrateName::Start {
+                first_char: '💝',
+                what: "crate".into(),
+                name: "💝".into()
+            }
         );
-        // Crate names cannot start with an underscore on crates.io.
+        assert_ok!(Crate::validate_crate_name("crate", "foo_underscore"));
+        assert_ok!(Crate::validate_crate_name("crate", "foo-dash"));
         assert_err_eq!(
-            Crate::validate_crate_name("_foo"),
-            InvalidCrateName::Start('_', "_foo".into())
+            Crate::validate_crate_name("crate", "foo+plus"),
+            InvalidCrateName::Char {
+                ch: '+',
+                what: "crate".into(),
+                name: "foo+plus".into()
+            }
         );
         assert_err_eq!(
-            Crate::validate_crate_name("-foo"),
-            InvalidCrateName::Start('-', "-foo".into())
+            Crate::validate_crate_name("crate", "_foo"),
+            InvalidCrateName::Start {
+                first_char: '_',
+                what: "crate".into(),
+                name: "_foo".into()
+            }
         );
         assert_err_eq!(
-            Crate::validate_crate_name("123"),
-            InvalidCrateName::StartWithDigit("123".into())
+            Crate::validate_crate_name("crate", "-foo"),
+            InvalidCrateName::Start {
+                first_char: '-',
+                what: "crate".into(),
+                name: "-foo".into()
+            }
         );
         assert_err_eq!(
-            Crate::validate_crate_name("o".repeat(MAX_NAME_LENGTH + 1).as_str()),
-            InvalidCrateName::TooLong("o".repeat(MAX_NAME_LENGTH + 1).as_str().into())
+            Crate::validate_crate_name("crate", "123"),
+            InvalidCrateName::StartWithDigit {
+                what: "crate".into(),
+                name: "123".into()
+            }
+        );
+        assert_err_eq!(
+            Crate::validate_crate_name("crate", "o".repeat(MAX_NAME_LENGTH + 1).as_str()),
+            InvalidCrateName::TooLong {
+                what: "crate".into(),
+                name: "o".repeat(MAX_NAME_LENGTH + 1).as_str().into()
+            }
         );
     }
 

src/models/token/scopes.rs

@@ -108,7 +108,7 @@ impl CrateScope {
         }
 
         let name_without_wildcard = pattern.strip_suffix('*').unwrap_or(pattern);
-        Crate::validate_crate_name(name_without_wildcard).is_ok()
+        Crate::validate_crate_name("crate", name_without_wildcard).is_ok()
     }
 
     pub fn matches(&self, crate_name: &str) -> bool {

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "invalid character `🦀` in crate name: `🦀`, the first character must be an ASCII character"
+      "detail": "invalid character `🦀` in dependency name: `🦀`, the first character must be an ASCII character"
     }
   ]
 }