Repository: alexcrichton/tar-rs (language: rust)

Issue:
Err on GNU sparse files with content at end
GNU tar `--sparse` produces archives we can't process, we hit this error:
https://github.com/alexcrichton/tar-rs/blob/8d51b538da744eb7a30022b2a3d13b6a49f6f419/src/archive.rs#L410-L413

A sparse file with content at the end, such as:

```
$ for i in $(seq 0 1); do truncate -s ${i}M a.big; echo "${i}MB through" >> a.big; done
$ hexyl a.big
┌────────┬─────────────────────────┬─────────────────────────┬────────┬────────┐
│00000000│ 30 4d 42 20 74 68 72 6f ┊ 75 67 68 0a 00 00 00 00 │0MB thro┊ugh_0000│
│00000010│ 00 00 00 00 00 00 00 00 ┊ 00 00 00 00 00 00 00 00 │00000000┊00000000│
│*       │                         ┊                         │        ┊        │
│00100000│ 31 4d 42 20 74 68 72 6f ┊ 75 67 68 0a             │1MB thro┊ugh_    │
└────────┴─────────────────────────┴─────────────────────────┴────────┴────────┘
```

Packed into a tar with `tar -cf a.tar --sparse a.big` into [a.tar.gz](https://github.com/alexcrichton/tar-rs/files/5970459/a.tar.gz)
 apparently produces this header structure:

```
regular header 0: GnuSparseHeader { offset: 0, length: 4096 }
regular header 1: GnuSparseHeader { offset: 1048576, length: 12 }
regular header 2: GnuSparseHeader { offset: 1048588, length: 0 }
```

...and we hit the error as `12 % 512 != 0`.

I notice `remaining` does hit zero after this subtraction:

https://github.com/alexcrichton/tar-rs/blob/8d51b538da744eb7a30022b2a3d13b6a49f6f419/src/archive.rs#L426-L431

Do we need to break the header parsing if we see a sparse header with a length of zero?
Do we need to skip the 512 byte alignment check for the last chunk, or if we're at the end of the entry?

Tested with GNU tar 1.30 (ubuntu 20.10 amd64). We can parse its `-H pax` output fine.


Discussion / hints:
Apparently we can't parse the pax output fine either; the unpacked files are not even the right length. I'll try and raise a different bug about that.
Thanks for the report! I don't have a ton of experience with sparse archives, but given how relatively rarely they're used I wouldn't be surprised if there's a bug in the handling within this crate. 

I don't have a ton of time to investigate this myself at the moment but I would be happy to review a PR!

Fix patch:
diff --git a/src/archive.rs b/src/archive.rs
index 921bc5e0..8c0b0894 100644
--- a/src/archive.rs
+++ b/src/archive.rs
@@ -405,8 +405,7 @@ impl<'a> EntriesFields<'a> {
                 }
                 let off = block.offset()?;
                 let len = block.length()?;
-
-                if (size - remaining) % 512 != 0 {
+                if len != 0 && (size - remaining) % 512 != 0 {
                     return Err(other(
                         "previous block in sparse file was not \
                          aligned to 512-byte boundary",


Test patch:
diff --git a/tests/all.rs b/tests/all.rs
index 6ba29f04..4a31cd35 100644
--- a/tests/all.rs
+++ b/tests/all.rs
@@ -934,6 +934,20 @@ fn extract_sparse() {
     assert!(s[0x2fa0 + 6..0x4000].chars().all(|x| x == '\u{0}'));
 }
 
+#[test]
+fn sparse_with_trailing() {
+    let rdr = Cursor::new(tar!("sparse-1.tar"));
+    let mut ar = Archive::new(rdr);
+    let mut entries = t!(ar.entries());
+    let mut a = t!(entries.next().unwrap());
+    let mut s = String::new();
+    t!(a.read_to_string(&mut s));
+    assert_eq!(0x100_00c, s.len());
+    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");
+}
+
 #[test]
 fn path_separators() {
     let mut ar = Builder::new(Vec::new());
diff --git a/tests/archives/sparse-1.tar b/tests/archives/sparse-1.tar
new file mode 100644
index 00000000..b3aef029
Binary files /dev/null and b/tests/archives/sparse-1.tar differ
diff --git a/tests/archives/sparse-n.sh b/tests/archives/sparse-n.sh
new file mode 100755
index 00000000..72746c76
--- /dev/null
+++ b/tests/archives/sparse-n.sh
@@ -0,0 +1,9 @@
+#!/bin/sh
+set -eu
+T="$(mktemp)"
+trap 'rm "$T"' EXIT
+for i in $(seq 0 "$1"); do
+  truncate -s "${i}M" a.big
+  echo "${i}MB through" >> a.big
+ done
+tar -cf sparse-"$1".tar --sparse a.big