Skip to content

Commit e81f172

Browse files
authoredMay 27, 2021
Avoid zero-mtime files (#250)
With rust-lang/cargo#9512 discovered this is, uh, odd fallout of the "deterministic" header mode in this crate. This is an unintended consequence of the deterministic header mode which should ideally be fixable. This commit changes the `tar` crate to use a different constant than 0 when creating archives with the deterministic header mode (specifically 123456789: Nov 29, 1973). It also will now refuse to create files with a 0 mtime, instead resetting the mtime to 1 which should help the mtime be nonzero so tools like lldb don't accidentally think it's zero.
1 parent 7f2a355 commit e81f172

File tree

3 files changed

+39
-2
lines changed

3 files changed

+39
-2
lines changed
 

Diff for: ‎src/entry.rs

+7
Original file line numberDiff line numberDiff line change
@@ -610,6 +610,13 @@ impl<'a> EntryFields<'a> {
610610

611611
if self.preserve_mtime {
612612
if let Ok(mtime) = self.header.mtime() {
613+
// For some more information on this see the comments in
614+
// `Header::fill_platform_from`, but the general idea is that
615+
// we're trying to avoid 0-mtime files coming out of archives
616+
// since some tools don't ingest them well. Perhaps one day
617+
// when Cargo stops working with 0-mtime archives we can remove
618+
// this.
619+
let mtime = if mtime == 0 { 1 } else { mtime };
613620
let mtime = FileTime::from_unix_time(mtime as i64, 0);
614621
filetime::set_file_handle_times(&f, Some(mtime), Some(mtime)).map_err(|e| {
615622
TarError::new(&format!("failed to set mtime for `{}`", dst.display()), e)

Diff for: ‎src/header.rs

+10-1
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,16 @@ impl Header {
734734
self.set_mode(meta.mode() as u32);
735735
}
736736
HeaderMode::Deterministic => {
737-
self.set_mtime(0);
737+
// We could in theory set the mtime to zero here, but not all
738+
// tools seem to behave well when ingesting files with a 0
739+
// timestamp. For example rust-lang/cargo#9512 shows that lldb
740+
// doesn't ingest files with a zero timestamp correctly.
741+
//
742+
// We just need things to be deterministic here so just pick
743+
// something that isn't zero. This time, chosen after careful
744+
// deliberation, corresponds to Nov 29, 1973.
745+
self.set_mtime(123456789);
746+
738747
self.set_uid(0);
739748
self.set_gid(0);
740749

Diff for: ‎tests/all.rs

+22-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use std::iter::repeat;
1111
use std::path::{Path, PathBuf};
1212

1313
use filetime::FileTime;
14-
use tar::{Archive, Builder, EntryType, Header};
14+
use tar::{Archive, Builder, EntryType, Header, HeaderMode};
1515
use tempfile::{Builder as TempBuilder, TempDir};
1616

1717
macro_rules! t {
@@ -645,6 +645,27 @@ fn file_times() {
645645
assert_eq!(atime.nanoseconds(), 0);
646646
}
647647

648+
#[test]
649+
fn zero_file_times() {
650+
let td = t!(TempBuilder::new().prefix("tar-rs").tempdir());
651+
652+
let mut ar = Builder::new(Vec::new());
653+
ar.mode(HeaderMode::Deterministic);
654+
let path = td.path().join("tmpfile");
655+
t!(File::create(&path));
656+
t!(ar.append_path_with_name(&path, "a"));
657+
658+
let data = t!(ar.into_inner());
659+
let mut ar = Archive::new(&data[..]);
660+
assert!(ar.unpack(td.path()).is_ok());
661+
662+
let meta = fs::metadata(td.path().join("a")).unwrap();
663+
let mtime = FileTime::from_last_modification_time(&meta);
664+
let atime = FileTime::from_last_access_time(&meta);
665+
assert!(mtime.unix_seconds() != 0);
666+
assert!(atime.unix_seconds() != 0);
667+
}
668+
648669
#[test]
649670
fn backslash_treated_well() {
650671
// Insert a file into an archive with a backslash

0 commit comments

Comments
 (0)
Please sign in to comment.