Auto merge of #11936 - ehuss:uncanonicalized-path-prefix, r=arlosi

Don't query permutations of the path prefix.

This fixes the use of the `UncanonicalizedIter` to not create permutations that are not valid paths if the crate has a dash or underscore in the first four characters of the name.

Previously it was permuting the path in the prefix, creating invalid paths. For example, `my-tool` would create a path like `my/_t/my-tool` which isn't valid since the filename doesn't match the prefix directories.

This fixes it so that it permutes just the name, and then generates the index path *after* permuting it.

The test included here goes from 16 queries to just 4 queries.

Fixes #11935

Author: bors

crates/cargo-test-support/src/registry.rs

@@ -78,6 +78,8 @@ impl Token {
     }
 }
 
+type RequestCallback = Box<dyn Send + Fn(&Request, &HttpServer) -> Response>;
+
 /// A builder for initializing registries.
 pub struct RegistryBuilder {
     /// If set, configures an alternate registry with the given name.
@@ -97,7 +99,9 @@ pub struct RegistryBuilder {
     /// Write the registry in configuration.
     configure_registry: bool,
     /// API responders.
-    custom_responders: HashMap<String, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
+    custom_responders: HashMap<String, RequestCallback>,
+    /// Handler for 404 responses.
+    not_found_handler: RequestCallback,
     /// If nonzero, the git index update to be delayed by the given number of seconds.
     delayed_index_update: usize,
 }
@@ -149,6 +153,13 @@ impl TestRegistry {
 impl RegistryBuilder {
     #[must_use]
     pub fn new() -> RegistryBuilder {
+        let not_found = |_req: &Request, _server: &HttpServer| -> Response {
+            Response {
+                code: 404,
+                headers: vec![],
+                body: b"not found".to_vec(),
+            }
+        };
         RegistryBuilder {
             alternative: None,
             token: None,
@@ -159,6 +170,7 @@ impl RegistryBuilder {
             configure_registry: true,
             configure_token: true,
             custom_responders: HashMap::new(),
+            not_found_handler: Box::new(not_found),
             delayed_index_update: 0,
         }
     }
@@ -175,6 +187,15 @@ impl RegistryBuilder {
         self
     }
 
+    #[must_use]
+    pub fn not_found_handler<R: 'static + Send + Fn(&Request, &HttpServer) -> Response>(
+        mut self,
+        responder: R,
+    ) -> Self {
+        self.not_found_handler = Box::new(responder);
+        self
+    }
+
     /// Configures the git index update to be delayed by the given number of seconds.
     #[must_use]
     pub fn delayed_index_update(mut self, delay: usize) -> Self {
@@ -276,6 +297,7 @@ impl RegistryBuilder {
                 token.clone(),
                 self.auth_required,
                 self.custom_responders,
+                self.not_found_handler,
                 self.delayed_index_update,
             );
             let index_url = if self.http_index {
@@ -602,7 +624,8 @@ pub struct HttpServer {
     addr: SocketAddr,
     token: Token,
     auth_required: bool,
-    custom_responders: HashMap<String, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
+    custom_responders: HashMap<String, RequestCallback>,
+    not_found_handler: RequestCallback,
     delayed_index_update: usize,
 }
 
@@ -622,7 +645,8 @@ impl HttpServer {
         api_path: PathBuf,
         token: Token,
         auth_required: bool,
-        api_responders: HashMap<String, Box<dyn Send + Fn(&Request, &HttpServer) -> Response>>,
+        custom_responders: HashMap<String, RequestCallback>,
+        not_found_handler: RequestCallback,
         delayed_index_update: usize,
     ) -> HttpServerHandle {
         let listener = TcpListener::bind("127.0.0.1:0").unwrap();
@@ -635,7 +659,8 @@ impl HttpServer {
             addr,
             token,
             auth_required,
-            custom_responders: api_responders,
+            custom_responders,
+            not_found_handler,
             delayed_index_update,
         };
         let handle = Some(thread::spawn(move || server.start()));
@@ -928,12 +953,8 @@ impl HttpServer {
     }
 
     /// Not found response
-    pub fn not_found(&self, _req: &Request) -> Response {
-        Response {
-            code: 404,
-            headers: vec![],
-            body: b"not found".to_vec(),
-        }
+    pub fn not_found(&self, req: &Request) -> Response {
+        (self.not_found_handler)(req, self)
     }
 
     /// Respond OK without doing anything

src/cargo/sources/registry/index.rs

@@ -360,15 +360,15 @@ impl<'cfg> RegistryIndex<'cfg> {
             .chars()
             .flat_map(|c| c.to_lowercase())
             .collect::<String>();
-        let raw_path = make_dep_path(&fs_name, false);
 
         let mut any_pending = false;
         // Attempt to handle misspellings by searching for a chain of related
-        // names to the original `raw_path` name. Only return summaries
+        // names to the original `fs_name` name. Only return summaries
         // associated with the first hit, however. The resolver will later
         // reject any candidates that have the wrong name, and with this it'll
         // along the way produce helpful "did you mean?" suggestions.
-        for (i, path) in UncanonicalizedIter::new(&raw_path).take(1024).enumerate() {
+        for (i, name_permutation) in UncanonicalizedIter::new(&fs_name).take(1024).enumerate() {
+            let path = make_dep_path(&name_permutation, false);
             let summaries = Summaries::parse(
                 root,
                 &cache_root,

tests/testsuite/registry.rs

@@ -4,7 +4,7 @@ use cargo::core::SourceId;
 use cargo_test_support::cargo_process;
 use cargo_test_support::paths::{self, CargoPathExt};
 use cargo_test_support::registry::{
-    self, registry_path, Dependency, Package, RegistryBuilder, TestRegistry,
+    self, registry_path, Dependency, Package, RegistryBuilder, Response, TestRegistry,
 };
 use cargo_test_support::{basic_manifest, project};
 use cargo_test_support::{git, install::cargo_home, t};
@@ -3135,3 +3135,60 @@ fn corrupted_ok_overwritten() {
     p.cargo("fetch").with_stderr("").run();
     assert_eq!(fs::read_to_string(&ok).unwrap(), "ok");
 }
+
+#[cargo_test]
+fn not_found_permutations() {
+    // Test for querying permutations for a missing dependency.
+    let misses = Arc::new(Mutex::new(Vec::new()));
+    let misses2 = misses.clone();
+    let _registry = RegistryBuilder::new()
+        .http_index()
+        .not_found_handler(move |req, _server| {
+            let mut misses = misses2.lock().unwrap();
+            misses.push(req.url.path().to_string());
+            Response {
+                code: 404,
+                headers: vec![],
+                body: b"not found".to_vec(),
+            }
+        })
+        .build();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.0.1"
+                authors = []
+
+                [dependencies]
+                a-b-c = "1.0"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+
+    p.cargo("check")
+        .with_status(101)
+        .with_stderr(
+            "\
+[UPDATING] `dummy-registry` index
+error: no matching package named `a-b-c` found
+location searched: registry `crates-io`
+required by package `foo v0.0.1 ([ROOT]/foo)`
+",
+        )
+        .run();
+    let misses = misses.lock().unwrap();
+    assert_eq!(
+        &*misses,
+        &[
+            "/index/a-/b-/a-b-c",
+            "/index/a_/b-/a_b-c",
+            "/index/a-/b_/a-b_c",
+            "/index/a_/b_/a_b_c"
+        ]
+    );
+}