Auto merge of #6873 - fluffysquirrels:rel-registry-url, r=ehuss

Support relative paths for registries

Closes #2252

I haven't finished the tests for this yet as I wasn't sure what was necessary. Is it OK to just test through `registry_configuration()` in `src/cargo/ops/registry.rs` or are integration tests required? If integration tests then I'm not sure what cargo sub-command I can run without a real remote registry to play with; any thoughts?

Author: bors

src/cargo/ops/registry.rs

@@ -324,9 +324,9 @@ pub fn registry_configuration(
             )
         }
         None => {
-            // Checking out for default index and token
+            // Checking for default index and token
             (
-                config.get_string("registry.index")?.map(|p| p.val),
+                config.get_default_registry_index()?.map(|url| url.to_string()),
                 config.get_string("registry.token")?.map(|p| p.val),
             )
         }

src/cargo/util/config.rs

@@ -29,7 +29,7 @@ use crate::util::errors::{self, internal, CargoResult, CargoResultExt};
 use crate::util::toml as cargo_toml;
 use crate::util::Filesystem;
 use crate::util::Rustc;
-use crate::util::ToUrl;
+use crate::util::{ToUrl, ToUrlWithBase};
 use crate::util::{paths, validate_package_name};
 
 /// Configuration information for cargo. This is not specific to a build, it is information
@@ -667,18 +667,33 @@ impl Config {
         validate_package_name(registry, "registry name", "")?;
         Ok(
             match self.get_string(&format!("registries.{}.index", registry))? {
-                Some(index) => {
-                    let url = index.val.to_url()?;
-                    if url.password().is_some() {
-                        failure::bail!("Registry URLs may not contain passwords");
-                    }
-                    url
-                }
+                Some(index) => self.resolve_registry_index(index)?,
                 None => failure::bail!("No index found for registry: `{}`", registry),
             },
         )
     }
 
+    /// Gets the index for the default registry.
+    pub fn get_default_registry_index(&self) -> CargoResult<Option<Url>> {
+        Ok(
+            match self.get_string("registry.index")? {
+                Some(index) => Some(self.resolve_registry_index(index)?),
+                None => None,
+            },
+        )
+    }
+
+    fn resolve_registry_index(&self, index: Value<String>) -> CargoResult<Url> {
+        let base = index.definition.root(&self).join("truncated-by-url_with_base");
+        // Parse val to check it is a URL, not a relative path without a protocol.
+        let _parsed = index.val.to_url()?;
+        let url = index.val.to_url_with_base(Some(&*base))?;
+        if url.password().is_some() {
+            failure::bail!("Registry URLs may not contain passwords");
+        }
+        Ok(url)
+    }
+
     /// Loads credentials config from the credentials file into the `ConfigValue` object, if
     /// present.
     fn load_credentials(&self, cfg: &mut ConfigValue) -> CargoResult<()> {
@@ -1648,3 +1663,4 @@ pub fn save_credentials(cfg: &Config, token: String, registry: Option<String>) -
         Ok(())
     }
 }
+

src/cargo/util/mod.rs

@@ -21,6 +21,7 @@ pub use self::rustc::Rustc;
 pub use self::sha256::Sha256;
 pub use self::to_semver::ToSemver;
 pub use self::to_url::ToUrl;
+pub use self::to_url_with_base::ToUrlWithBase;
 pub use self::vcs::{existing_vcs_repo, FossilRepo, GitRepo, HgRepo, PijulRepo};
 pub use self::workspace::{
     print_available_benches, print_available_binaries, print_available_examples,
@@ -51,6 +52,7 @@ pub mod rustc;
 mod sha256;
 pub mod to_semver;
 pub mod to_url;
+mod to_url_with_base;
 pub mod toml;
 mod vcs;
 mod workspace;

src/cargo/util/to_url.rs

@@ -1,4 +1,4 @@
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 use url::Url;
 
@@ -22,3 +22,9 @@ impl<'a> ToUrl for &'a Path {
             .map_err(|()| failure::format_err!("invalid path url `{}`", self.display()))
     }
 }
+
+impl<'a> ToUrl for &'a PathBuf {
+    fn to_url(self) -> CargoResult<Url> {
+        self.as_path().to_url()
+    }
+}

src/cargo/util/to_url_with_base.rs

