From f6e5a867cf538f3ff7308e8eca1f8cd3a74fdadf Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 27 Sep 2023 11:56:36 +0200 Subject: [PATCH 1/4] controllers/krate/publish: Use `existing_crate` for upload size check There are a couple of cases to consider here: - if a krate with the same name exists, `existing_crate` will be `Some(_)` and the `max_upload_size` value is used. - if no krate with the same name exists yet, `existing_crate` will be `None` and the default size limits are used. - if no krate with the same name exists yet and there are two concurrent publish requests, the default size limits are used. this should be fine since there is no way to increase the size limits during publish or via the API in general. tl;dr there is a potential race condition here, but it doesn't matter for this specific case --- src/controllers/krate/publish.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index e9584be1bd9..2b5a0fd0d26 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -110,6 +110,21 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult maximums.max_upload_size { + return Err(cargo_err(&format_args!( + "max upload size is: {}", + maximums.max_upload_size + ))); + } + // Create a transaction on the database, if there are no errors, // commit the transactions to record a new or updated crate. conn.transaction(|conn| { @@ -177,21 +192,6 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult maximums.max_upload_size { - return Err(cargo_err(&format_args!( - "max upload size is: {}", - maximums.max_upload_size - ))); - } - // Read tarball from request let hex_cksum: String = Sha256::digest(&tarball_bytes).encode_hex(); From 1ffba9594098d78938f6bde74483bf65063e7994 Mon Sep 17 00:00:00 2001 From: Tobias Bieniek Date: Wed, 27 Sep 2023 12:22:47 +0200 Subject: [PATCH 2/4] controllers/krate/publish: Move `process_tarball()` call ahead of `INSERT` SQL queries This will allow us to use the data from the `Cargo.toml` file in the tarball instead of the JSON metadata blob. --- src/controllers/krate/publish.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index 2b5a0fd0d26..d21ff81ae6e 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -125,6 +125,14 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult AppResult Date: Wed, 27 Sep 2023 12:51:13 +0200 Subject: [PATCH 3/4] controllers/krate/publish: Use `description`, `license` and `license_file` fields from embedded `Cargo.toml` file ... instead of the metadata JSON blob --- src/controllers/krate/publish.rs | 47 +++++++++++++++++--------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/src/controllers/krate/publish.rs b/src/controllers/krate/publish.rs index d21ff81ae6e..b9e2bd28586 100644 --- a/src/controllers/krate/publish.rs +++ b/src/controllers/krate/publish.rs @@ -51,25 +51,6 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult) -> bool { - s.map_or(true, String::is_empty) - } - - // It can have up to three elements per below conditions. - let mut missing = Vec::with_capacity(3); - - if empty(metadata.description.as_ref()) { - missing.push("description"); - } - if empty(metadata.license.as_ref()) && empty(metadata.license_file.as_ref()) { - missing.push("license"); - } - if !missing.is_empty() { - let message = missing_metadata_error_message(&missing); - return Err(cargo_err(&message)); - } - conduit_compat(move || { let conn = &mut *app.db_write()?; @@ -133,6 +114,28 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult) -> bool { + s.map_or(true, String::is_empty) + } + + // It can have up to three elements per below conditions. + let mut missing = Vec::with_capacity(3); + if empty(description.as_ref()) { + missing.push("description"); + } + if empty(license.as_ref()) && empty(license_file.as_ref()) { + missing.push("license"); + } + if !missing.is_empty() { + let message = missing_metadata_error_message(&missing); + return Err(cargo_err(&message)); + } + // Create a transaction on the database, if there are no errors, // commit the transactions to record a new or updated crate. conn.transaction(|conn| { @@ -159,7 +162,7 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult AppResult AppResult Date: Wed, 27 Sep 2023 12:57:54 +0200 Subject: [PATCH 4/4] views/krate_publish: Remove obsolete `description`, `license`, and `license_file` fields These fields are no longer read by our server, so we can skip the deserialization step. --- src/tests/builders/publish.rs | 3 --- .../all__krate__publish__categories__too_many_categories.snap | 2 +- .../all__krate__publish__keywords__bad_keywords-2.snap | 2 +- .../all__krate__publish__keywords__bad_keywords-3.snap | 2 +- .../snapshots/all__krate__publish__keywords__bad_keywords.snap | 2 +- .../all__krate__publish__keywords__too_many_keywords.snap | 2 +- src/views/krate_publish.rs | 3 --- 7 files changed, 5 insertions(+), 11 deletions(-) diff --git a/src/tests/builders/publish.rs b/src/tests/builders/publish.rs index cb7c832adfe..6a344a62531 100644 --- a/src/tests/builders/publish.rs +++ b/src/tests/builders/publish.rs @@ -142,7 +142,6 @@ impl PublishBuilder { vers: u::EncodableCrateVersion(self.version.clone()), features: self.features.clone(), deps: self.deps.clone(), - description: self.desc.clone(), homepage: None, documentation: self.doc_url.clone(), readme: self.readme, @@ -161,8 +160,6 @@ impl PublishBuilder { .map(u::EncodableCategory) .collect(), ), - license: self.license.clone(), - license_file: self.license_file.clone(), repository: None, links: None, }; diff --git a/src/tests/krate/publish/snapshots/all__krate__publish__categories__too_many_categories.snap b/src/tests/krate/publish/snapshots/all__krate__publish__categories__too_many_categories.snap index 8233e425e68..f1300f6967d 100644 --- a/src/tests/krate/publish/snapshots/all__krate__publish__categories__too_many_categories.snap +++ b/src/tests/krate/publish/snapshots/all__krate__publish__categories__too_many_categories.snap @@ -5,7 +5,7 @@ expression: response.into_json() { "errors": [ { - "detail": "invalid upload request: invalid length 6, expected at most 5 categories per crate at line 1 column 219" + "detail": "invalid upload request: invalid length 6, expected at most 5 categories per crate at line 1 column 191" } ] } diff --git a/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords-2.snap b/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords-2.snap index d72df4ab2c3..82ef682c51d 100644 --- a/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords-2.snap +++ b/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords-2.snap @@ -5,7 +5,7 @@ expression: response.into_json() { "errors": [ { - "detail": "invalid upload request: invalid value: string \"?@?%\", expected a valid keyword specifier at line 1 column 178" + "detail": "invalid upload request: invalid value: string \"?@?%\", expected a valid keyword specifier at line 1 column 150" } ] } diff --git a/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords-3.snap b/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords-3.snap index 87a68ab7391..f55b4d0f859 100644 --- a/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords-3.snap +++ b/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords-3.snap @@ -5,7 +5,7 @@ expression: response.into_json() { "errors": [ { - "detail": "invalid upload request: invalid value: string \"áccênts\", expected a valid keyword specifier at line 1 column 183" + "detail": "invalid upload request: invalid value: string \"áccênts\", expected a valid keyword specifier at line 1 column 155" } ] } diff --git a/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords.snap b/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords.snap index 00eb6ecb4ec..287958e9cb0 100644 --- a/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords.snap +++ b/src/tests/krate/publish/snapshots/all__krate__publish__keywords__bad_keywords.snap @@ -5,7 +5,7 @@ expression: response.into_json() { "errors": [ { - "detail": "invalid upload request: invalid length 29, expected a keyword with less than 20 characters at line 1 column 203" + "detail": "invalid upload request: invalid length 29, expected a keyword with less than 20 characters at line 1 column 175" } ] } diff --git a/src/tests/krate/publish/snapshots/all__krate__publish__keywords__too_many_keywords.snap b/src/tests/krate/publish/snapshots/all__krate__publish__keywords__too_many_keywords.snap index 1590ab10118..45130bd6de1 100644 --- a/src/tests/krate/publish/snapshots/all__krate__publish__keywords__too_many_keywords.snap +++ b/src/tests/krate/publish/snapshots/all__krate__publish__keywords__too_many_keywords.snap @@ -5,7 +5,7 @@ expression: response.into_json() { "errors": [ { - "detail": "invalid upload request: invalid length 6, expected at most 5 keywords per crate at line 1 column 203" + "detail": "invalid upload request: invalid length 6, expected at most 5 keywords per crate at line 1 column 175" } ] } diff --git a/src/views/krate_publish.rs b/src/views/krate_publish.rs index e61769bf924..4c1c94be9eb 100644 --- a/src/views/krate_publish.rs +++ b/src/views/krate_publish.rs @@ -18,7 +18,6 @@ pub struct PublishMetadata { pub vers: EncodableCrateVersion, pub deps: Vec, pub features: BTreeMap>, - pub description: Option, pub homepage: Option, pub documentation: Option, pub readme: Option, @@ -27,8 +26,6 @@ pub struct PublishMetadata { pub keywords: EncodableKeywordList, #[serde(default)] pub categories: EncodableCategoryList, - pub license: Option, - pub license_file: Option, pub repository: Option, #[serde(default)] pub links: Option,