fix for archiving long paths that have path components starting with …

…".." crossing the 100-character mark (#390)

* test showing problem with long paths that have a component starting with .. at 100-character mark

his test checks very particular scenario where path component starting with
".." of a long path gets split at 100-byte mark so that ".." goes into header
and gets interpreted as parent dir (and rejected).

* fix for long path that have path starting .. at the 100-character mark

This is a fix for very specific scenario where a path component starting with
".." of a long path gets split at 100-byte mark so that ".." goes into header
and gets interpreted as parent dir (and rejected).

See tests for repro of the issue.

* rustfmt

* Coding style fixes following reviewer's suggestions

* Fix comments

src/builder.rs

@@ -768,7 +768,7 @@ fn prepare_header_path(dst: &mut dyn Write, header: &mut Header, path: &Path) ->
             Ok(s) => s,
             Err(e) => str::from_utf8(&data[..e.valid_up_to()]).unwrap(),
         };
-        header.set_path(truncated)?;
+        header.set_truncated_path_for_gnu_header(&truncated)?;
     }
     Ok(())
 }

src/header.rs

@@ -380,14 +380,29 @@ impl Header {
     /// use `Builder` methods to insert a long-name extension at the same time
     /// as the file content.
     pub fn set_path<P: AsRef<Path>>(&mut self, p: P) -> io::Result<()> {
-        self._set_path(p.as_ref())
+        self.set_path_inner(p.as_ref(), false)
     }
 
-    fn _set_path(&mut self, path: &Path) -> io::Result<()> {
+    // Sets the truncated path for GNU header
+    //
+    // Same as set_path but skips some validations.
+    pub(crate) fn set_truncated_path_for_gnu_header<P: AsRef<Path>>(
+        &mut self,
+        p: P,
+    ) -> io::Result<()> {
+        self.set_path_inner(p.as_ref(), true)
+    }
+
+    fn set_path_inner(&mut self, path: &Path, is_truncated_gnu_long_path: bool) -> io::Result<()> {
         if let Some(ustar) = self.as_ustar_mut() {
             return ustar.set_path(path);
         }
-        copy_path_into(&mut self.as_old_mut().name, path, false).map_err(|err| {
+        if is_truncated_gnu_long_path {
+            copy_path_into_gnu_long(&mut self.as_old_mut().name, path, false)
+        } else {
+            copy_path_into(&mut self.as_old_mut().name, path, false)
+        }
+        .map_err(|err| {
             io::Error::new(
                 err.kind(),
                 format!("{} when setting path for {}", err, self.path_lossy()),
@@ -1532,25 +1547,28 @@ fn copy_into(slot: &mut [u8], bytes: &[u8]) -> io::Result<()> {
     }
 }
 
-/// Copies `path` into the `slot` provided
-///
-/// Returns an error if:
-///
-/// * the path is too long to fit
-/// * a nul byte was found
-/// * an invalid path component is encountered (e.g. a root path or parent dir)
-/// * the path itself is empty
-fn copy_path_into(mut slot: &mut [u8], path: &Path, is_link_name: bool) -> io::Result<()> {
+fn copy_path_into_inner(
+    mut slot: &mut [u8],
+    path: &Path,
+    is_link_name: bool,
+    is_truncated_gnu_long_path: bool,
+) -> io::Result<()> {
     let mut emitted = false;
     let mut needs_slash = false;
-    for component in path.components() {
+    let mut iter = path.components().peekable();
+    while let Some(component) = iter.next() {
         let bytes = path2bytes(Path::new(component.as_os_str()))?;
         match (component, is_link_name) {
             (Component::Prefix(..), false) | (Component::RootDir, false) => {
                 return Err(other("paths in archives must be relative"));
             }
             (Component::ParentDir, false) => {
-                return Err(other("paths in archives must not have `..`"));
+                // If it's last component of a gnu long path we know that there might be more
+                // to the component than .. (the rest is stored elsewhere)
+                // Otherwise it's a clear error
+                if !is_truncated_gnu_long_path || iter.peek().is_some() {
+                    return Err(other("paths in archives must not have `..`"));
+                }
             }
             // Allow "./" as the path
             (Component::CurDir, false) if path.components().count() == 1 => {}
@@ -1587,6 +1605,32 @@ fn copy_path_into(mut slot: &mut [u8], path: &Path, is_link_name: bool) -> io::R
     }
 }
 
+/// Copies `path` into the `slot` provided
+///
+/// Returns an error if:
+///
+/// * the path is too long to fit
+/// * a nul byte was found
+/// * an invalid path component is encountered (e.g. a root path or parent dir)
+/// * the path itself is empty
+fn copy_path_into(slot: &mut [u8], path: &Path, is_link_name: bool) -> io::Result<()> {
+    copy_path_into_inner(slot, path, is_link_name, false)
+}
+
+/// Copies `path` into the `slot` provided
+///
+/// Returns an error if:
+///
+/// * the path is too long to fit
+/// * a nul byte was found
+/// * an invalid path component is encountered (e.g. a root path or parent dir)
+/// * the path itself is empty
+///
+/// This is less restrictive version meant to be used for truncated GNU paths.
+fn copy_path_into_gnu_long(slot: &mut [u8], path: &Path, is_link_name: bool) -> io::Result<()> {
+    copy_path_into_inner(slot, path, is_link_name, true)
+}
+
 #[cfg(target_arch = "wasm32")]
 fn ends_with_slash(p: &Path) -> bool {
     p.to_string_lossy().ends_with('/')

tests/all.rs

@@ -203,6 +203,35 @@ fn large_filename() {
     assert!(entries.next().is_none());
 }
 
+// This test checks very particular scenario where a path component starting
+// with ".." of a long path gets split at 100-byte mark so that ".." part goes
+// into header and gets interpreted as parent dir (and rejected) .
+#[test]
+fn large_filename_with_dot_dot_at_100_byte_mark() {
+    let mut ar = Builder::new(Vec::new());
+
+    let mut header = Header::new_gnu();
+    header.set_mode(0o644);
+    header.set_size(4);
+
+    let mut long_name_with_dot_dot = "tdir/".repeat(19);
+    long_name_with_dot_dot.push_str("tt/..file");
+
+    t!(ar.append_data(&mut header, &long_name_with_dot_dot, b"test".as_slice()));
+
+    let rd = Cursor::new(t!(ar.into_inner()));
+    let mut ar = Archive::new(rd);
+    let mut entries = t!(ar.entries());
+
+    let mut f = entries.next().unwrap().unwrap();
+    assert_eq!(&*f.path_bytes(), long_name_with_dot_dot.as_bytes());
+    assert_eq!(f.header().size().unwrap(), 4);
+    let mut s = String::new();
+    t!(f.read_to_string(&mut s));
+    assert_eq!(s, "test");
+    assert!(entries.next().is_none());
+}
+
 fn reading_entries_common<R: Read>(mut entries: Entries<R>) {
     let mut a = t!(entries.next().unwrap());
     assert_eq!(&*a.header().path_bytes(), b"a");