diff --git a/src/models/dependency.rs b/src/models/dependency.rs index d05e1f6bbc3..c0a699177de 100644 --- a/src/models/dependency.rs +++ b/src/models/dependency.rs @@ -20,7 +20,7 @@ pub struct Dependency { pub id: i32, pub version_id: i32, pub crate_id: i32, - pub req: semver::VersionReq, + pub req: String, pub optional: bool, pub default_features: bool, pub features: Vec, @@ -56,7 +56,7 @@ impl Dependency { id: self.id, version_id: self.version_id, crate_id: crate_name.into(), - req: self.req.to_string(), + req: self.req, optional: self.optional, default_features: self.default_features, features: self.features, @@ -95,7 +95,7 @@ pub fn add_dependencies( let krate:Crate = Crate::by_exact_name(&dep.name) .first(&*conn) .map_err(|_| cargo_err(&format_args!("no known crate named `{}`", &*dep.name)))?; - if dep.version_req == semver::VersionReq::parse("*").unwrap() { + if semver::VersionReq::parse(&dep.version_req.0) == semver::VersionReq::parse("*") { return Err(cargo_err(WILDCARD_ERROR_MESSAGE)); } diff --git a/src/tests/builders/dependency.rs b/src/tests/builders/dependency.rs index 8757e499845..d1c37efddfa 100644 --- a/src/tests/builders/dependency.rs +++ b/src/tests/builders/dependency.rs @@ -15,7 +15,7 @@ impl DependencyBuilder { explicit_name_in_toml: None, name: name.to_string(), registry: None, - version_req: u::EncodableCrateVersionReq(semver::VersionReq::parse(">= 0").unwrap()), + version_req: u::EncodableCrateVersionReq("> 0".to_string()), } } @@ -38,10 +38,7 @@ impl DependencyBuilder { /// Panics if the `version_req` string specified isn't a valid `semver::VersionReq`. #[track_caller] pub fn version_req(mut self, version_req: &str) -> Self { - self.version_req = u::EncodableCrateVersionReq( - semver::VersionReq::parse(version_req) - .expect("version req isn't a valid semver::VersionReq"), - ); + self.version_req = u::EncodableCrateVersionReq(version_req.to_string()); self } diff --git a/src/tests/builders/publish.rs b/src/tests/builders/publish.rs index 6ea7f5f0f2d..6066c0037dc 100644 --- a/src/tests/builders/publish.rs +++ b/src/tests/builders/publish.rs @@ -180,8 +180,7 @@ impl PublishBuilder { self } - /// Consume this builder to make the Put request body - pub fn body(self) -> Vec { + pub fn build(self) -> (String, Vec) { let new_crate = u::EncodableCrateUpload { name: u::EncodableCrateName(self.krate_name.clone()), vers: u::EncodableCrateVersion(self.version), @@ -209,7 +208,16 @@ impl PublishBuilder { links: None, }; - let json = serde_json::to_string(&new_crate).unwrap(); + (serde_json::to_string(&new_crate).unwrap(), self.tarball) + } + + /// Consume this builder to make the Put request body + pub fn body(self) -> Vec { + let (json, tarball) = self.build(); + PublishBuilder::create_publish_body(&json, &tarball) + } + + pub fn create_publish_body(json: &str, tarball: &[u8]) -> Vec { let mut body = Vec::new(); body.extend( [ @@ -223,7 +231,6 @@ impl PublishBuilder { ); body.extend(json.as_bytes().iter().cloned()); - let tarball = &self.tarball; body.extend(&[ tarball.len() as u8, (tarball.len() >> 8) as u8, diff --git a/src/tests/http-data/krate_publish_new_krate_with_broken_dependency_requirement b/src/tests/http-data/krate_publish_new_krate_with_broken_dependency_requirement new file mode 100644 index 00000000000..e42cd6d87de --- /dev/null +++ b/src/tests/http-data/krate_publish_new_krate_with_broken_dependency_requirement @@ -0,0 +1,69 @@ +[ + { + "request": { + "uri": "http://alexcrichton-test.s3.amazonaws.com/crates/new_dep/new_dep-1.0.0.crate", + "method": "PUT", + "headers": [ + [ + "host", + "alexcrichton-test.s3.amazonaws.com" + ], + [ + "authorization", + "AWS AKIAICL5IWUZYWWKA7JA:lH91IZJxBDr21vB3eSZXwXTQelk=" + ], + [ + "date", + "Fri, 15 Sep 2017 07:53:06 -0700" + ], + [ + "content-type", + "application/x-tar" + ], + [ + "accept-encoding", + "gzip" + ], + [ + "content-length", + "35" + ], + [ + "accept", + "*/*" + ] + ], + "body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA=" + }, + "response": { + "status": 200, + "headers": [ + [ + "Server", + "AmazonS3" + ], + [ + "content-length", + "0" + ], + [ + "x-amz-request-id", + "8190F375F5651D47" + ], + [ + "x-amz-id-2", + "PQEXq2S9IgVfG/u0l2cjHuWU8EDPlRSwxqai2oNPCz8TyX/CEwn2ydL+A1qnNK3Ng+FpC9ukVl8=" + ], + [ + "date", + "Fri, 15 Sep 2017 14:53:07 GMT" + ], + [ + "ETag", + "\"f9016ad360cebb4fe2e6e96e5949f022\"" + ] + ], + "body": "" + } + } +] \ No newline at end of file diff --git a/src/tests/krate/dependencies.rs b/src/tests/krate/dependencies.rs index 96d50cf0618..4a741721fc4 100644 --- a/src/tests/krate/dependencies.rs +++ b/src/tests/krate/dependencies.rs @@ -5,8 +5,8 @@ use cargo_registry::views::EncodableDependency; use http::StatusCode; #[derive(Deserialize)] -struct Deps { - dependencies: Vec, +pub struct Deps { + pub dependencies: Vec, } #[test] diff --git a/src/tests/krate/publish.rs b/src/tests/krate/publish.rs index 87f1f0b232f..f5870a03089 100644 --- a/src/tests/krate/publish.rs +++ b/src/tests/krate/publish.rs @@ -173,7 +173,9 @@ fn new_with_renamed_dependency() { #[test] fn new_krate_with_dependency() { - let (app, _, user, token) = TestApp::full().with_token(); + use super::dependencies::Deps; + + let (app, anon, user, token) = TestApp::full().with_token(); app.db(|conn| { // Insert a crate directly into the database so that new_dep can depend on it @@ -183,12 +185,56 @@ fn new_krate_with_dependency() { CrateBuilder::new("foo-dep", user.as_model().id).expect_build(conn); }); - let dependency = DependencyBuilder::new("foo-dep"); + let dependency = DependencyBuilder::new("foo-dep").version_req("1.0.0"); let crate_to_publish = PublishBuilder::new("new_dep") .version("1.0.0") .dependency(dependency); + token.enqueue_publish(crate_to_publish).good(); + + let dependencies = anon + .get::("/api/v1/crates/new_dep/1.0.0/dependencies") + .good() + .dependencies; + + assert_eq!(dependencies.len(), 1); + assert_eq!(dependencies[0].crate_id, "foo-dep"); + assert_eq!(dependencies[0].req, "1.0.0"); +} + +#[test] +fn new_krate_with_broken_dependency_requirement() { + let (app, _, user, token) = TestApp::full().with_token(); + + app.db(|conn| { + // Insert a crate directly into the database so that new_dep can depend on it + // The name choice of `foo-dep` is important! It has the property of + // name != canon_crate_name(name) and is a regression test for + // https://github.com/rust-lang/crates.io/issues/651 + CrateBuilder::new("foo-dep", user.as_model().id).expect_build(conn); + }); + + let dependency = DependencyBuilder::new("foo-dep").version_req("1.2.3"); + + let crate_to_publish = PublishBuilder::new("new_dep") + .version("1.0.0") + .dependency(dependency); + + // create a request body with `version_req: "broken"` + let (json, tarball) = crate_to_publish.build(); + let new_json = json.replace("\"version_req\":\"1.2.3\"", "\"version_req\":\"broken\""); + assert_ne!(json, new_json); + let body = PublishBuilder::create_publish_body(&new_json, &tarball); + + let response = token + .put::("/api/v1/crates/new", &body) + .good(); + + assert_eq!( + response, + json!({"errors": [{"detail": "invalid upload request: invalid value: string \"broken\", expected a valid version req at line 1 column 136"}]}) + ); } #[test] diff --git a/src/views/krate_publish.rs b/src/views/krate_publish.rs index 6592ca969e7..6a28f5c763e 100644 --- a/src/views/krate_publish.rs +++ b/src/views/krate_publish.rs @@ -41,7 +41,7 @@ pub struct EncodableCrateName(pub String); #[derive(Debug, Deref)] pub struct EncodableCrateVersion(pub semver::Version); #[derive(Debug, Deref)] -pub struct EncodableCrateVersionReq(pub semver::VersionReq); +pub struct EncodableCrateVersionReq(pub String); #[derive(Serialize, Debug, Deref, Default)] pub struct EncodableKeywordList(pub Vec); #[derive(Serialize, Debug, Deref)] @@ -152,7 +152,7 @@ impl<'de> Deserialize<'de> for EncodableCrateVersionReq { fn deserialize>(d: D) -> Result { let s = String::deserialize(d)?; match semver::VersionReq::parse(&s) { - Ok(v) => Ok(EncodableCrateVersionReq(v)), + Ok(_) => Ok(EncodableCrateVersionReq(s)), Err(..) => { let value = de::Unexpected::Str(&s); let expected = "a valid version req"; @@ -162,15 +162,6 @@ impl<'de> Deserialize<'de> for EncodableCrateVersionReq { } } -impl PartialEq for EncodableCrateVersionReq -where - semver::VersionReq: PartialEq, -{ - fn eq(&self, rhs: &T) -> bool { - self.0 == *rhs - } -} - impl<'de> Deserialize<'de> for EncodableKeywordList { fn deserialize>(d: D) -> Result { let inner = as Deserialize<'de>>::deserialize(d)?;