Revert "Apply PAX size overrides to entry headers" (#114)

Signed-off-by: William Woodruff <william@yossarian.net>

Author: woodruffw

CHANGELOG.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## 0.7.0
+
+* Remove all unsafe code from the crate
+* New API: `Entry::effective_size` returns the effective size of an archive entry,
+  including any pax `size` records that apply to it
+* Changed: `Header::entry_size` is now `Header::raw_entry_size`
+* Changed: `Header::size` is now `Header::raw_file_size`
+
 ## 0.5.6
 
 * Fixed a parser desynchronization vulnerability when reading tar archives that

Cargo.lock

@@ -1,6 +1,6 @@
 [package]
 name = "astral-tokio-tar"
-version = "0.6.4"
+version = "0.7.0"
 authors = [
     "Alex Crichton <alex@alexcrichton.com>",
     "dignifiedquire <me@dignifiequire.com>",

Cargo.toml

@@ -18,8 +18,8 @@ fn main() {
             println!("-------------------------- Entry {}", i);
             let mut f = file.unwrap();
             println!("path: {}", f.path().unwrap().display());
-            println!("size: {}", f.header().size().unwrap());
-            println!("entry size: {}", f.header().entry_size().unwrap());
+            println!("raw size: {}", f.header().raw_file_size().unwrap());
+            println!("raw entry size: {}", f.header().raw_entry_size().unwrap());
             println!("link name: {:?}", f.link_name().unwrap());
             println!("file type: {:#x}", f.header().entry_type().as_byte());
             println!("mode: {:#o}", f.header().mode().unwrap());

examples/raw_list.rs

@@ -236,6 +236,11 @@ impl<R: Read + Unpin> Archive<R> {
     /// sequence. If entries are processed out of sequence (from what the
     /// stream returns), then the contents read for each entry may be
     /// corrupted.
+    ///
+    /// **IMPORTANT**: Most users want [`Self::entries`], not this API.
+    /// This API returns every *physical* entry in the archive, rather
+    /// than their composed "logical" entries. It should only be used
+    /// for low-level diagnostic parsing.
     pub fn entries_raw(&mut self) -> io::Result<RawEntries<R>> {
         if self.inner.pax_only {
             return Err(other("raw entries are not supported by pax-only mode"));
@@ -721,7 +726,7 @@ fn poll_next_raw<R: Read + Unpin>(
     let mut header = current_header.take().unwrap();
 
     // note when pax extensions are available, the size from the header will be ignored
-    let mut size = header.entry_size()?;
+    let mut size = header.raw_entry_size()?;
     let mut pax_username = None;
     let mut pax_groupname = None;
 
@@ -761,7 +766,6 @@ fn poll_next_raw<R: Read + Unpin>(
                         .value()
                         .map_err(|_e| other("failed to parse pax size as string"))?;
                     size = parse_pax_decimal(size_str, "failed to parse pax size")?;
-                    header.set_size(size);
                 }
 
                 "uid" => {

src/archive.rs

@@ -342,6 +342,14 @@ impl<R: Read + Unpin> Entry<R> {
         &self.fields.header
     }
 
+    /// Returns the total size of this entry's data in bytes.
+    ///
+    /// Unlike [`Header::raw_entry_size`], this includes any `size` override from PAX
+    /// extensions describing this entry.
+    pub fn effective_size(&self) -> u64 {
+        self.fields.size
+    }
+
     /// Returns the starting position, in bytes, of the header of this entry in
     /// the archive.
     ///

src/entry.rs

@@ -315,13 +315,17 @@ impl Header {
 
     /// Returns the size of entry's data this header represents.
     ///
-    /// This is different from `Header::size` for sparse files, which have
-    /// some longer `size()` but shorter `entry_size()`. The `entry_size()`
-    /// listed here should be the number of bytes in the archive this header
-    /// describes.
+    /// This is different from [`Header::raw_size`] for sparse files, which have
+    /// some longer [`Header::raw_size`] but shorter [`Header::raw_entry_size`].
+    /// The [`Header::raw_entry_size`] listed here should be the number of bytes in the
+    /// archive this header describes.
     ///
     /// May return an error if the field is corrupted.
-    pub fn entry_size(&self) -> io::Result<u64> {
+    ///
+    /// **IMPORTANT**: This does not account for PAX `size` overrides.
+    /// Use [`Entry::effective_size`](crate::Entry::effective_size) to obtain
+    /// the effective size of an archive entry.
+    pub fn raw_entry_size(&self) -> io::Result<u64> {
         num_field_wrapper_from(&self.as_old().size).map_err(|err| {
             io::Error::new(
                 err.kind(),
@@ -330,16 +334,21 @@ impl Header {
         })
     }
 
-    /// Returns the file size this header represents.
+    /// Returns the file size this header represents, adjusted for any "real size"
+    /// if the header is a GNU sparse header.
     ///
     /// May return an error if the field is corrupted.
-    pub fn size(&self) -> io::Result<u64> {
+    ///
+    /// **IMPORTANT**: This does not account for PAX `size` overrides. Use
+    /// [`Entry::effective_size`](crate::Entry::effective_size) to obtain the effective size of an
+    /// archive entry.
+    pub fn raw_file_size(&self) -> io::Result<u64> {
         if self.entry_type().is_gnu_sparse() {
             self.as_gnu()
                 .ok_or_else(|| other("sparse header was not a gnu header"))
                 .and_then(|h| h.real_size())
         } else {
-            self.entry_size()
+            self.raw_entry_size()
         }
     }
 
@@ -871,10 +880,10 @@ impl Header {
     }
 
     fn debug_fields(&self, b: &mut fmt::DebugStruct) {
-        if let Ok(entry_size) = self.entry_size() {
+        if let Ok(entry_size) = self.raw_entry_size() {
             b.field("entry_size", &entry_size);
         }
-        if let Ok(size) = self.size() {
+        if let Ok(size) = self.raw_file_size() {
             b.field("size", &size);
         }
         if let Ok(path) = self.path() {

src/header.rs

@@ -194,7 +194,7 @@ async fn writing_files() {
     let mut f = t!(entries.next().await.unwrap());
 
     assert_eq!(&*f.header().path_bytes(), b"test2");
-    assert_eq!(f.header().size().unwrap(), 4);
+    assert_eq!(f.header().raw_file_size().unwrap(), 4);
     let mut s = String::new();
     t!(f.read_to_string(&mut s).await);
     assert_eq!(s, "test");
@@ -231,15 +231,15 @@ async fn large_filename() {
     // The short entry added with `append`
     let mut f = entries.next().await.unwrap().unwrap();
     assert_eq!(&*f.header().path_bytes(), filename.as_bytes());
-    assert_eq!(f.header().size().unwrap(), 4);
+    assert_eq!(f.header().raw_file_size().unwrap(), 4);
     let mut s = String::new();
     t!(f.read_to_string(&mut s).await);
     assert_eq!(s, "test");
 
     // The long entry added with `append_file`
     let mut f = entries.next().await.unwrap().unwrap();
     assert_eq!(&*t!(f.path_bytes()), too_long.as_bytes());
-    assert_eq!(f.header().size().unwrap(), 4);
+    assert_eq!(f.header().raw_file_size().unwrap(), 4);
     let mut s = String::new();
     t!(f.read_to_string(&mut s).await);
     assert_eq!(s, "test");
@@ -248,7 +248,7 @@ async fn large_filename() {
     let mut f = entries.next().await.unwrap().unwrap();
     assert!(f.header().path_bytes().len() < too_long.len());
     assert_eq!(&*t!(f.path_bytes()), too_long.as_bytes());
-    assert_eq!(f.header().size().unwrap(), 4);
+    assert_eq!(f.header().raw_file_size().unwrap(), 4);
     let mut s = String::new();
     t!(f.read_to_string(&mut s).await);
     assert_eq!(s, "test");
@@ -280,7 +280,7 @@ async fn large_filename_with_dot_dot_at_100_byte_mark() {
 
     let mut f = t!(entries.next().await.unwrap());
     assert_eq!(&*t!(f.path_bytes()), long_name_with_dot_dot.as_bytes());
-    assert_eq!(f.header().size().unwrap(), 4);
+    assert_eq!(f.header().raw_file_size().unwrap(), 4);
     let mut s = String::new();
     t!(f.read_to_string(&mut s).await);
     assert_eq!(s, "test");
@@ -784,7 +784,7 @@ async fn octal_spaces() {
     assert_eq!(entry.header().mode().unwrap() & 0o777, 0o777);
     assert_eq!(entry.header().uid().unwrap(), 0);
     assert_eq!(entry.header().gid().unwrap(), 0);
-    assert_eq!(entry.header().size().unwrap(), 2);
+    assert_eq!(entry.header().raw_file_size().unwrap(), 2);
     assert_eq!(entry.header().mtime().unwrap(), 0o12_440_016_664);
     assert_eq!(entry.header().cksum().unwrap(), 0o4253);
 }
@@ -1318,6 +1318,68 @@ async fn pax_precedence() {
     assert!(entries.next().await.is_none());
 }
 
+#[tokio::test]
+async fn pax_size_overrides_header() {
+    let mut ar = Archive::new(tar!("pax-header-precedence.tar"));
+    let mut entries = t!(ar.entries());
+
+    let first = t!(entries.next().await.unwrap());
+    assert!(t!(first.path()).ends_with("normal.txt"));
+    assert_eq!(first.effective_size(), 6);
+    assert_eq!(first.effective_size(), t!(first.header().raw_file_size()));
+
+    let mut second = t!(entries.next().await.unwrap());
+    assert!(t!(second.path()).ends_with("blob.bin"));
+    assert_eq!(second.effective_size(), 1024);
+    assert_eq!(t!(second.header().raw_file_size()), 0);
+
+    let mut byte = [0];
+    t!(second.read_exact(&mut byte).await);
+    assert_eq!(second.effective_size(), 1024);
+
+    let mut contents = Vec::new();
+    t!(second.read_to_end(&mut contents).await);
+    assert_eq!(contents.len(), 1023);
+    assert_eq!(second.effective_size(), 1024);
+    assert_eq!(t!(second.header().raw_file_size()), 0);
+
+    let third = t!(entries.next().await.unwrap());
+    assert!(t!(third.path()).ends_with("marker.txt"));
+    assert_eq!(third.effective_size(), 7);
+    assert_eq!(third.effective_size(), t!(third.header().raw_file_size()));
+    assert!(entries.next().await.is_none());
+}
+
+#[tokio::test]
+async fn last_pax_size_wins() {
+    let mut sizes = pax_record("size", b"3");
+    sizes.extend(pax_record("size", b"4"));
+
+    let mut builder = Builder::new(Vec::new());
+    let mut extension = Header::new_ustar();
+    extension.set_size(sizes.len() as u64);
+    extension.set_entry_type(EntryType::new(b'x'));
+    t!(builder.append_data(&mut extension, "pax", &sizes[..]).await);
+
+    // PAX determines the payload size; keep the raw header size different.
+    let mut file = Header::new_ustar();
+    file.set_size(1);
+    t!(builder.append_data(&mut file, "file", &b"DATA"[..]).await);
+
+    let bytes = t!(builder.into_inner().await);
+    let mut archive = Archive::new(&bytes[..]);
+    let mut entries = t!(archive.entries());
+
+    let mut file = t!(entries.next().await.unwrap());
+    assert_eq!(file.effective_size(), 4);
+    assert_eq!(t!(file.header().raw_file_size()), 1);
+
+    let mut contents = Vec::new();
+    t!(file.read_to_end(&mut contents).await);
+    assert_eq!(contents, b"DATA");
+    assert!(entries.next().await.is_none());
+}
+
 async fn pax_numeric_override_archive(pax: &[u8]) -> Vec<u8> {
     let mut builder = Builder::new(Vec::new());
 
@@ -1344,7 +1406,7 @@ async fn pax_numeric_overrides_accept_decimal_digits() {
     let mut entries = t!(archive.entries());
 
     let entry = t!(entries.next().await.unwrap());
-    assert_eq!(t!(entry.header().size()), 2);
+    assert_eq!(t!(entry.header().raw_file_size()), 2);
     assert_eq!(t!(entry.header().uid()), 42);
     assert_eq!(t!(entry.header().gid()), 43);
     assert!(entries.next().await.is_none());
@@ -1619,47 +1681,6 @@ async fn pax_non_utf8_owner_names_are_exposed_as_bytes() {
     assert!(entries.next().await.is_none());
 }
 
-#[tokio::test]
-async fn pax_size_updates_header() {
-    let mut ar = Archive::new(tar!("pax-header-precedence.tar"));
-    let mut entries = t!(ar.entries());
-
-    let first = t!(entries.next().await.unwrap());
-    assert!(first.path().unwrap().ends_with("normal.txt"));
-
-    let second = t!(entries.next().await.unwrap());
-    assert!(second.path().unwrap().ends_with("blob.bin"));
-    assert_eq!(second.header().size().unwrap(), 1024);
-}
-
-#[tokio::test]
-async fn last_pax_size_wins() {
-    let mut sizes = pax_record("size", b"3");
-    sizes.extend(pax_record("size", b"4"));
-
-    let mut builder = Builder::new(Vec::new());
-    let mut extension = Header::new_ustar();
-    extension.set_size(sizes.len() as u64);
-    extension.set_entry_type(EntryType::new(b'x'));
-    t!(builder.append_data(&mut extension, "pax", &sizes[..]).await);
-
-    let mut file = Header::new_ustar();
-    file.set_size(4);
-    t!(builder.append_data(&mut file, "file", &b"DATA"[..]).await);
-
-    let bytes = t!(builder.into_inner().await);
-    let mut archive = Archive::new(&bytes[..]);
-    let mut entries = t!(archive.entries());
-
-    let mut file = t!(entries.next().await.unwrap());
-    assert_eq!(t!(file.header().size()), 4);
-
-    let mut contents = Vec::new();
-    t!(file.read_to_end(&mut contents).await);
-    assert_eq!(contents, b"DATA");
-    assert!(entries.next().await.is_none());
-}
-
 #[tokio::test]
 async fn solaris_extended_header_applies_pax_path() {
     let mut builder = Builder::new(Vec::new());
@@ -1969,6 +1990,7 @@ async fn large_sparse() {
     let a = t!(entries.next().await.unwrap());
     let h = a.header().as_gnu().unwrap();
     assert_eq!(h.real_size().unwrap(), 12626929280);
+    assert_eq!(a.effective_size(), 12626929280);
 }
 
 #[tokio::test]
@@ -1977,9 +1999,11 @@ async fn sparse_with_trailing() {
     let mut ar = Archive::new(rdr);
     let mut entries = t!(ar.entries());
     let mut a = t!(entries.next().await.unwrap());
+    assert_eq!(a.effective_size(), 0x100_00c);
     let mut s = String::new();
     t!(a.read_to_string(&mut s).await);
     assert_eq!(0x100_00c, s.len());
+    assert_eq!(a.effective_size(), s.len() as u64);
     assert_eq!(&s[..0xc], "0MB through\n");
     assert!(s[0xc..0x100_000].chars().all(|x| x == '\u{0}'));
     assert_eq!(&s[0x100_000..], "1MB through\n");
@@ -2080,23 +2104,23 @@ async fn append_path_symlink() {
         t!(entry.link_name()),
         Some(Cow::from(Path::new("testdest")))
     );
-    assert_eq!(t!(entry.header().size()), 0);
+    assert_eq!(t!(entry.header().raw_file_size()), 0);
 
     let entry = t!(entries.next().await.unwrap());
     assert_eq!(t!(entry.path()), Path::new("test2"));
     assert_eq!(
         t!(entry.link_name()),
         Some(Cow::from(Path::new(&long_linkname)))
     );
-    assert_eq!(t!(entry.header().size()), 0);
+    assert_eq!(t!(entry.header().raw_file_size()), 0);
 
     let entry = t!(entries.next().await.unwrap());
     assert_eq!(t!(entry.path()), Path::new(&long_pathname));
     assert_eq!(
         t!(entry.link_name()),
         Some(Cow::from(Path::new(&long_linkname)))
     );
-    assert_eq!(t!(entry.header().size()), 0);
+    assert_eq!(t!(entry.header().raw_file_size()), 0);
 
     assert!(entries.next().await.is_none());
 }

tests/all.rs

@@ -213,7 +213,7 @@ fn set_metadata_deterministic() {
     let two = mk_header(tmppath.as_path(), true);
 
     // Always expected to match.
-    assert_eq!(t!(one.size()), t!(two.size()));
+    assert_eq!(t!(one.raw_file_size()), t!(two.raw_file_size()));
     assert_eq!(t!(one.path()), t!(two.path()));
     assert_eq!(t!(one.mode()), t!(two.mode()));
 
@@ -236,9 +236,9 @@ fn extended_numeric_format() {
     h.as_header_mut().set_size(44);
     assert_eq!(h.size, [48, 48, 48, 48, 48, 48, 48, 48, 48, 53, 52, 0]);
     h.size = [0x80, 0, 0, 0, 0, 0, 0, 0x02, 0, 0, 0, 0];
-    assert_eq!(h.as_header().entry_size().unwrap(), 0x0200000000);
+    assert_eq!(h.as_header().raw_entry_size().unwrap(), 0x0200000000);
     h.size = [48, 48, 48, 48, 48, 48, 48, 48, 48, 53, 51, 0];
-    assert_eq!(h.as_header().entry_size().unwrap(), 43);
+    assert_eq!(h.as_header().raw_entry_size().unwrap(), 43);
 
     h.as_header_mut().set_gid(42);
     assert_eq!(h.gid, [48, 48, 48, 48, 48, 53, 50, 0]);