Skip to content

Commit

Permalink
Merge pull request #6518 from Turbo87/build-metadata-block
Browse files Browse the repository at this point in the history
Prevent similar versions that only differ in build metadata from being published
  • Loading branch information
Turbo87 committed May 30, 2023
2 parents 0f95c06 + 456fb9b commit 5c68d55
Show file tree
Hide file tree
Showing 9 changed files with 359 additions and 5 deletions.
18 changes: 14 additions & 4 deletions src/models/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::util::errors::{cargo_err, AppResult};

use crate::models::{Crate, Dependency, User};
use crate::schema::*;
use crate::sql::split_part;

// Queryable has a custom implementation below
#[derive(Clone, Identifiable, Associations, Debug, Queryable, Deserialize, Serialize)]
Expand Down Expand Up @@ -168,14 +169,16 @@ impl NewVersion {
use diesel::{insert_into, select};

conn.transaction(|conn| {
let num_no_build = strip_build_metadata(&self.num);

let already_uploaded = versions
.filter(crate_id.eq(self.crate_id))
.filter(num.eq(&self.num));
.filter(split_part(num, "+", 1).eq(num_no_build));

if select(exists(already_uploaded)).get_result(conn)? {
return Err(cargo_err(&format_args!(
"crate version `{}` is already \
uploaded",
self.num
"crate version `{}` is already uploaded",
num_no_build
)));
}

Expand Down Expand Up @@ -219,6 +222,13 @@ fn validate_license_expr(s: &str) -> AppResult<()> {
Ok(())
}

fn strip_build_metadata(version: &str) -> &str {
version
.split_once('+')
.map(|parts| parts.0)
.unwrap_or(version)
}

#[cfg(test)]
mod tests {
use super::{validate_license_expr, TopVersions};
Expand Down
3 changes: 2 additions & 1 deletion src/sql.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use diesel::sql_types::{Date, Double, Interval, SingleValue, Text, Timestamp};
use diesel::sql_types::{Date, Double, Integer, Interval, SingleValue, Text, Timestamp};

sql_function!(#[aggregate] fn array_agg<T: SingleValue>(x: T) -> Array<T>);
sql_function!(fn canon_crate_name(x: Text) -> Text);
Expand All @@ -12,3 +12,4 @@ sql_function! {
sql_function!(fn floor(x: Double) -> Integer);
sql_function!(fn greatest<T: SingleValue>(x: T, y: T) -> T);
sql_function!(fn least<T: SingleValue>(x: T, y: T) -> T);
sql_function!(fn split_part(string: Text, delimiter: Text, n: Integer) -> Text);
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
[
{
"request": {
"uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo/foo-1.0.0+foo.crate",
"method": "PUT",
"headers": [
[
"accept",
"*/*"
],
[
"accept-encoding",
"gzip"
],
[
"content-length",
"35"
],
[
"content-type",
"application/gzip"
]
],
"body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA="
},
"response": {
"status": 200,
"headers": [],
"body": ""
}
},
{
"request": {
"uri": "http://alexcrichton-test.s3.amazonaws.com/3/f/foo",
"method": "PUT",
"headers": [
[
"accept",
"*/*"
],
[
"accept-encoding",
"gzip"
],
[
"content-length",
"148"
],
[
"content-type",
"text/plain"
]
],
"body": "{\"name\":\"foo\",\"vers\":\"1.0.0+foo\",\"deps\":[],\"cksum\":\"acb5604b126ac894c1eb11c4575bf2072fea61232a888e453770c79d7ed56419\",\"features\":{},\"yanked\":false}\n"
},
"response": {
"status": 200,
"headers": [],
"body": ""
}
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
[
{
"request": {
"uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo/foo-1.0.0-beta.1.crate",
"method": "PUT",
"headers": [
[
"accept",
"*/*"
],
[
"accept-encoding",
"gzip"
],
[
"content-length",
"35"
],
[
"content-type",
"application/gzip"
]
],
"body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA="
},
"response": {
"status": 200,
"headers": [],
"body": ""
}
},
{
"request": {
"uri": "http://alexcrichton-test.s3.amazonaws.com/3/f/foo",
"method": "PUT",
"headers": [
[
"accept",
"*/*"
],
[
"accept-encoding",
"gzip"
],
[
"content-length",
"151"
],
[
"content-type",
"text/plain"
]
],
"body": "{\"name\":\"foo\",\"vers\":\"1.0.0-beta.1\",\"deps\":[],\"cksum\":\"acb5604b126ac894c1eb11c4575bf2072fea61232a888e453770c79d7ed56419\",\"features\":{},\"yanked\":false}\n"
},
"response": {
"status": 200,
"headers": [],
"body": ""
}
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
[
{
"request": {
"uri": "http://alexcrichton-test.s3.amazonaws.com/crates/foo/foo-1.0.0+foo.crate",
"method": "PUT",
"headers": [
[
"accept",
"*/*"
],
[
"accept-encoding",
"gzip"
],
[
"content-length",
"35"
],
[
"content-type",
"application/gzip"
]
],
"body": "H4sIAAAAAAAA/+3AAQEAAACCIP+vbkhQwKsBLq+17wAEAAA="
},
"response": {
"status": 200,
"headers": [],
"body": ""
}
},
{
"request": {
"uri": "http://alexcrichton-test.s3.amazonaws.com/3/f/foo",
"method": "PUT",
"headers": [
[
"accept",
"*/*"
],
[
"accept-encoding",
"gzip"
],
[
"content-length",
"148"
],
[
"content-type",
"text/plain"
]
],
"body": "{\"name\":\"foo\",\"vers\":\"1.0.0+foo\",\"deps\":[],\"cksum\":\"acb5604b126ac894c1eb11c4575bf2072fea61232a888e453770c79d7ed56419\",\"features\":{},\"yanked\":false}\n"
},
"response": {
"status": 200,
"headers": [],
"body": ""
}
}
]
52 changes: 52 additions & 0 deletions src/tests/krate/publish.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::builders::{CrateBuilder, DependencyBuilder, PublishBuilder};
use crate::new_category;
use crate::util::insta::assert_yaml_snapshot;
use crate::util::{RequestHelper, TestApp};
use cargo_registry::controllers::krate::publish::{
missing_metadata_error_message, MISSING_RIGHTS_ERROR_MESSAGE, WILDCARD_ERROR_MESSAGE,
Expand Down Expand Up @@ -1035,3 +1036,54 @@ fn empty_payload() {
json!({ "errors": [{ "detail": "invalid metadata length" }] })
);
}

fn version_with_build_metadata(v1: &str, v2: &str, expected_error: &str) {
let (_app, _anon, _cookie, token) = TestApp::full().with_token();

let response = token.publish_crate(PublishBuilder::new("foo").version(v1));
assert_eq!(response.status(), StatusCode::OK);
assert_yaml_snapshot!(response.into_json(), {
".crate.created_at" => "[datetime]",
".crate.updated_at" => "[datetime]",
});

let response = token.publish_crate(PublishBuilder::new("foo").version(v2));
assert_eq!(response.status(), StatusCode::OK);
assert_eq!(
response.into_json(),
json!({ "errors": [{ "detail": expected_error }] })
);
}

#[test]
fn version_with_build_metadata_1() {
insta::with_settings!({ snapshot_suffix => "build_metadata_1" }, {
version_with_build_metadata(
"1.0.0+foo",
"1.0.0+bar",
"crate version `1.0.0` is already uploaded",
);
});
}

#[test]
fn version_with_build_metadata_2() {
insta::with_settings!({ snapshot_suffix => "build_metadata_2" }, {
version_with_build_metadata(
"1.0.0-beta.1",
"1.0.0-beta.1+2",
"crate version `1.0.0-beta.1` is already uploaded",
);
});
}

#[test]
fn version_with_build_metadata_3() {
insta::with_settings!({ snapshot_suffix => "build_metadata_3" }, {
version_with_build_metadata(
"1.0.0+foo",
"1.0.0",
"crate version `1.0.0` is already uploaded",
);
});
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
source: src/tests/krate/publish.rs
expression: response.into_json()
---
crate:
badges: ~
categories: ~
created_at: "[datetime]"
description: description
documentation: ~
downloads: 0
exact_match: false
homepage: ~
id: foo
keywords: ~
links:
owner_team: /api/v1/crates/foo/owner_team
owner_user: /api/v1/crates/foo/owner_user
owners: /api/v1/crates/foo/owners
reverse_dependencies: /api/v1/crates/foo/reverse_dependencies
version_downloads: /api/v1/crates/foo/downloads
versions: /api/v1/crates/foo/versions
max_stable_version: 1.0.0+foo
max_version: 1.0.0+foo
name: foo
newest_version: 1.0.0+foo
recent_downloads: ~
repository: ~
updated_at: "[datetime]"
versions: ~
warnings:
invalid_badges: []
invalid_categories: []
other: []

Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
---
source: src/tests/krate/publish.rs
expression: response.into_json()
---
crate:
badges: ~
categories: ~
created_at: "[datetime]"
description: description
documentation: ~
downloads: 0
exact_match: false
homepage: ~
id: foo
keywords: ~
links:
owner_team: /api/v1/crates/foo/owner_team
owner_user: /api/v1/crates/foo/owner_user
owners: /api/v1/crates/foo/owners
reverse_dependencies: /api/v1/crates/foo/reverse_dependencies
version_downloads: /api/v1/crates/foo/downloads
versions: /api/v1/crates/foo/versions
max_stable_version: ~
max_version: 1.0.0-beta.1
name: foo
newest_version: 1.0.0-beta.1
recent_downloads: ~
repository: ~
updated_at: "[datetime]"
versions: ~
warnings:
invalid_badges: []
invalid_categories: []
other: []

Loading

0 comments on commit 5c68d55

Please sign in to comment.