Merge pull request #6848 from Turbo87/fix-readme-parsing

Fix `readme` field parsing of `Cargo.toml` files

Author: Turbo87

Cargo.lock

@@ -8,6 +8,7 @@ edition = "2021"
 builder = []
 
 [dependencies]
+cargo_toml = "=0.15.2"
 derive_deref = "=1.1.1"
 flate2 = "=1.0.26"
 semver = { version = "=1.0.18", features = ["serde"] }

crates_io_tarball/Cargo.toml

@@ -103,6 +103,7 @@ pub fn process_tarball<R: Read>(
 mod tests {
     use super::process_tarball;
     use crate::TarballBuilder;
+    use cargo_toml::OptionalFile;
     use std::path::Path;
 
     #[test]
@@ -170,7 +171,7 @@ repository = "https://github.com/foo/bar"
         let limit = 512 * 1024 * 1024;
         let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, limit));
         let manifest = assert_some!(tarball_info.manifest);
-        assert_some_eq!(manifest.package.readme, Path::new("README.md"));
+        assert_some_eq!(manifest.package.readme.as_path(), Path::new("README.md"));
         assert_some_eq!(manifest.package.repository, "https://github.com/foo/bar");
         assert_some_eq!(manifest.package.rust_version, "1.59");
     }
@@ -191,4 +192,37 @@ repository = "https://github.com/foo/bar"
         let manifest = assert_some!(tarball_info.manifest);
         assert_some_eq!(manifest.package.rust_version, "1.23");
     }
+
+    #[test]
+    fn process_tarball_test_manifest_with_default_readme() {
+        let tarball = TarballBuilder::new("foo", "0.0.1")
+            .add_raw_manifest(
+                br#"
+                [package]
+                "#,
+            )
+            .build();
+
+        let limit = 512 * 1024 * 1024;
+        let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, limit));
+        let manifest = assert_some!(tarball_info.manifest);
+        assert_matches!(manifest.package.readme, OptionalFile::Flag(true));
+    }
+
+    #[test]
+    fn process_tarball_test_manifest_with_boolean_readme() {
+        let tarball = TarballBuilder::new("foo", "0.0.1")
+            .add_raw_manifest(
+                br#"
+                [package]
+                readme = false
+                "#,
+            )
+            .build();
+
+        let limit = 512 * 1024 * 1024;
+        let tarball_info = assert_ok!(process_tarball("foo-0.0.1", &*tarball, limit));
+        let manifest = assert_some!(tarball_info.manifest);
+        assert_matches!(manifest.package.readme, OptionalFile::Flag(false));
+    }
 }

crates_io_tarball/src/lib.rs

@@ -1,6 +1,6 @@
+use cargo_toml::OptionalFile;
 use derive_deref::Deref;
 use serde::{de, Deserialize, Deserializer};
-use std::path::PathBuf;
 
 #[derive(Debug, Deserialize)]
 pub struct Manifest {
@@ -11,7 +11,8 @@ pub struct Manifest {
 #[derive(Debug, Deserialize)]
 #[serde(rename_all = "kebab-case")]
 pub struct Package {
-    pub readme: Option<PathBuf>,
+    #[serde(default)]
+    pub readme: OptionalFile,
     pub repository: Option<String>,
     pub rust_version: Option<RustVersion>,
 }

crates_io_tarball/src/manifest.rs

@@ -181,11 +181,13 @@ fn render_pkg_readme<R: Read>(mut archive: Archive<R>, pkg_name: &str) -> anyhow
     };
 
     let rendered = {
-        let readme_path = manifest
-            .package
-            .readme
-            .unwrap_or_else(|| Path::new("README.md").to_path_buf());
-        let path = Path::new(pkg_name).join(&readme_path);
+        let readme = manifest.package.readme;
+        if !readme.is_some() {
+            return Ok("".to_string());
+        }
+
+        let readme_path = readme.as_path().unwrap_or_else(|| Path::new("README.md"));
+        let path = Path::new(pkg_name).join(readme_path);
         let contents = find_file_by_path(&mut entries, Path::new(&path))
             .with_context(|| format!("Failed to read {} file", readme_path.display()))?;
 

src/admin/render_readmes.rs

@@ -1185,3 +1185,29 @@ fn version_with_build_metadata_3() {
         );
     });
 }
+
+#[test]
+fn boolean_readme() {
+    // see https://github.com/rust-lang/crates.io/issues/6847
+
+    let (_app, _anon, _cookie, token) = TestApp::full().with_token();
+
+    let tarball = TarballBuilder::new("foo", "1.0.0")
+        .add_raw_manifest(
+            br#"[package]
+            name = "foo"
+            version = "1.0.0"
+            rust-version = "1.69"
+            readme = false"#,
+        )
+        .build();
+
+    let response =
+        token.publish_crate(PublishBuilder::new("foo").version("1.0.0").tarball(tarball));
+    assert_eq!(response.status(), StatusCode::OK);
+
+    let response = token.get::<()>("/api/v1/crates/foo/1.0.0");
+    assert_eq!(response.status(), StatusCode::OK);
+    let json = response.into_json();
+    assert_some_eq!(json["version"]["rust_version"].as_str(), "1.69");
+}