add the Cows back in and add a test to keep them

Eh2406 · Eh2406 · commit ea957da75a29 · 2018-07-07T11:12:22.000-04:00
diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs
@@ -1,4 +1,6 @@
+use std::borrow::Borrow;
 use std::collections::{BTreeMap, HashMap};
+use std::fmt::Display;
 use std::mem;
 use std::rc::Rc;
 
@@ -30,13 +32,14 @@ struct Inner {
 }
 
 impl Summary {
-    pub fn new(
+    pub fn new<K>(
         pkg_id: PackageId,
         dependencies: Vec<Dependency>,
-        features: BTreeMap<&str, Vec<&str>>,
-        links: Option<&str>,
+        features: BTreeMap<K, Vec<impl AsRef<str>>>,
+        links: Option<impl AsRef<str>>,
         namespaced_features: bool,
-    ) -> CargoResult<Summary> {
+    ) -> CargoResult<Summary>
+    where K: Borrow<str> + Ord + Display {
         for dep in dependencies.iter() {
             if !namespaced_features && features.get(&*dep.name()).is_some() {
                 bail!(
@@ -59,7 +62,7 @@ impl Summary {
                 dependencies,
                 features: feature_map,
                 checksum: None,
-                links: links.map(|l| InternedString::new(&l)),
+                links: links.map(|l| InternedString::new(l.as_ref())),
                 namespaced_features,
             }),
         })
@@ -134,11 +137,12 @@ impl PartialEq for Summary {
 
 // Checks features for errors, bailing out a CargoResult:Err if invalid,
 // and creates FeatureValues for each feature.
-fn build_feature_map(
-    features: BTreeMap<&str, Vec<&str>>,
+fn build_feature_map<K>(
+    features: BTreeMap<K, Vec<impl AsRef<str>>>,
     dependencies: &[Dependency],
     namespaced: bool,
-) -> CargoResult<FeatureMap> {
+) -> CargoResult<FeatureMap>
+where K: Borrow<str> + Ord + Display {
     use self::FeatureValue::*;
     let mut dep_map = HashMap::new();
     for dep in dependencies.iter() {
@@ -149,7 +153,7 @@ fn build_feature_map(
     }
 
     let mut map = BTreeMap::new();
-    for (&feature, list) in features.iter() {
+    for (feature, list) in features.iter() {
         // If namespaced features is active and the key is the same as that of an
         // optional dependency, that dependency must be included in the values.
         // Thus, if a `feature` is found that has the same name as a dependency, we
@@ -160,7 +164,7 @@ fn build_feature_map(
         // as the name of an optional dependency. If so, it gets set to true during
         // iteration over the list if the dependency is found in the list.
         let mut dependency_found = if namespaced {
-            match dep_map.get(feature) {
+            match dep_map.get(feature.borrow()) {
                 Some(ref dep_data) => {
                     if !dep_data.iter().any(|d| d.is_optional()) {
                         bail!(
@@ -186,7 +190,7 @@ fn build_feature_map(
         let mut values = vec![];
         for dep in list {
             let val = FeatureValue::build(
-                InternedString::new(dep),
+                InternedString::new(dep.as_ref()),
                 |fs| features.contains_key(fs.as_str()),
                 namespaced,
             );
@@ -206,7 +210,7 @@ fn build_feature_map(
             if let FeatureValue::Crate(ref dep_name) = val {
                 // If we have a dependency value, check if this is the dependency named
                 // the same as the feature that we were looking for.
-                if !dependency_found && feature == dep_name.as_str() {
+                if !dependency_found && feature.borrow() == dep_name.as_str() {
                     dependency_found = true;
                 }
             }
@@ -316,7 +320,7 @@ fn build_feature_map(
             )
         }
 
-        map.insert(InternedString::new(&feature), values);
+        map.insert(InternedString::new(feature.borrow()), values);
     }
     Ok(map)
 }
diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs
@@ -158,12 +158,15 @@
 //!         ...
 //! ```
 
+use std::borrow::Cow;
 use std::collections::BTreeMap;
 use std::fs::File;
 use std::path::{Path, PathBuf};
 
 use flate2::read::GzDecoder;
 use semver::Version;
+#[cfg(test)]
+use serde_json;
 use tar::Archive;
 
 use core::dependency::{Dependency, Kind};
@@ -211,13 +214,43 @@ pub struct RegistryConfig {
 
 #[derive(Deserialize)]
 pub struct RegistryPackage<'a> {
-    name: &'a str,
+    name: Cow<'a, str>,
     vers: Version,
     deps: Vec<RegistryDependency<'a>>,
-    features: BTreeMap<&'a str, Vec<&'a str>>,
+    features: BTreeMap<Cow<'a, str>, Vec<Cow<'a, str>>>,
     cksum: String,
     yanked: Option<bool>,
-    links: Option<&'a str>,
+    links: Option<Cow<'a, str>>,
+}
+
+#[test]
+fn escaped_cher_in_json() {
+    let _: RegistryPackage = serde_json::from_str(
+        r#"{"name":"a","vers":"0.0.1","deps":[],"cksum":"bae3","features":{}}"#
+    ).unwrap();
+    let _: RegistryPackage = serde_json::from_str(
+        r#"{"name":"a","vers":"0.0.1","deps":[],"cksum":"bae3","features":{"test":["k","q"]},"links":"a-sys"}"#
+    ).unwrap();
+
+    // Now we add escaped cher all the places they can go
+    // these are not valid, but it should error later than json parsing
+    let _: RegistryPackage = serde_json::from_str(r#"{
+        "name":"This name has a escaped cher in it \n\t\" ",
+        "vers":"0.0.1",
+        "deps":[{
+            "name": " \n\t\" ",
+            "req": " \n\t\" ",
+            "features": [" \n\t\" "],
+            "optional": true,
+            "default_features": true,
+            "target": " \n\t\" ",
+            "kind": " \n\t\" ",
+            "registry": " \n\t\" "
+        }],
+        "cksum":"bae3",
+        "features":{"test \n\t\" ":["k \n\t\" ","q \n\t\" "]},
+        "links":" \n\t\" "}"#
+    ).unwrap();
 }
 
 #[derive(Deserialize)]
@@ -234,14 +267,14 @@ enum Field {
 
 #[derive(Deserialize)]
 struct RegistryDependency<'a> {
-    name: &'a str,
-    req: &'a str,
-    features: Vec<&'a str>,
+    name: Cow<'a, str>,
+    req: Cow<'a, str>,
+    features: Vec<Cow<'a, str>>,
     optional: bool,
     default_features: bool,
-    target: Option<&'a str>,
-    kind: Option<&'a str>,
-    registry: Option<&'a str>,
+    target: Option<Cow<'a, str>>,
+    kind: Option<Cow<'a, str>>,
+    registry: Option<Cow<'a, str>>,
 }
 
 impl<'a> RegistryDependency<'a> {
diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs
@@ -889,7 +889,7 @@ impl TomlManifest {
                 .as_ref()
                 .map(|x| {
                     x.iter()
-                        .map(|(k, v)| (k.as_str(), v.iter().map(|s| s.as_str()).collect()))
+                        .map(|(k, v)| (k.as_str(), v.iter().collect()))
                         .collect()
                 })
                 .unwrap_or_else(BTreeMap::new),
diff --git a/tests/testsuite/resolve.rs b/tests/testsuite/resolve.rs
@@ -38,7 +38,7 @@ fn resolve_with_config(
         }
     }
     let mut registry = MyRegistry(registry);
-    let summary = Summary::new(pkg.clone(), deps, BTreeMap::new(), None, false).unwrap();
+    let summary = Summary::new(pkg.clone(), deps, BTreeMap::<String, Vec<String>>::new(), None::<String>, false).unwrap();
     let method = Method::Everything;
     let resolve = resolver::resolve(
         &[(summary, method)],
@@ -100,13 +100,13 @@ macro_rules! pkg {
         let pkgid = $pkgid.to_pkgid();
         let link = if pkgid.name().ends_with("-sys") {Some(pkgid.name().as_str())} else {None};
 
-        Summary::new(pkgid, d, BTreeMap::new(), link, false).unwrap()
+        Summary::new(pkgid, d, BTreeMap::<String, Vec<String>>::new(), link, false).unwrap()
     });
 
     ($pkgid:expr) => ({
         let pkgid = $pkgid.to_pkgid();
         let link = if pkgid.name().ends_with("-sys") {Some(pkgid.name().as_str())} else {None};
-        Summary::new(pkgid, Vec::new(), BTreeMap::new(), link, false).unwrap()
+        Summary::new(pkgid, Vec::new(), BTreeMap::<String, Vec<String>>::new(), link, false).unwrap()
     })
 }
 
@@ -121,7 +121,7 @@ fn pkg(name: &str) -> Summary {
     } else {
         None
     };
-    Summary::new(pkg_id(name), Vec::new(), BTreeMap::new(), link, false).unwrap()
+    Summary::new(pkg_id(name), Vec::new(), BTreeMap::<String, Vec<String>>::new(), link, false).unwrap()
 }
 
 fn pkg_id(name: &str) -> PackageId {
@@ -145,7 +145,7 @@ fn pkg_loc(name: &str, loc: &str) -> Summary {
     Summary::new(
         pkg_id_loc(name, loc),
         Vec::new(),
-        BTreeMap::new(),
+        BTreeMap::<String, Vec<String>>::new(),
         link,
         false,
     ).unwrap()