@@ -0,0 +1,45 @@
+use crate::util::{CargoResult, ToUrl};
+
+use url::Url;
+
+/// A type that can be interpreted as a relative Url and converted to
+/// a Url.
+pub trait ToUrlWithBase {
+    /// Performs the conversion
+    fn to_url_with_base<U: ToUrl>(self, base: Option<U>) -> CargoResult<Url>;
+}
+
+impl<'a> ToUrlWithBase for &'a str {
+    fn to_url_with_base<U: ToUrl>(self, base: Option<U>) -> CargoResult<Url> {
+        let base_url = match base {
+            Some(base) =>
+                Some(base.to_url().map_err(|s| {
+                    failure::format_err!("invalid url `{}`: {}", self, s)
+                })?),
+            None => None
+        };
+
+        Url::options()
+            .base_url(base_url.as_ref())
+            .parse(self).map_err(|s| {
+                failure::format_err!("invalid url `{}`: {}", self, s)
+            })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use crate::util::ToUrlWithBase;
+
+    #[test]
+    fn to_url_with_base() {
+        assert_eq!("rel/path".to_url_with_base(Some("file:///abs/path/"))
+                       .unwrap()
+                       .to_string(),
+                   "file:///abs/path/rel/path");
+        assert_eq!("rel/path".to_url_with_base(Some("file:///abs/path/popped-file"))
+                       .unwrap()
+                       .to_string(),
+                   "file:///abs/path/rel/path");
+    }
+}

tests/testsuite/alt_registry.rs

@@ -528,7 +528,31 @@ fn publish_with_crates_io_dep() {
 }
 
 #[test]
-fn passwords_in_url_forbidden() {
+fn passwords_in_registry_index_url_forbidden() {
+    registry::init();
+
+    let config = paths::home().join(".cargo/config");
+
+    File::create(config)
+        .unwrap()
+        .write_all(
+            br#"
+        [registry]
+        index = "ssh://git:secret@foobar.com"
+        "#,
+        )
+        .unwrap();
+
+    let p = project().file("src/main.rs", "fn main() {}").build();
+
+    p.cargo("publish")
+        .with_status(101)
+        .with_stderr_contains("error: Registry URLs may not contain passwords")
+        .run();
+}
+
+#[test]
+fn passwords_in_registries_index_url_forbidden() {
     registry::init();
 
     let config = paths::home().join(".cargo/config");
@@ -1141,3 +1165,147 @@ fn unknown_registry() {
         )
         .run();
 }
+
+#[test]
+fn registries_index_relative_url() {
+    let config = paths::root().join(".cargo/config");
+    fs::create_dir_all(config.parent().unwrap()).unwrap();
+    File::create(&config).unwrap()
+        .write_all(br#"
+            [registries.relative]
+            index = "file:alternative-registry"
+        "#).unwrap();
+
+    registry::init();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies.bar]
+            version = "0.0.1"
+            registry = "relative"
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    Package::new("bar", "0.0.1")
+        .alternative(true)
+        .publish();
+
+    p.cargo("build")
+        .with_stderr(&format!(
+            "\
+[UPDATING] `{reg}` index
+[DOWNLOADING] crates ...
+[DOWNLOADED] bar v0.0.1 (registry `[ROOT][..]`)
+[COMPILING] bar v0.0.1 (registry `[ROOT][..]`)
+[COMPILING] foo v0.0.1 ([CWD])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s
+",
+            reg = registry::alt_registry_path().to_str().unwrap()
+        ))
+        .run();
+}
+
+#[test]
+fn registry_index_relative_url() {
+    let config = paths::root().join(".cargo/config");
+    fs::create_dir_all(config.parent().unwrap()).unwrap();
+    File::create(&config).unwrap()
+        .write_all(br#"
+            [registry]
+            index = "file:alternative-registry"
+        "#).unwrap();
+
+    registry::init();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies.bar]
+            version = "0.0.1"
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    Package::new("bar", "0.0.1")
+        .alternative(true)
+        .publish();
+
+    fs::remove_file(paths::home().join(".cargo/config")).unwrap();
+
+    p.cargo("build")
+        .with_stderr(&format!(
+            "\
+warning: custom registry support via the `registry.index` configuration is being removed, this functionality will not work in the future
+[UPDATING] `{reg}` index
+[DOWNLOADING] crates ...
+[DOWNLOADED] bar v0.0.1 (registry `[ROOT][..]`)
+[COMPILING] bar v0.0.1 (registry `[ROOT][..]`)
+[COMPILING] foo v0.0.1 ([CWD])
+[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s
+",
+            reg = registry::alt_registry_path().to_str().unwrap()
+        ))
+        .run();
+}
+
+#[test]
+fn registries_index_relative_path_not_allowed() {
+    let config = paths::root().join(".cargo/config");
+    fs::create_dir_all(config.parent().unwrap()).unwrap();
+    File::create(&config).unwrap()
+        .write_all(br#"
+            [registries.relative]
+            index = "alternative-registry"
+        "#).unwrap();
+
+    registry::init();
+
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+
+            [dependencies.bar]
+            version = "0.0.1"
+            registry = "relative"
+        "#,
+        )
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    Package::new("bar", "0.0.1")
+        .alternative(true)
+        .publish();
+
+    p.cargo("build")
+        .with_stderr(&format!(
+            "\
+error: failed to parse manifest at `{root}/foo/Cargo.toml`
+
+Caused by:
+  invalid url `alternative-registry`: relative URL without a base
+"
+        , root = paths::root().to_str().unwrap()))
+        .with_status(101)
+        .run();
+}