Don't panic on bad crate names

6 files changed, 45 insertions, 24 deletions

diff --git a/deps_index/src/deps_stats.rs b/deps_index/src/deps_stats.rs
index e78fcf3..4702a36 100644
--- a/deps_index/src/deps_stats.rs
+++ b/deps_index/src/deps_stats.rs
@@ -1,4 +1,5 @@
 use crate::index::*;
+use crate::Origin;
 use crate::DepsErr;
 use parking_lot::Mutex;
 use rayon::prelude::*;
@@ -178,7 +179,7 @@ impl Index {
                 let t = n.versions.entry(semver).or_insert(0);
                 *t = t.checked_add(1).expect("overflow");
                 if depinf.direct {
-                    debug_assert!(!parent_name.is_empty());
+                    assert!(Origin::is_valid_crate_name(&parent_name));
                     n.rev_dep_names.push(&parent_name);
                 }
                 match depinf.ty {
diff --git a/kitchen_sink/Cargo.toml b/kitchen_sink/Cargo.toml
index 40fa8ee..b1e0bfc 100644
--- a/kitchen_sink/Cargo.toml
+++ b/kitchen_sink/Cargo.toml
@@ -20,7 +20,7 @@ user_db = { path = "../user_db", version = "0.3" }
 crate_db = { path = "../crate_db", version = "0.4.0" }
 categories = { path = "../categories" }
 render_readme = { path = "../render_readme" }
-rich_crate = { path = "../rich_crate", version = "0.5" }
+rich_crate = { path = "../rich_crate", version = "0.5.1" }
 simple_cache = { path = "../simple_cache", version = "0.7.0" }
 repo_url = { path = "../repo_url", version = "0.3.0" }
 cargo_toml = "0.8.0"
diff --git a/reindex/src/bin/reindex_crates.rs b/reindex/src/bin/reindex_crates.rs
index 73d71a8..7ab1383 100644
--- a/reindex/src/bin/reindex_crates.rs
+++ b/reindex/src/bin/reindex_crates.rs
@@ -259,8 +259,9 @@ async fn crate_overall_score(crates: &KitchenSink, all: &RichCrate, k: &RichCrat
         temp_inp.number_of_indirect_reverse_deps = deps.runtime.def.max(deps.build.def).into();
         temp_inp.number_of_indirect_reverse_optional_deps = indirect_reverse_optional_deps;
         let tmp = futures::future::join_all(deps.rev_dep_names.iter()
-            .map(|name| async move {
-                crates.downloads_per_month(&Origin::from_crates_io_name(name)).await
+            .filter_map(|name| Origin::try_from_crates_io_name(name))
+            .map(|o| async move {
+                crates.downloads_per_month(&o).await
             })).await;
         let biggest = tmp.into_iter().filter_map(|x| x.ok().and_then(|x| x)).max().unwrap_or(0);
         temp_inp.downloads_per_month_minus_most_downloaded_user = downloads_per_month.saturating_sub(biggest as u32);
diff --git a/rich_crate/Cargo.toml b/rich_crate/Cargo.toml
index 8d3288c..a0b0090 100644
--- a/rich_crate/Cargo.toml
+++ b/rich_crate/Cargo.toml
@@ -1,7 +1,7 @@
 [package]
 edition = "2018"
 name = "rich_crate"
-version = "0.5.0"
+version = "0.5.1"
 authors = ["Kornel <kornel@geekhood.net>"]
 description = "Crate struct enriched with additional crates.rs metadata"
 license = "Apache-2.0 OR MIT"
diff --git a/rich_crate/src/lib.rs b/rich_crate/src/lib.rs
index 1b19abf..40f63a2 100644
--- a/rich_crate/src/lib.rs
+++ b/rich_crate/src/lib.rs
@@ -33,19 +33,39 @@ impl fmt::Debug for Origin {
 }
 
 impl Origin {
+    #[inline]
     pub fn from_crates_io_name(name: &str) -> Self {
-        assert!(!name.is_empty() && is_alnum(name), "bad crate name: '{}'", name);
-        Origin::CratesIo(name.to_ascii_lowercase().into())
+        match Self::try_from_crates_io_name(name) {
+            Some(n) => n,
+            None => panic!("bad crate name: '{}'", name),
+        }
+    }
+
+    #[inline]
+    pub fn try_from_crates_io_name(name: &str) -> Option<Self> {
+        if Self::is_valid_crate_name(name) {
+            Some(Origin::CratesIo(name.to_ascii_lowercase().into()))
+        } else {
+            None
+        }
     }
 
+    #[inline(always)]
+    pub fn is_valid_crate_name(name: &str) -> bool {
+        !name.is_empty() && is_alnum(name)
+    }
+
+    #[inline]
     pub fn from_github(repo: SimpleRepo, package: impl Into<Box<str>>) -> Self {
         Origin::GitHub { repo, package: package.into() }
     }
 
+    #[inline]
     pub fn from_gitlab(repo: SimpleRepo, package: impl Into<Box<str>>) -> Self {
         Origin::GitLab { repo, package: package.into() }
     }
 
+    #[inline]
     pub fn from_repo(r: &Repo, package: &str) -> Option<Self> {
         match r.host() {
             RepoHost::GitHub(r) => Some(Self::from_github(r.clone(), package)),
@@ -83,6 +103,7 @@ impl Origin {
         }
     }
 
+    #[inline]
     pub fn short_crate_name(&self) -> &str {
         match *self {
             Origin::CratesIo(ref s) => s,
diff --git a/server/src/main.rs b/server/src/main.rs
index 05b5305..cc00495 100644
--- a/server/src/main.rs
+++ b/server/src/main.rs
@@ -264,19 +264,17 @@ async fn default_handler(req: HttpRequest) -> Result<HttpResponse, ServerError>
     let name = path.trim_matches('/').to_owned();
     let crates = state.crates.load();
     let (found_crate, found_keyword) = rt_run_timeout(&state.rt, 10, async move {
-        let crate_maybe = if is_alnum(&name) {
-            crates.rich_crate_async(&Origin::from_crates_io_name(&name)).await.ok()
-        } else {
-            None
+        let crate_maybe = match Origin::try_from_crates_io_name(&name) {
+            Some(o) => crates.rich_crate_async(&o).await.ok(),
+            _ => None,
         };
         match crate_maybe {
             Some(c) => Ok((Some(c), None)),
             None => {
                 let inverted_hyphens: String = name.chars().map(|c| if c == '-' {'_'} else if c == '_' {'-'} else {c.to_ascii_lowercase()}).collect();
-                let crate_maybe = if is_alnum(&name) {
-                    crates.rich_crate_async(&Origin::from_crates_io_name(&inverted_hyphens)).await.ok()
-                } else {
-                    None
+                let crate_maybe = match Origin::try_from_crates_io_name(&inverted_hyphens) {
+                    Some(o) => crates.rich_crate_async(&o).await.ok(),
+                    _ => None,
                 };
                 match crate_maybe {
                     Some(c) => Ok((Some(c), None)),
@@ -392,7 +390,7 @@ fn get_origin_from_subpath(q: &actix_web::dev::Path<Url>) -> Option<Origin> {
     let mut parts = parts.splitn(4, '/');
     let first = parts.next()?;
     match parts.next() {
-        None => Some(Origin::from_crates_io_name(&first)),
+        None => Origin::try_from_crates_io_name(&first),
         Some(owner) => {
             let repo = parts.next()?;
             let package = parts.next()?;
@@ -475,10 +473,10 @@ async fn handle_crate(req: HttpRequest) -> Result<HttpResponse, ServerError> {
     println!("crate page for {:?}", crate_name);
     let state: &AServerState = req.app_data().expect("appdata");
     let crates = state.crates.load();
-    let origin = Origin::from_crates_io_name(&crate_name);
-    if !is_alnum(&crate_name) || !crates.crate_exists(&origin) {
-        return render_404_page(state, &crate_name, "crate");
-    }
+    let origin = match Origin::try_from_crates_io_name(&crate_name).filter(|o| crates.crate_exists(o)) {
+        Some(o) => o,
+        None => return render_404_page(state, &crate_name, "crate"),
+    };
     let cache_file = state.page_cache_dir.join(format!("{}.html", crate_name));
     Ok(serve_cached(with_file_cache(state, cache_file, 800, {
         render_crate_page(state.clone(), origin)
@@ -490,10 +488,10 @@ async fn handle_crate_reverse_dependencies(req: HttpRequest) -> Result<HttpRespo
     println!("rev deps for {:?}", crate_name);
     let state: &AServerState = req.app_data().expect("appdata");
     let crates = state.crates.load();
-    let origin = Origin::from_crates_io_name(&crate_name);
-    if !is_alnum(&crate_name) || !crates.crate_exists(&origin) {
-        return render_404_page(&state, &crate_name, "crate");
-    }
+    let origin = match Origin::try_from_crates_io_name(&crate_name).filter(|o| crates.crate_exists(o)) {
+        Some(o) => o,
+        None => return render_404_page(state, &crate_name, "crate"),
+    };
     Ok(serve_cached(render_crate_reverse_dependencies(state.clone(), origin).await?))
 }