Auto merge of #11665 - ehuss:cargo-ok-truncated, r=epage

Handle .cargo-ok being truncated

This fixes an issue where if the `.cargo-ok` file is truncated then Cargo will get stuck with being unable to extract the package. Creating the `.cargo-ok` file uses `create_new` which will fail if the file exists. If the file gets created, but there is a failure to flush it, or if the filesystem otherwise gets corrupted, then the `create_new` step will fail with `File exists`.

The solution here is to delete the cache directory if the `.cargo-ok` file is truncated, as there may be some uncertainty of the validity of the cache. This also adds better error handling if there is some problem opening the `.cargo-ok` file instead of ignoring errors.

Closes #11638

Author: bors

src/cargo/sources/registry/mod.rs

@@ -162,12 +162,12 @@ use std::borrow::Cow;
 use std::collections::BTreeMap;
 use std::collections::HashSet;
 use std::fs::{File, OpenOptions};
-use std::io::Write;
+use std::io::{self, Write};
 use std::path::{Path, PathBuf};
 use std::task::Poll;
 
 use anyhow::Context as _;
-use cargo_util::paths::exclude_from_backups_and_indexing;
+use cargo_util::paths::{self, exclude_from_backups_and_indexing};
 use flate2::read::GzDecoder;
 use log::debug;
 use semver::Version;
@@ -619,15 +619,22 @@ impl<'cfg> RegistrySource<'cfg> {
         // unpacked.
         let package_dir = format!("{}-{}", pkg.name(), pkg.version());
         let dst = self.src_path.join(&package_dir);
-        dst.create_dir()?;
         let path = dst.join(PACKAGE_SOURCE_LOCK);
         let path = self.config.assert_package_cache_locked(&path);
         let unpack_dir = path.parent().unwrap();
-        if let Ok(meta) = path.metadata() {
-            if meta.len() > 0 {
-                return Ok(unpack_dir.to_path_buf());
+        match path.metadata() {
+            Ok(meta) if meta.len() > 0 => return Ok(unpack_dir.to_path_buf()),
+            Ok(_meta) => {
+                // The file appears to be corrupted. Perhaps it failed to flush,
+                // or the filesystem was corrupted in some way. To be safe, let's
+                // assume something is wrong and clear it and start over.
+                log::warn!("unexpected length of {path:?}, clearing cache");
+                paths::remove_dir_all(dst.as_path_unlocked())?;
             }
+            Err(e) if e.kind() == io::ErrorKind::NotFound => {}
+            Err(e) => anyhow::bail!("failed to access package completion {path:?}: {e}"),
         }
+        dst.create_dir()?;
         let mut tar = {
             let size_limit = max_unpack_size(tarball.metadata()?.len());
             let gz = GzDecoder::new(tarball);

tests/testsuite/registry.rs

@@ -2866,3 +2866,48 @@ required by package `foo v0.1.0 ([ROOT]/foo)`
         .with_status(101)
         .run();
 }
+
+#[cargo_test]
+fn corrupted_ok_overwritten() {
+    // Checks what happens if .cargo-ok gets truncated, such as if the file is
+    // created, but the flush/close is interrupted.
+    Package::new("bar", "1.0.0").publish();
+    let p = project()
+        .file(
+            "Cargo.toml",
+            r#"
+                [package]
+                name = "foo"
+                version = "0.1.0"
+
+                [dependencies]
+                bar = "1"
+            "#,
+        )
+        .file("src/lib.rs", "")
+        .build();
+    p.cargo("fetch")
+        .with_stderr(
+            "\
+[UPDATING] `dummy-registry` index
+[DOWNLOADING] crates ...
+[DOWNLOADED] bar v1.0.0 (registry `dummy-registry`)
+",
+        )
+        .run();
+    let ok = glob::glob(
+        paths::home()
+            .join(".cargo/registry/src/*/bar-1.0.0/.cargo-ok")
+            .to_str()
+            .unwrap(),
+    )
+    .unwrap()
+    .next()
+    .unwrap()
+    .unwrap();
+    // Simulate cargo being interrupted, or filesystem corruption.
+    fs::write(&ok, "").unwrap();
+    assert_eq!(fs::read_to_string(&ok).unwrap(), "");
+    p.cargo("fetch").with_stderr("").run();
+    assert_eq!(fs::read_to_string(&ok).unwrap(), "ok");
+}