Use the same crate name validation rule from Cargo

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

Author: 0xPoe

src/controllers/krate/publish.rs

@@ -16,7 +16,6 @@ use tokio::runtime::Handle;
 use url::Url;
 
 use crate::controllers::cargo_prelude::*;
-use crate::models::krate::MAX_NAME_LENGTH;
 use crate::models::{
     insert_version_owner_action, Category, Crate, DependencyKind, Keyword, NewCrate, NewVersion,
     Rights, VersionAction,
@@ -53,14 +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}")))?;
 
-    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
-        )));
-    }
+    Crate::valid_name(&metadata.name)?;
 
     let version = match semver::Version::parse(&metadata.vers) {
         Ok(parsed) => parsed,
@@ -582,14 +574,7 @@ fn convert_dependency(
 }
 
 pub fn validate_dependency(dep: &EncodableCrateDependency) -> AppResult<()> {
-    if !Crate::valid_name(&dep.name) {
-        return Err(cargo_err(&format_args!(
-            "\"{}\" is an invalid dependency name (dependency names must \
-            start with a letter, contain only letters, numbers, hyphens, \
-            or underscores and have at most {MAX_NAME_LENGTH} characters)",
-            dep.name
-        )));
-    }
+    Crate::valid_name(&dep.name)?;
 
     for feature in &dep.features {
         if !Crate::valid_feature(feature) {
@@ -622,14 +607,7 @@ pub fn validate_dependency(dep: &EncodableCrateDependency) -> AppResult<()> {
     }
 
     if let Some(toml_name) = &dep.explicit_name_in_toml {
-        if !Crate::valid_dependency_name(toml_name) {
-            return Err(cargo_err(&format_args!(
-                "\"{toml_name}\" is an invalid dependency name (dependency \
-                names must start with a letter or underscore, contain only \
-                letters, numbers, hyphens, or underscores and have at most \
-                {MAX_NAME_LENGTH} characters)"
-            )));
-        }
+        Crate::valid_dependency_name(toml_name)?;
     }
 
     Ok(())

src/models/krate.rs

@@ -192,32 +192,63 @@ impl Crate {
             })
     }
 
-    pub fn valid_name(name: &str) -> bool {
-        let under_max_length = name.chars().take(MAX_NAME_LENGTH + 1).count() <= MAX_NAME_LENGTH;
-        Crate::valid_ident(name) && under_max_length
+    pub fn valid_name(name: &str) -> AppResult<()> {
+        if name.chars().count() > MAX_NAME_LENGTH {
+            return Err(cargo_err(&format!(
+                "the name `{}` is too long (max {} characters)",
+                name, MAX_NAME_LENGTH
+            )));
+        }
+        Crate::valid_ident(name, "crate name")
     }
 
-    fn valid_ident(name: &str) -> bool {
-        Self::valid_feature_prefix(name)
-            && name
-                .chars()
-                .next()
-                .map(char::is_alphabetic)
-                .unwrap_or(false)
+    pub fn valid_dependency_name(name: &str) -> AppResult<()> {
+        if name.chars().count() > MAX_NAME_LENGTH {
+            return Err(cargo_err(&format!(
+                "the name `{}` is too long (max {} characters)",
+                name, MAX_NAME_LENGTH
+            )));
+        }
+        Crate::valid_ident(name, "dependency name")
     }
 
-    pub fn valid_dependency_name(name: &str) -> bool {
-        let under_max_length = name.chars().take(MAX_NAME_LENGTH + 1).count() <= MAX_NAME_LENGTH;
-        Crate::valid_dependency_ident(name) && under_max_length
-    }
+    // Checks that the name is a valid identifier.
+    // 1. The name must be non-empty.
+    // 2. The first character must be an ASCII character or `_`.
+    // 3. The remaining characters must be ASCII alphanumerics or `-` or `_`.
+    fn valid_ident(name: &str, what: &str) -> AppResult<()> {
+        if name.is_empty() {
+            return Err(cargo_err(&format!("the {} cannot be an empty", what)));
+        }
+        let mut chars = name.chars();
+        if let Some(ch) = chars.next() {
+            if ch.is_ascii_digit() {
+                return Err(cargo_err(&format!(
+                    "the name `{}` cannot be used as a {}, \
+                the name cannot start with a digit",
+                    name, what,
+                )));
+            }
+            if !(ch.is_ascii_alphabetic() || ch == '_') {
+                return Err(cargo_err(&format!(
+                    "invalid character `{}` in {}: `{}`, \
+                the first character must be an ASCII character, or `_`",
+                    ch, what, name
+                )));
+            }
+        }
 
-    fn valid_dependency_ident(name: &str) -> bool {
-        Self::valid_feature_prefix(name)
-            && name
-                .chars()
-                .next()
-                .map(|n| n.is_alphabetic() || n == '_')
-                .unwrap_or(false)
+        for ch in chars {
+            if !(ch.is_ascii_alphanumeric() || ch == '-' || ch == '_') {
+                return Err(cargo_err(&format!(
+                    "invalid character `{}` in {}: `{}`, \
+                characters must be an ASCII alphanumeric characters, `-`, or `_`",
+                    ch, what, name
+                )));
+            }
+        }
+
+        Ok(())
     }
 
     /// Validates the THIS parts of `features = ["THIS", "and/THIS", "dep:THIS", "dep?/THIS"]`.
@@ -257,21 +288,13 @@ impl Crate {
         Ok(())
     }
 
-    /// Validates the prefix in front of the slash: `features = ["THIS/feature"]`.
-    /// Normally this corresponds to the crate name of a dependency.
-    fn valid_feature_prefix(name: &str) -> bool {
-        !name.is_empty()
-            && name
-                .chars()
-                .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '-')
-    }
-
     /// Validates a whole feature string, `features = ["THIS", "ALL/THIS"]`.
     pub fn valid_feature(name: &str) -> bool {
         match name.split_once('/') {
             Some((dep, dep_feat)) => {
                 let dep = dep.strip_suffix('?').unwrap_or(dep);
-                Crate::valid_feature_prefix(dep) && Crate::valid_feature_name(dep_feat).is_ok()
+                Crate::valid_dependency_name(dep).is_ok()
+                    && Crate::valid_feature_name(dep_feat).is_ok()
             }
             None => Crate::valid_feature_name(name.strip_prefix("dep:").unwrap_or(name)).is_ok(),
         }
@@ -518,30 +541,28 @@ mod tests {
 
     #[test]
     fn valid_name() {
-        assert!(Crate::valid_name("foo"));
-        assert!(!Crate::valid_name("京"));
-        assert!(!Crate::valid_name(""));
-        assert!(!Crate::valid_name("💝"));
-        assert!(Crate::valid_name("foo_underscore"));
-        assert!(Crate::valid_name("foo-dash"));
-        assert!(!Crate::valid_name("foo+plus"));
-        // Starting with an underscore is an invalid crate name.
-        assert!(!Crate::valid_name("_foo"));
-        assert!(!Crate::valid_name("-foo"));
+        assert!(Crate::valid_name("foo").is_ok());
+        assert!(Crate::valid_name("京").is_err());
+        assert!(Crate::valid_name("").is_err());
+        assert!(Crate::valid_name("💝").is_err());
+        assert!(Crate::valid_name("foo_underscore").is_ok());
+        assert!(Crate::valid_name("foo-dash").is_ok());
+        assert!(Crate::valid_name("foo+plus").is_err());
+        assert!(Crate::valid_name("_foo").is_ok());
+        assert!(Crate::valid_name("-foo").is_err());
     }
 
     #[test]
     fn valid_dependency_name() {
-        assert!(Crate::valid_dependency_name("foo"));
-        assert!(!Crate::valid_dependency_name("京"));
-        assert!(!Crate::valid_dependency_name(""));
-        assert!(!Crate::valid_dependency_name("💝"));
-        assert!(Crate::valid_dependency_name("foo_underscore"));
-        assert!(Crate::valid_dependency_name("foo-dash"));
-        assert!(!Crate::valid_dependency_name("foo+plus"));
-        // Starting with an underscore is a valid dependency name.
-        assert!(Crate::valid_dependency_name("_foo"));
-        assert!(!Crate::valid_dependency_name("-foo"));
+        assert!(Crate::valid_dependency_name("foo").is_ok());
+        assert!(Crate::valid_dependency_name("京").is_err());
+        assert!(Crate::valid_dependency_name("").is_err());
+        assert!(Crate::valid_dependency_name("💝").is_err());
+        assert!(Crate::valid_dependency_name("foo_underscore").is_ok());
+        assert!(Crate::valid_dependency_name("foo-dash").is_ok());
+        assert!(Crate::valid_dependency_name("foo+plus").is_err());
+        assert!(Crate::valid_dependency_name("_foo").is_ok());
+        assert!(Crate::valid_dependency_name("-foo").is_err());
     }
 
     #[test]

src/models/token/scopes.rs

@@ -108,7 +108,7 @@ impl CrateScope {
         }
 
         let name_without_wildcard = pattern.strip_suffix('*').unwrap_or(pattern);
-        Crate::valid_name(name_without_wildcard)
+        Crate::valid_name(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": "\"🦀\" is an invalid dependency name (dependency names must start with a letter, contain only letters, numbers, hyphens, or underscores and have at most 64 characters)"
+      "detail": "invalid character `🦀` in crate name: `🦀`, the first character must be an ASCII character, or `_`"
     }
   ]
 }

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "detail": "\"💩\" is an invalid dependency name (dependency names must start with a letter or underscore, contain only letters, numbers, hyphens, or underscores and have at most 64 characters)"
+      "detail": "invalid character `💩` in dependency name: `💩`, the first character must be an ASCII character, or `_`"
     }
   ]
 }

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "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)"
+      "detail": "invalid character ` ` in crate name: `foo bar`, characters must be an ASCII alphanumeric characters, `-`, or `_`"
     }
   ]
 }

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "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)"
+      "detail": "the name `aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa` is too long (max 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": "\"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)"
+      "detail": "invalid character `☃` in crate name: `snow☃`, characters must be an ASCII alphanumeric characters, `-`, or `_`"
     }
   ]
 }

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "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)"
+      "detail": "invalid character `á` in crate name: `áccênts`, the first character must be an ASCII character, or `_`"
     }
   ]
 }

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

@@ -5,7 +5,7 @@ expression: response.into_json()
 {
   "errors": [
     {
-      "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)"
+      "detail": "the crate name cannot be an empty"
     }
   ]
 }