Skip upload for empty README files (#6794)

Author: Turbo87

src/admin/render_readmes.rs

@@ -114,15 +114,16 @@ pub fn run(opts: Opts) -> anyhow::Result<()> {
             let handle = thread::spawn::<_, anyhow::Result<()>>(move || {
                 println!("[{}-{}] Rendering README...", krate_name, version.num);
                 let readme = get_readme(base_config.uploader(), &client, &version, &krate_name)?;
-
-                let rt = tokio::runtime::Builder::new_current_thread()
-                    .enable_all()
-                    .build()
-                    .context("Failed to initialize tokio runtime")
-                    .unwrap();
-
-                rt.block_on(storage.upload_readme(&krate_name, &version.num, readme.into()))
-                    .context("Failed to upload rendered README file to S3")?;
+                if !readme.is_empty() {
+                    let rt = tokio::runtime::Builder::new_current_thread()
+                        .enable_all()
+                        .build()
+                        .context("Failed to initialize tokio runtime")
+                        .unwrap();
+
+                    rt.block_on(storage.upload_readme(&krate_name, &version.num, readme.into()))
+                        .context("Failed to upload rendered README file to S3")?;
+                }
 
                 Ok(())
             });

src/controllers/krate/publish.rs

@@ -236,16 +236,18 @@ pub async fn publish(app: AppState, req: BytesRequest) -> AppResult<Json<GoodCra
             let pkg_path_in_vcs = tarball_info.vcs_info.map(|info| info.path_in_vcs);
 
             if let Some(readme) = new_crate.readme {
-                Job::render_and_upload_readme(
-                    version.id,
-                    readme,
-                    new_crate
-                        .readme_file
-                        .unwrap_or_else(|| String::from("README.md")),
-                    repo,
-                    pkg_path_in_vcs,
-                )
-                .enqueue_with_priority(conn, PRIORITY_RENDER_README)?;
+                if !readme.is_empty() {
+                    Job::render_and_upload_readme(
+                        version.id,
+                        readme,
+                        new_crate
+                            .readme_file
+                            .unwrap_or_else(|| String::from("README.md")),
+                        repo,
+                        pkg_path_in_vcs,
+                    )
+                    .enqueue_with_priority(conn, PRIORITY_RENDER_README)?;
+                }
             }
 
             // Upload crate tarball

src/storage.rs

@@ -341,14 +341,15 @@ mod tests {
     async fn upload_readme() {
         let s = Storage::from_config(&StorageConfig::InMemory);
 
-        s.upload_readme("foo", "1.2.3", Bytes::new()).await.unwrap();
+        let bytes = Bytes::from_static(b"hello world");
+        s.upload_readme("foo", "1.2.3", bytes.clone())
+            .await
+            .unwrap();
 
         let expected_files = vec!["readmes/foo/foo-1.2.3.html"];
         assert_eq!(stored_files(&s.store).await, expected_files);
 
-        s.upload_readme("foo", "2.0.0+foo", Bytes::new())
-            .await
-            .unwrap();
+        s.upload_readme("foo", "2.0.0+foo", bytes).await.unwrap();
 
         let expected_files = vec![
             "readmes/foo/foo-1.2.3.html",

src/tests/http-data/krate_publish_new_krate_with_empty_readme.json

@@ -0,0 +1,85 @@
+[
+  {
+    "request": {
+      "uri": "http://127.0.0.1:19000/crates-index-test/fo/o_/foo_readme",
+      "method": "PUT",
+      "headers": [
+        [
+          "accept",
+          "*/*"
+        ],
+        [
+          "accept-encoding",
+          "gzip"
+        ],
+        [
+          "content-length",
+          "151"
+        ],
+        [
+          "content-type",
+          "text/plain"
+        ]
+      ],
+      "body": "{\"name\":\"foo_readme\",\"vers\":\"1.0.0\",\"deps\":[],\"cksum\":\"acb5604b126ac894c1eb11c4575bf2072fea61232a888e453770c79d7ed56419\",\"features\":{},\"yanked\":false}\n"
+    },
+    "response": {
+      "status": 200,
+      "headers": [
+        [
+          "accept-ranges",
+          "bytes"
+        ],
+        [
+          "content-length",
+          "0"
+        ],
+        [
+          "content-security-policy",
+          "block-all-mixed-content"
+        ],
+        [
+          "date",
+          "Tue, 13 Jun 2023 23:04:33 GMT"
+        ],
+        [
+          "etag",
+          "\"cf380f14a440ced148f97ac8a6d0bc0d\""
+        ],
+        [
+          "server",
+          "MinIO"
+        ],
+        [
+          "strict-transport-security",
+          "max-age=31536000; includeSubDomains"
+        ],
+        [
+          "vary",
+          "Accept-Encoding"
+        ],
+        [
+          "vary",
+          "Origin"
+        ],
+        [
+          "x-amz-id-2",
+          "dd9025bab4ad464b049177c95eb6ebf374d3b3fd1af9251148b658df7ac2e3e8"
+        ],
+        [
+          "x-amz-request-id",
+          "17685A6A60B5472D"
+        ],
+        [
+          "x-content-type-options",
+          "nosniff"
+        ],
+        [
+          "x-xss-protection",
+          "1; mode=block"
+        ]
+      ],
+      "body": ""
+    }
+  }
+]

src/tests/krate/publish.rs

@@ -634,7 +634,7 @@ fn new_krate_dependency_missing() {
 fn new_krate_with_readme() {
     let (app, _, _, token) = TestApp::full().with_token();
 
-    let crate_to_publish = PublishBuilder::new("foo_readme").readme("");
+    let crate_to_publish = PublishBuilder::new("foo_readme").readme("hello world");
     let json = token.publish_crate(crate_to_publish).good();
 
     assert_eq!(json.krate.name, "foo_readme");
@@ -650,13 +650,27 @@ fn new_krate_with_readme() {
     );
 }
 
+#[test]
+fn new_krate_with_empty_readme() {
+    let (app, _, _, token) = TestApp::full().with_token();
+
+    let crate_to_publish = PublishBuilder::new("foo_readme").readme("");
+    let json = token.publish_crate(crate_to_publish).good();
+
+    assert_eq!(json.krate.name, "foo_readme");
+    assert_eq!(json.krate.max_version, "1.0.0");
+
+    let files = app.stored_files();
+    assert_eq!(files, vec!["crates/foo_readme/foo_readme-1.0.0.crate"]);
+}
+
 #[test]
 fn new_krate_with_readme_and_plus_version() {
     let (app, _, _, token) = TestApp::full().with_token();
 
     let crate_to_publish = PublishBuilder::new("foo_readme")
         .version("1.0.0+foo")
-        .readme("");
+        .readme("hello world");
     let json = token.publish_crate(crate_to_publish).good();
 
     assert_eq!(json.krate.name, "foo_readme");

src/worker/readmes.rs

@@ -24,6 +24,9 @@ pub fn perform_render_and_upload_readme(
     info!(?version_id, "Rendering README");
 
     let rendered = text_to_html(text, readme_path, base_url, pkg_path_in_vcs);
+    if rendered.is_empty() {
+        return Ok(());
+    }
 
     conn.transaction(|conn| {
         Version::record_readme_rendering(version_id, conn)?;