Fix GNU long-name extension stream corruption on validation error (#434)

In prepare_header_path, the GNU long-name extension entry was written to
the stream before validating the truncated path via
set_truncated_path_for_gnu_header. If validation failed (e.g., the
truncated path contained '..'), the extension entry was already committed
to the stream with no rollback. The Builder remained usable, so
subsequent writes succeeded — but their data got associated with the
orphaned long-name path, silently corrupting the archive.

Fix by moving the truncation and validation above the append() call.
Since set_truncated_path_for_gnu_header only writes to the in-memory
header buffer (not the stream), reordering is safe.

Also audited prepare_header_link — it does not have this issue because
link names allow all path components (ParentDir, RootDir, etc.) and
there is no post-append validation step.

Author: michelhe

src/builder.rs

@@ -756,21 +756,25 @@ fn prepare_header_path(dst: &mut dyn Write, header: &mut Header, path: &Path) ->
         if data.len() < max {
             return Err(e);
         }
-        let header2 = prepare_header(data.len() as u64, b'L');
-        // null-terminated string
-        let mut data2 = data.chain(io::repeat(0).take(1));
-        append(dst, &header2, &mut data2)?;
-
         // Truncate the path to store in the header we're about to emit to
         // ensure we've got something at least mentioned. Note that we use
         // `str`-encoding to be compatible with Windows, but in general the
         // entry in the header itself shouldn't matter too much since extraction
         // doesn't look at it.
+        //
+        // Validate the truncated path BEFORE writing the long-name extension
+        // to the stream. If validation fails after writing, the orphaned
+        // extension entry corrupts subsequent archive entries.
         let truncated = match str::from_utf8(&data[..max]) {
             Ok(s) => s,
             Err(e) => str::from_utf8(&data[..e.valid_up_to()]).unwrap(),
         };
         header.set_truncated_path_for_gnu_header(truncated)?;
+
+        let header2 = prepare_header(data.len() as u64, b'L');
+        // null-terminated string
+        let mut data2 = data.chain(io::repeat(0).take(1));
+        append(dst, &header2, &mut data2)?;
     }
     Ok(())
 }

tests/all.rs

@@ -1787,3 +1787,42 @@ fn pax_and_gnu_uid_gid() {
         }
     }
 }
+
+#[test]
+fn append_data_error_does_not_corrupt_subsequent_entries() {
+    // When append_data fails (e.g., path contains ".."), subsequent
+    // successful writes must not be corrupted by an orphaned GNU
+    // long-name extension entry left in the stream.
+    let mut ar = Builder::new(Vec::new());
+
+    // First write: a long path (>100 bytes to trigger GNU long-name extension)
+    // containing ".." not as the last component, which will fail validation
+    // in set_truncated_path_for_gnu_header.
+    let dotdot_path = "a/../b/".to_string() + &"c".repeat(100);
+    let mut header = Header::new_gnu();
+    header.set_size(5);
+    header.set_cksum();
+    let result = ar.append_data(&mut header, &dotdot_path, &b"first"[..]);
+    assert!(result.is_err());
+
+    // Second write: a clean path that should succeed normally.
+    let mut header = Header::new_gnu();
+    header.set_size(6);
+    header.set_cksum();
+    ar.append_data(&mut header, "clean.txt", &b"second"[..])
+        .unwrap();
+
+    // Verify: the archive should contain exactly one entry at "clean.txt"
+    // with content "second". Before the fix, it contained an entry at the
+    // dotdot path with content "second" — the orphaned long-name stole the data.
+    let data = ar.into_inner().unwrap();
+    let mut archive = Archive::new(&data[..]);
+    let entries: Vec<_> = archive
+        .entries()
+        .unwrap()
+        .collect::<Result<_, _>>()
+        .unwrap();
+
+    assert_eq!(entries.len(), 1);
+    assert_eq!(entries[0].path().unwrap().to_str().unwrap(), "clean.txt");
+}