diff --git a/src/put/mod.rs b/src/put/mod.rs index 8accffa..773de7a 100644 --- a/src/put/mod.rs +++ b/src/put/mod.rs @@ -419,7 +419,7 @@ fn discover_changes(cwd: &Path) -> Result> { /// upload date. Matching on the digest makes `--force` the only way to /// re-upload an identical file, while a rebuilt file (new digest) never /// trips the guard. -#[derive(Debug, Clone, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] struct UploadRecord { target: String, file: String, @@ -461,12 +461,52 @@ fn upload_log_path() -> Result> { Ok(path) } -/// Load the upload log, empty when the file does not exist yet +/// Load the upload log, empty when the file does not exist yet. +/// +/// The log only backs the advisory duplicate-upload guard (Launchpad +/// re-verifies every upload), so a corrupt log must not abort the upload: +/// like the context configuration in [`crate::context::manager`], the corrupt +/// file is first backed up to `.bak` (best effort) so a later +/// [`record_upload`] cannot destroy it, then an empty log is used. fn load_upload_log(path: &Path) -> Vec { - std::fs::read_to_string(path) - .ok() - .and_then(|content| serde_json::from_str(&content).ok()) - .unwrap_or_default() + let content = match std::fs::read_to_string(path) { + Ok(content) => content, + Err(e) => { + if e.kind() != std::io::ErrorKind::NotFound { + log::error!( + "Cannot read upload log {}: {e}; continuing as if nothing had been uploaded", + path.display() + ); + } + return Vec::new(); + } + }; + match serde_json::from_str(&content) { + Ok(entries) => entries, + Err(e) => { + log::error!( + "Upload log {} is corrupt ({e}); ignoring its content, earlier uploads may not be detected as duplicates anymore", + path.display() + ); + backup_corrupt_log(path); + Vec::new() + } + } +} + +/// Back up a corrupt upload log (best effort) so a later +/// [`record_upload`] cannot silently destroy its content. +fn backup_corrupt_log(path: &Path) { + let mut os = path.as_os_str().to_os_string(); + os.push(".bak"); + let backup_path = PathBuf::from(os); + match std::fs::copy(path, &backup_path) { + Ok(_) => log::warn!("Corrupt upload log backed up to {}", backup_path.display()), + Err(e) => log::warn!( + "Could not back up corrupt upload log to {}: {e}", + backup_path.display() + ), + } } /// The previous upload of `record` from the log at `log`, if any (same @@ -484,7 +524,16 @@ fn find_previous_upload( fn record_upload(log: &Path, record: &UploadRecord) -> Result<(), Box> { let mut entries = load_upload_log(log); entries.push(record.clone()); - std::fs::write(log, serde_json::to_string_pretty(&entries)?)?; + // Write to a temporary file next to the log, then rename it over the + // log: rename is atomic within a filesystem, so a crash mid-write leaves + // the previous log intact instead of a truncated (corrupt) file. + // The temporary file is created with default permissions; the log holds + // no secrets and lives in the user's data directory, so the previous + // mode is not carried over (same choice as `Files::save_atomic`). + let tmp = log.with_extension("new"); + std::fs::write(&tmp, serde_json::to_string_pretty(&entries)?) + .map_err(|e| format!("cannot write '{}': {}", tmp.display(), e))?; + std::fs::rename(&tmp, log).map_err(|e| format!("cannot install '{}': {}", log.display(), e))?; Ok(()) } @@ -632,6 +681,65 @@ mod tests { assert!(find_previous_upload(&log, &rebuilt).unwrap().is_none()); } + /// A plain upload record for the log tests + fn upload_log_record(sha256: &str) -> UploadRecord { + UploadRecord { + target: "ppa:user/ppa".to_string(), + file: "hello_1.0-1_source.changes".to_string(), + sha256: sha256.to_string(), + date: "2025-09-16 12:00".to_string(), + } + } + + /// A corrupt log must not be silently reset: it is backed up byte for + /// byte, and the guard restarts from an empty log. + #[test] + fn corrupt_upload_log_is_backed_up_and_reset() { + let dir = tempfile::tempdir().unwrap(); + let log = dir.path().join("uploads.json"); + let corrupt = b"{ \"not\": valid json"; + std::fs::write(&log, corrupt).unwrap(); + + assert!(load_upload_log(&log).is_empty()); + + let backup = dir.path().join("uploads.json.bak"); + assert_eq!(std::fs::read(&backup).unwrap(), corrupt); + + // Recording after a corrupt log starts a fresh, parseable log + // without clobbering the backup + record_upload(&log, &upload_log_record("abc")).unwrap(); + assert_eq!(load_upload_log(&log), vec![upload_log_record("abc")]); + assert_eq!(std::fs::read(&backup).unwrap(), corrupt); + } + + /// `record_upload` leaves a log that parses back, with no temporary + /// file left behind + #[test] + fn recorded_upload_parses_back_atomically() { + let dir = tempfile::tempdir().unwrap(); + let log = dir.path().join("uploads.json"); + + record_upload(&log, &upload_log_record("abc")).unwrap(); + + assert_eq!(load_upload_log(&log), vec![upload_log_record("abc")]); + assert!(!dir.path().join("uploads.new").exists()); + } + + /// Recording a second upload appends to the log instead of replacing it + #[test] + fn record_over_existing_log_preserves_prior_entries() { + let dir = tempfile::tempdir().unwrap(); + let log = dir.path().join("uploads.json"); + + record_upload(&log, &upload_log_record("abc")).unwrap(); + record_upload(&log, &upload_log_record("def")).unwrap(); + + assert_eq!( + load_upload_log(&log), + vec![upload_log_record("abc"), upload_log_record("def")] + ); + } + /// A minimal in-memory changes file for the superseded-version check fn changes_fixture(version: &str) -> changes::ChangesFile { changes::ChangesFile {