Merge pull request #718 from integer32llc/limit-names

Limit crate names to 64 characters

Author: alexcrichton

src/krate.rs

@@ -67,6 +67,8 @@ pub const ALL_COLUMNS: AllColumns = (crates::id, crates::name,
     crates::readme, crates::license, crates::repository,
     crates::max_upload_size);
 
+pub const MAX_NAME_LENGTH: usize = 64;
+
 type CrateQuery<'a> = crates::BoxedQuery<'a, Pg, <AllColumns as Expression>::SqlType>;
 
 #[derive(RustcEncodable, RustcDecodable)]
@@ -370,6 +372,13 @@ 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
+    }
+
+    fn valid_ident(name: &str) -> bool {
         if name.is_empty() { return false }
         name.chars().next().unwrap().is_alphabetic() &&
             name.chars().all(|c| c.is_alphanumeric() || c == '_' || c == '-') &&
@@ -379,12 +388,12 @@ impl Crate {
     pub fn valid_feature_name(name: &str) -> bool {
         let mut parts = name.split('/');
         match parts.next() {
-            Some(part) if !Crate::valid_name(part) => return false,
+            Some(part) if !Crate::valid_ident(part) => return false,
             None => return false,
             _ => {}
         }
         match parts.next() {
-            Some(part) if !Crate::valid_name(part) => return false,
+            Some(part) if !Crate::valid_ident(part) => return false,
             _ => {}
         }
         parts.next().is_none()

src/tests/krate.rs

@@ -14,7 +14,7 @@ use cargo_registry::dependency::EncodableDependency;
 use cargo_registry::download::EncodableVersionDownload;
 use cargo_registry::git;
 use cargo_registry::keyword::{Keyword, EncodableKeyword};
-use cargo_registry::krate::{Crate, EncodableCrate};
+use cargo_registry::krate::{Crate, EncodableCrate, MAX_NAME_LENGTH};
 use cargo_registry::upload as u;
 use cargo_registry::user::EncodableUser;
 use cargo_registry::version::EncodableVersion;
@@ -397,14 +397,14 @@ fn new_bad_names() {
         let (_b, app, middle) = ::app();
         let mut req = ::new_req(app, name, "1.0.0");
         ::mock_user(&mut req, ::user("foo"));
-        ::logout(&mut req);
         let json = bad_resp!(middle.call(&mut req));
         assert!(json.errors[0].detail.contains("invalid crate name"),
                 "{:?}", json.errors);
     }
 
     bad_name("");
     bad_name("foo bar");
+    bad_name(&"a".repeat(MAX_NAME_LENGTH + 1));
 }
 
 #[test]

src/upload.rs

@@ -6,7 +6,7 @@ use semver;
 use dependency::Kind as DependencyKind;
 
 use keyword::Keyword as CrateKeyword;
-use krate::Crate;
+use krate::{Crate, MAX_NAME_LENGTH};
 
 #[derive(RustcDecodable, RustcEncodable)]
 pub struct NewCrate {
@@ -52,7 +52,10 @@ impl Decodable for CrateName {
     fn decode<D: Decoder>(d: &mut D) -> Result<CrateName, D::Error> {
         let s = d.read_str()?;
         if !Crate::valid_name(&s) {
-            return Err(d.error(&format!("invalid crate name specified: {}", s)))
+            return Err(d.error(&format!("invalid crate name specified: {}. \
+                Valid crate names must start with a letter; contain only \
+                letters, numbers, hyphens, or underscores; and have {} or \
+                fewer characters.", s, MAX_NAME_LENGTH)))
         }
         Ok(CrateName(s))
     }