From ea957da75a29583626707463e05767f2c6f32469 Mon Sep 17 00:00:00 2001 From: Eh2406 Date: Sat, 7 Jul 2018 11:12:22 -0400 Subject: [PATCH] add the Cows back in and add a test to keep them --- src/cargo/core/summary.rs | 30 ++++++++++-------- src/cargo/sources/registry/mod.rs | 51 +++++++++++++++++++++++++------ src/cargo/util/toml/mod.rs | 2 +- tests/testsuite/resolve.rs | 10 +++--- 4 files changed, 65 insertions(+), 28 deletions(-) diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index 1eacd0b59a2..15055f4730e 100644 --- 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( pkg_id: PackageId, dependencies: Vec, - features: BTreeMap<&str, Vec<&str>>, - links: Option<&str>, + features: BTreeMap>>, + links: Option>, namespaced_features: bool, - ) -> CargoResult { + ) -> CargoResult + where K: Borrow + 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( + features: BTreeMap>>, dependencies: &[Dependency], namespaced: bool, -) -> CargoResult { +) -> CargoResult +where K: Borrow + 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 index 1e87bd7e1b9..0d72ae74659 100644 --- 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>, - features: BTreeMap<&'a str, Vec<&'a str>>, + features: BTreeMap, Vec>>, cksum: String, yanked: Option, - links: Option<&'a str>, + links: Option>, +} + +#[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>, optional: bool, default_features: bool, - target: Option<&'a str>, - kind: Option<&'a str>, - registry: Option<&'a str>, + target: Option>, + kind: Option>, + registry: Option>, } impl<'a> RegistryDependency<'a> { diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 603ca2fca68..4a5bf690002 100644 --- 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 index d343d14d739..24d5ff9efb0 100644 --- 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::>::new(), None::, 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::>::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::>::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::>::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::>::new(), link, false, ).unwrap()