put: back up a corrupt uploads.json and write it atomically
A corrupt upload log was silently treated as 'never uploaded', disabling the duplicate-upload guard without a diagnostic, and record_upload truncated the file in place — a crash mid-write produced exactly that corrupt state. Parse failures now log an error, back the file up to uploads.json.bak (so a later successful upload cannot destroy the recoverable history) and continue with an empty log; the log itself is written to a temp file and renamed into place.
This commit is contained in:
+115
-7
@@ -419,7 +419,7 @@ fn discover_changes(cwd: &Path) -> Result<PathBuf, Box<dyn std::error::Error>> {
|
|||||||
/// upload date. Matching on the digest makes `--force` the only way to
|
/// upload date. Matching on the digest makes `--force` the only way to
|
||||||
/// re-upload an identical file, while a rebuilt file (new digest) never
|
/// re-upload an identical file, while a rebuilt file (new digest) never
|
||||||
/// trips the guard.
|
/// trips the guard.
|
||||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||||
struct UploadRecord {
|
struct UploadRecord {
|
||||||
target: String,
|
target: String,
|
||||||
file: String,
|
file: String,
|
||||||
@@ -461,12 +461,52 @@ fn upload_log_path() -> Result<PathBuf, Box<dyn std::error::Error>> {
|
|||||||
Ok(path)
|
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 `<path>.bak` (best effort) so a later
|
||||||
|
/// [`record_upload`] cannot destroy it, then an empty log is used.
|
||||||
fn load_upload_log(path: &Path) -> Vec<UploadRecord> {
|
fn load_upload_log(path: &Path) -> Vec<UploadRecord> {
|
||||||
std::fs::read_to_string(path)
|
let content = match std::fs::read_to_string(path) {
|
||||||
.ok()
|
Ok(content) => content,
|
||||||
.and_then(|content| serde_json::from_str(&content).ok())
|
Err(e) => {
|
||||||
.unwrap_or_default()
|
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
|
/// 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<dyn std::error::Error>> {
|
fn record_upload(log: &Path, record: &UploadRecord) -> Result<(), Box<dyn std::error::Error>> {
|
||||||
let mut entries = load_upload_log(log);
|
let mut entries = load_upload_log(log);
|
||||||
entries.push(record.clone());
|
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(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -632,6 +681,65 @@ mod tests {
|
|||||||
assert!(find_previous_upload(&log, &rebuilt).unwrap().is_none());
|
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
|
/// A minimal in-memory changes file for the superseded-version check
|
||||||
fn changes_fixture(version: &str) -> changes::ChangesFile {
|
fn changes_fixture(version: &str) -> changes::ChangesFile {
|
||||||
changes::ChangesFile {
|
changes::ChangesFile {
|
||||||
|
|||||||
Reference in New Issue
Block a user