Auto merge of #4957 - sfackler:registry-elaboration, r=alexcrichton

Elaborate registry names to index URLs when publishing

This avoids introducing a dependency on the publisher's name for the
registry.

Closes #4880

src/cargo/core/package.rs

@@ -134,8 +134,8 @@ impl Package {
         }
     }
 
-    pub fn to_registry_toml(&self) -> CargoResult<String> {
-        let manifest = self.manifest().original().prepare_for_publish();
+    pub fn to_registry_toml(&self, config: &Config) -> CargoResult<String> {
+        let manifest = self.manifest().original().prepare_for_publish(config)?;
         let toml = toml::to_string(&manifest)?;
         Ok(format!("\
             # THIS FILE IS AUTOMATICALLY GENERATED BY CARGO\n\

src/cargo/ops/cargo_package.rs

@@ -249,7 +249,7 @@ fn tar(ws: &Workspace,
             })?;
 
             let mut header = Header::new_ustar();
-            let toml = pkg.to_registry_toml()?;
+            let toml = pkg.to_registry_toml(ws.config())?;
             header.set_path(&path)?;
             header.set_entry_type(EntryType::file());
             header.set_mode(0o644);

src/cargo/util/config.rs

@@ -552,7 +552,13 @@ impl Config {
     /// Gets the index for a registry.
     pub fn get_registry_index(&self, registry: &str) -> CargoResult<Url> {
         Ok(match self.get_string(&format!("registries.{}.index", registry))? {
-            Some(index) => index.val.to_url()?,
+            Some(index) => {
+                let url = index.val.to_url()?;
+                if url.username() != "" || url.password().is_some() {
+                    bail!("Registry URLs may not contain credentials");
+                }
+                url
+            }
             None => bail!("No index found for registry: `{}`", registry),
         })
     }

src/cargo/util/toml/mod.rs

@@ -178,25 +178,26 @@ impl<'de> de::Deserialize<'de> for TomlDependency {
 }
 
 #[derive(Deserialize, Serialize, Clone, Debug, Default)]
+#[serde(rename_all = "kebab-case")]
 pub struct DetailedTomlDependency {
     version: Option<String>,
     registry: Option<String>,
+    registry_index: Option<String>,
     path: Option<String>,
     git: Option<String>,
     branch: Option<String>,
     tag: Option<String>,
     rev: Option<String>,
     features: Option<Vec<String>>,
     optional: Option<bool>,
-    #[serde(rename = "default-features")]
     default_features: Option<bool>,
     #[serde(rename = "default_features")]
     default_features2: Option<bool>,
 }
 
 #[derive(Debug, Deserialize, Serialize)]
+#[serde(rename_all = "kebab-case")]
 pub struct TomlManifest {
-    #[serde(rename = "cargo-features")]
     cargo_features: Option<Vec<String>>,
     package: Option<Box<TomlProject>>,
     project: Option<Box<TomlProject>>,
@@ -207,11 +208,9 @@ pub struct TomlManifest {
     test: Option<Vec<TomlTestTarget>>,
     bench: Option<Vec<TomlTestTarget>>,
     dependencies: Option<BTreeMap<String, TomlDependency>>,
-    #[serde(rename = "dev-dependencies")]
     dev_dependencies: Option<BTreeMap<String, TomlDependency>>,
     #[serde(rename = "dev_dependencies")]
     dev_dependencies2: Option<BTreeMap<String, TomlDependency>>,
-    #[serde(rename = "build-dependencies")]
     build_dependencies: Option<BTreeMap<String, TomlDependency>>,
     #[serde(rename = "build_dependencies")]
     build_dependencies2: Option<BTreeMap<String, TomlDependency>>,
@@ -471,13 +470,13 @@ struct Context<'a, 'b> {
 }
 
 impl TomlManifest {
-    pub fn prepare_for_publish(&self) -> TomlManifest {
+    pub fn prepare_for_publish(&self, config: &Config) -> CargoResult<TomlManifest> {
         let mut package = self.package.as_ref()
                               .or_else(|| self.project.as_ref())
                               .unwrap()
                               .clone();
         package.workspace = None;
-        return TomlManifest {
+        return Ok(TomlManifest {
             package: Some(package),
             project: None,
             profile: self.profile.clone(),
@@ -486,56 +485,68 @@ impl TomlManifest {
             example: self.example.clone(),
             test: self.test.clone(),
             bench: self.bench.clone(),
-            dependencies: map_deps(self.dependencies.as_ref()),
-            dev_dependencies: map_deps(self.dev_dependencies.as_ref()
-                                         .or_else(|| self.dev_dependencies2.as_ref())),
+            dependencies: map_deps(config, self.dependencies.as_ref())?,
+            dev_dependencies: map_deps(config, self.dev_dependencies.as_ref()
+                                         .or_else(|| self.dev_dependencies2.as_ref()))?,
             dev_dependencies2: None,
-            build_dependencies: map_deps(self.build_dependencies.as_ref()
-                                         .or_else(|| self.build_dependencies2.as_ref())),
+            build_dependencies: map_deps(config, self.build_dependencies.as_ref()
+                                         .or_else(|| self.build_dependencies2.as_ref()))?,
             build_dependencies2: None,
             features: self.features.clone(),
-            target: self.target.as_ref().map(|target_map| {
+            target: match self.target.as_ref().map(|target_map| {
                 target_map.iter().map(|(k, v)| {
-                    (k.clone(), TomlPlatform {
-                        dependencies: map_deps(v.dependencies.as_ref()),
-                        dev_dependencies: map_deps(v.dev_dependencies.as_ref()
-                                                     .or_else(|| v.dev_dependencies2.as_ref())),
+                    Ok((k.clone(), TomlPlatform {
+                        dependencies: map_deps(config, v.dependencies.as_ref())?,
+                        dev_dependencies: map_deps(config, v.dev_dependencies.as_ref()
+                                                     .or_else(|| v.dev_dependencies2.as_ref()))?,
                         dev_dependencies2: None,
-                        build_dependencies: map_deps(v.build_dependencies.as_ref()
-                                                     .or_else(|| v.build_dependencies2.as_ref())),
+                        build_dependencies: map_deps(config, v.build_dependencies.as_ref()
+                                                     .or_else(|| v.build_dependencies2.as_ref()))?,
                         build_dependencies2: None,
-                    })
+                    }))
                 }).collect()
-            }),
+            }) {
+                Some(Ok(v)) => Some(v),
+                Some(Err(e)) => return Err(e),
+                None => None,
+            },
             replace: None,
             patch: None,
             workspace: None,
             badges: self.badges.clone(),
             cargo_features: self.cargo_features.clone(),
-        };
+        });
 
-        fn map_deps(deps: Option<&BTreeMap<String, TomlDependency>>)
-                        -> Option<BTreeMap<String, TomlDependency>>
+        fn map_deps(config: &Config, deps: Option<&BTreeMap<String, TomlDependency>>)
+                        -> CargoResult<Option<BTreeMap<String, TomlDependency>>>
         {
             let deps = match deps {
                 Some(deps) => deps,
-                None => return None
+                None => return Ok(None),
             };
-            Some(deps.iter().map(|(k, v)| (k.clone(), map_dependency(v))).collect())
+            let deps = deps.iter()
+                .map(|(k, v)| Ok((k.clone(), map_dependency(config, v)?)))
+                .collect::<CargoResult<BTreeMap<_, _>>>()?;
+            Ok(Some(deps))
         }
 
-        fn map_dependency(dep: &TomlDependency) -> TomlDependency {
+        fn map_dependency(config: &Config, dep: &TomlDependency) -> CargoResult<TomlDependency> {
             match *dep {
                 TomlDependency::Detailed(ref d) => {
                     let mut d = d.clone();
                     d.path.take(); // path dependencies become crates.io deps
-                    TomlDependency::Detailed(d)
+                    // registry specifications are elaborated to the index URL
+                    if let Some(registry) = d.registry.take() {
+                        let src = SourceId::alt_registry(config, &registry)?;
+                        d.registry_index = Some(src.url().to_string());
+                    }
+                    Ok(TomlDependency::Detailed(d))
                 }
                 TomlDependency::Simple(ref s) => {
-                    TomlDependency::Detailed(DetailedTomlDependency {
+                    Ok(TomlDependency::Detailed(DetailedTomlDependency {
                         version: Some(s.clone()),
                         ..Default::default()
-                    })
+                    }))
                 }
             }
         }
@@ -933,10 +944,18 @@ impl TomlDependency {
             None => SourceId::crates_io(cx.config)?
         };
 
-        let new_source_id = match (details.git.as_ref(), details.path.as_ref(), details.registry.as_ref()) {
-            (Some(_), _, Some(_)) => bail!("dependency ({}) specification is ambiguous. \
+        let new_source_id = match (
+            details.git.as_ref(),
+            details.path.as_ref(),
+            details.registry.as_ref(),
+            details.registry_index.as_ref(),
+        ) {
+            (Some(_), _, Some(_), _) |
+            (Some(_), _, _, Some(_))=> bail!("dependency ({}) specification is ambiguous. \
                                             Only one of `git` or `registry` is allowed.", name),
-            (Some(git), maybe_path, _) => {
+            (_, _, Some(_), Some(_)) => bail!("dependency ({}) specification is ambiguous. \
+                                            Only one of `registry` or `registry-index` is allowed.", name),
+            (Some(git), maybe_path, _, _) => {
                 if maybe_path.is_some() {
                     let msg = format!("dependency ({}) specification is ambiguous. \
                                        Only one of `git` or `path` is allowed. \
@@ -963,7 +982,7 @@ impl TomlDependency {
                 let loc = git.to_url()?;
                 SourceId::for_git(&loc, reference)?
             },
-            (None, Some(path), _) => {
+            (None, Some(path), _, _) => {
                 cx.nested_paths.push(PathBuf::from(path));
                 // If the source id for the package we're parsing is a path
                 // source, then we normalize the path here to get rid of
@@ -981,8 +1000,12 @@ impl TomlDependency {
                     cx.source_id.clone()
                 }
             },
-            (None, None, Some(registry)) => SourceId::alt_registry(cx.config, registry)?,
-            (None, None, None) => SourceId::crates_io(cx.config)?,
+            (None, None, Some(registry), None) => SourceId::alt_registry(cx.config, registry)?,
+            (None, None, None, Some(registry_index)) => {
+                let url = registry_index.to_url()?;
+                SourceId::for_registry(&url)?
+            }
+            (None, None, None, None) => SourceId::crates_io(cx.config)?,
         };
 
         let version = details.version.as_ref().map(|v| &v[..]);

tests/alt-registry.rs

@@ -3,8 +3,10 @@ extern crate hamcrest;
 
 use cargotest::ChannelChanger;
 use cargotest::support::registry::{self, Package, alt_api_path};
-use cargotest::support::{project, execs};
+use cargotest::support::{paths, project, execs};
 use hamcrest::assert_that;
+use std::fs::File;
+use std::io::Write;
 
 #[test]
 fn is_feature_gated() {
@@ -423,3 +425,35 @@ fn publish_with_crates_io_dep() {
                  .arg("--registry").arg("alternative").arg("-Zunstable-options"),
                 execs().with_status(0));
 }
+
+#[test]
+fn credentials_in_url_forbidden() {
+    registry::init();
+
+    let config = paths::home().join(".cargo/config");
+
+    File::create(config)
+        .unwrap()
+        .write_all(br#"
+        [registries.alternative]
+        index = "ssh://git:secret@foobar.com"
+        "#)
+        .unwrap();
+
+    let p = project("foo")
+        .file("Cargo.toml", r#"
+            cargo-features = ["alternative-registries"]
+
+            [project]
+            name = "foo"
+            version = "0.0.1"
+            authors = []
+        "#)
+        .file("src/main.rs", "fn main() {}")
+        .build();
+
+    assert_that(p.cargo("publish").masquerade_as_nightly_cargo()
+                 .arg("--registry").arg("alternative").arg("-Zunstable-options"),
+                execs().with_status(101)
+                    .with_stderr_contains("error: Registry URLs may not contain credentials"));
+}

tests/package.rs

@@ -9,8 +9,8 @@ use std::fs::File;
 use std::io::prelude::*;
 use std::path::{Path, PathBuf};
 
-use cargotest::{cargo_process, process};
-use cargotest::support::{project, execs, paths, git, path2url, cargo_exe};
+use cargotest::{cargo_process, process, ChannelChanger};
+use cargotest::support::{project, execs, paths, git, path2url, cargo_exe, registry};
 use cargotest::support::registry::Package;
 use flate2::read::GzDecoder;
 use hamcrest::{assert_that, existing_file, contains, equal_to};
@@ -715,6 +715,8 @@ fn generated_manifest() {
     Package::new("ghi", "1.0.0").publish();
     let p = project("foo")
         .file("Cargo.toml", r#"
+            cargo-features = ["alternative-registries"]
+
             [project]
             name = "foo"
             version = "0.0.1"
@@ -730,7 +732,7 @@ fn generated_manifest() {
 
             [dependencies]
             bar = { path = "bar", version = "0.1" }
-            def = "1.0"
+            def = { version = "1.0", registry = "alternative" }
             ghi = "1.0"
             abc = "1.0"
         "#)
@@ -744,7 +746,9 @@ fn generated_manifest() {
         .file("bar/src/lib.rs", "")
         .build();
 
-    assert_that(p.cargo("package").arg("--no-verify"),
+    assert_that(p.cargo("package")
+                    .masquerade_as_nightly_cargo()
+                    .arg("--no-verify"),
                 execs().with_status(0));
 
     let f = File::open(&p.root().join("target/package/foo-0.0.1.crate")).unwrap();
@@ -761,6 +765,7 @@ fn generated_manifest() {
     // BTreeMap makes the order of dependencies in the generated file deterministic
     // by sorting alphabetically
     assert_that(&contents[..], equal_to(
+&*format!(
 r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
 #
 # When uploading crates to the registry Cargo will automatically
@@ -773,6 +778,8 @@ r#"# THIS FILE IS AUTOMATICALLY GENERATED BY CARGO
 # editing this file be aware that the upstream Cargo.toml
 # will likely look very different (and much more reasonable)
 
+cargo-features = ["alternative-registries"]
+
 [package]
 name = "foo"
 version = "0.0.1"
@@ -791,10 +798,12 @@ version = "0.1"
 
 [dependencies.def]
 version = "1.0"
+registry-index = "{}"
 
 [dependencies.ghi]
 version = "1.0"
-"#));
+"#,
+    registry::alt_registry())));
 }
 
 #[test]