Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support relative paths for registries #6873

Merged
merged 5 commits into from
Apr 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/cargo/ops/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
)
}
Expand Down
32 changes: 24 additions & 8 deletions src/cargo/util/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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<()> {
Expand Down Expand Up @@ -1648,3 +1663,4 @@ pub fn save_credentials(cfg: &Config, token: String, registry: Option<String>) -
Ok(())
}
}

2 changes: 2 additions & 0 deletions src/cargo/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 7 additions & 1 deletion src/cargo/util/to_url.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::path::Path;
use std::path::{Path, PathBuf};

use url::Url;

Expand All @@ -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()
}
}
45 changes: 45 additions & 0 deletions src/cargo/util/to_url_with_base.rs
Original file line number Diff line number Diff line change
@@ -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");
}
}
170 changes: 169 additions & 1 deletion tests/testsuite/alt_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:[email protected]"
"#,
)
.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");
Expand Down Expand Up @@ -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();
}