public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore
@ 2024-01-24 10:15 Filip Schauer
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 proxmox 1/3] compression: Refactor ZipEntry creation and add FileType enum Filip Schauer
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Filip Schauer @ 2024-01-24 10:15 UTC (permalink / raw)
  To: pbs-devel

Include symlinks when restoring files from a backup as a zip file.

Resulting ZIP files were successfully tested on Linux with: zipinfo,
unzip, unar

On Windows the extracted symlinks show up as regular files containing
the path to the destination. As far as I am aware Windows Explorer does
not support symlinks in ZIP files.

Changes since v3:
* Move content to Regular FileType enum
* Make ZipEntry private
* Add take_reader function to FileType enum

Changes since v2:
* Add a FileType enum and embed the symlink target into the enum.
* Add unit tests for ZipEncoder to the proxmox-compression crate

Changes since v1:
* Use P instead of &Path
* Fix compile error due to misplaced comma
* Check content before symlink_target, since regular files are more
  common than symlinks

proxmox:

Filip Schauer (3):
  compression: Refactor ZipEntry creation and add FileType enum
  compression: Add support for symlinks in zip files
  compression: Add unit tests for the ZipEncoder

 proxmox-compression/Cargo.toml   |   2 +-
 proxmox-compression/src/zip.rs   | 159 +++++++++++++++++++------------
 proxmox-compression/tests/zip.rs | 123 ++++++++++++++++++++++++
 3 files changed, 223 insertions(+), 61 deletions(-)
 create mode 100644 proxmox-compression/tests/zip.rs

proxmox-backup:

Filip Schauer (2):
  pxar: Adopt FileType enum when adding a zip entry
  fix #4995: pxar: Include symlinks in zip file creation

 pbs-client/src/pxar/extract.rs | 59 +++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 23 deletions(-)

-- 
2.39.2




^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH v4 proxmox 1/3] compression: Refactor ZipEntry creation and add FileType enum
  2024-01-24 10:15 [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore Filip Schauer
@ 2024-01-24 10:15 ` Filip Schauer
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 proxmox 2/3] compression: Add support for symlinks in zip files Filip Schauer
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2024-01-24 10:15 UTC (permalink / raw)
  To: pbs-devel

Refactor the process of adding an entry to the ZipEncoder. Instead of
passing a ZipEntry and the content seperately, pass all parameters
required to create a ZipEntry directly to add_entry, including a new
FileType enum that contains the file content if it is a regular file.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 proxmox-compression/src/zip.rs | 145 +++++++++++++++++++--------------
 1 file changed, 82 insertions(+), 63 deletions(-)

diff --git a/proxmox-compression/src/zip.rs b/proxmox-compression/src/zip.rs
index d2d3fd8..6229990 100644
--- a/proxmox-compression/src/zip.rs
+++ b/proxmox-compression/src/zip.rs
@@ -17,6 +17,7 @@ use std::time::SystemTime;
 use anyhow::{format_err, Error, Result};
 use endian_trait::Endian;
 use futures::ready;
+use libc::{S_IFDIR, S_IFREG};
 use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt, ReadBuf};
 
 use crc32fast::Hasher;
@@ -71,6 +72,20 @@ fn epoch_to_dos(epoch: i64) -> (u16, u16) {
     (date, time)
 }
 
+pub enum FileType<R> {
+    Directory,
+    Regular(R),
+}
+
+impl<R: AsyncRead + Unpin + Send> FileType<R> {
+    fn take_reader(&mut self) -> Option<HashWrapper<&mut (dyn AsyncRead + Unpin + Send)>> {
+        match self {
+            FileType::Directory => None,
+            FileType::Regular(content) => Some(HashWrapper::new(content)),
+        }
+    }
+}
+
 #[derive(Endian)]
 #[repr(C, packed)]
 struct Zip64Field {
@@ -192,9 +207,7 @@ where
 }
 
 /// Represents an Entry in a ZIP File
-///
-/// used to add to a ZipEncoder
-pub struct ZipEntry {
+struct ZipEntry {
     filename: OsString,
     mtime: i64,
     mode: u16,
@@ -202,16 +215,20 @@ pub struct ZipEntry {
     uncompressed_size: u64,
     compressed_size: u64,
     offset: u64,
-    is_file: bool,
     is_utf8_filename: bool,
 }
 
 impl ZipEntry {
     /// Creates a new ZipEntry
     ///
-    /// if is_file is false the path will contain an trailing separator,
+    /// if file_type is Directory the path will contain a trailing separator,
     /// so that the zip file understands that it is a directory
-    pub fn new<P: AsRef<Path>>(path: P, mtime: i64, mode: u16, is_file: bool) -> Self {
+    fn new<P: AsRef<Path>, R: AsyncRead + Unpin>(
+        path: P,
+        mtime: i64,
+        mode: u16,
+        file_type: &FileType<R>,
+    ) -> Self {
         let mut relpath = PathBuf::new();
 
         for comp in path.as_ref().components() {
@@ -220,22 +237,26 @@ impl ZipEntry {
             }
         }
 
-        if !is_file {
+        if matches!(file_type, FileType::Directory) {
             relpath.push(""); // adds trailing slash
         }
 
         let filename: OsString = relpath.into();
         let is_utf8_filename = filename.to_str().is_some();
 
+        let file_type_attr = match file_type {
+            FileType::Regular(_) => S_IFREG,
+            FileType::Directory => S_IFDIR,
+        };
+
         Self {
             filename,
             crc32: 0,
             mtime,
-            mode,
+            mode: mode | file_type_attr as u16,
             uncompressed_size: 0,
             compressed_size: 0,
             offset: 0,
-            is_file,
             is_utf8_filename,
         }
     }
@@ -342,6 +363,8 @@ impl ZipEntry {
             )
         };
 
+        let is_directory = (self.mode & 0xF000) == S_IFDIR as u16;
+
         write_struct(
             &mut buf,
             CentralDirectoryFileHeader {
@@ -360,7 +383,7 @@ impl ZipEntry {
                 comment_len: 0,
                 start_disk: 0,
                 internal_flags: 0,
-                external_flags: (self.mode as u32) << 16 | (!self.is_file as u32) << 4,
+                external_flags: (self.mode as u32) << 16 | (is_directory as u32) << 4,
                 offset,
             },
         )
@@ -437,7 +460,7 @@ where
 /// use anyhow::{Error, Result};
 /// use tokio::fs::File;
 ///
-/// use proxmox_compression::zip::{ZipEncoder, ZipEntry};
+/// use proxmox_compression::zip::{FileType, ZipEncoder};
 ///
 /// //#[tokio::main]
 /// async fn main_() -> Result<(), Error> {
@@ -445,12 +468,12 @@ where
 ///     let mut source = File::open("foo.txt").await?;
 ///
 ///     let mut zip = ZipEncoder::new(target);
-///     zip.add_entry(ZipEntry::new(
+///     zip.add_entry(
 ///         "foo.txt",
 ///         0,
 ///         0o100755,
-///         true,
-///     ), Some(source)).await?;
+///         FileType::Regular(source),
+///     ).await?;
 ///
 ///     zip.finish().await?;
 ///
@@ -475,34 +498,38 @@ impl<W: AsyncWrite + Unpin> ZipEncoder<W> {
         }
     }
 
-    pub async fn add_entry<R: AsyncRead + Unpin>(
+    pub async fn add_entry<P: AsRef<Path>, R: AsyncRead + Unpin + Send>(
         &mut self,
-        mut entry: ZipEntry,
-        content: Option<R>,
+        path: P,
+        mtime: i64,
+        mode: u16,
+        mut file_type: FileType<R>,
     ) -> Result<(), Error> {
         let mut target = self
             .target
             .take()
             .ok_or_else(|| format_err!("had no target during add entry"))?;
+        let mut entry = ZipEntry::new(path, mtime, mode, &file_type);
         entry.offset = self.byte_count.try_into()?;
         self.byte_count += entry.write_local_header(&mut target).await?;
-        if let Some(content) = content {
-            let mut reader = HashWrapper::new(content);
+
+        if !matches!(file_type, FileType::Directory) {
             let mut enc = DeflateEncoder::with_quality(target, Level::Fastest);
 
-            enc.compress(&mut reader).await?;
+            if let Some(mut reader) = file_type.take_reader() {
+                enc.compress(&mut reader).await?;
+                entry.crc32 = reader.finish().0;
+            }
+
             let total_in = enc.total_in();
             let total_out = enc.total_out();
             target = enc.into_inner();
 
-            let (crc32, _reader) = reader.finish();
-
             self.byte_count += total_out as usize;
             entry.compressed_size = total_out;
             entry.uncompressed_size = total_in;
-
-            entry.crc32 = crc32;
         }
+
         self.byte_count += entry.write_data_descriptor(&mut target).await?;
         self.target = Some(target);
 
@@ -640,46 +667,38 @@ where
             }
         };
 
-        let entry_path = entry.path().to_owned();
         let encoder = &mut encoder;
-
-        let entry = async move {
-            let entry_path_no_base = entry.path().strip_prefix(base_path)?;
-            let metadata = entry.metadata()?;
-            let mtime = match metadata
-                .modified()
-                .unwrap_or_else(|_| SystemTime::now())
-                .duration_since(SystemTime::UNIX_EPOCH)
-            {
-                Ok(dur) => dur.as_secs() as i64,
-                Err(time_error) => -(time_error.duration().as_secs() as i64),
-            };
-            let mode = metadata.mode() as u16;
-
-            if entry.file_type().is_file() {
-                let file = tokio::fs::File::open(entry.path()).await?;
-                let ze = ZipEntry::new(entry_path_no_base, mtime, mode, true);
-                Ok(Some((ze, Some(file))))
-            } else if entry.file_type().is_dir() {
-                let ze = ZipEntry::new(entry_path_no_base, mtime, mode, false);
-                let content: Option<tokio::fs::File> = None;
-                Ok(Some((ze, content)))
-            } else {
-                // ignore other file types
-                Ok::<_, Error>(None)
-            }
-        }
-        .await;
-        match entry {
-            Ok(Some((ze, content))) => encoder.add_entry(ze, content).await?,
-            Ok(None) => {}
-            Err(err) => {
-                eprintln!(
-                    "zip: error encoding file or directory '{}': {}",
-                    entry_path.display(),
-                    err
-                );
-            }
+        let entry_path_no_base = entry.path().strip_prefix(base_path)?;
+        let metadata = entry.metadata()?;
+        let mtime = match metadata
+            .modified()
+            .unwrap_or_else(|_| SystemTime::now())
+            .duration_since(SystemTime::UNIX_EPOCH)
+        {
+            Ok(dur) => dur.as_secs() as i64,
+            Err(time_error) => -(time_error.duration().as_secs() as i64),
+        };
+        let mode = metadata.mode() as u16;
+
+        if entry.file_type().is_file() {
+            let file = tokio::fs::File::open(entry.path()).await?;
+            encoder
+                .add_entry(
+                    entry_path_no_base,
+                    mtime,
+                    mode,
+                    FileType::Regular(Box::new(file)),
+                )
+                .await?;
+        } else if entry.file_type().is_dir() {
+            encoder
+                .add_entry(
+                    entry_path_no_base,
+                    mtime,
+                    mode,
+                    FileType::<&[u8]>::Directory,
+                )
+                .await?;
         }
     }
 
-- 
2.39.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH v4 proxmox 2/3] compression: Add support for symlinks in zip files
  2024-01-24 10:15 [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore Filip Schauer
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 proxmox 1/3] compression: Refactor ZipEntry creation and add FileType enum Filip Schauer
@ 2024-01-24 10:15 ` Filip Schauer
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 proxmox 3/3] compression: Add unit tests for the ZipEncoder Filip Schauer
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2024-01-24 10:15 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 proxmox-compression/src/zip.rs | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/proxmox-compression/src/zip.rs b/proxmox-compression/src/zip.rs
index 6229990..aec11a4 100644
--- a/proxmox-compression/src/zip.rs
+++ b/proxmox-compression/src/zip.rs
@@ -17,7 +17,7 @@ use std::time::SystemTime;
 use anyhow::{format_err, Error, Result};
 use endian_trait::Endian;
 use futures::ready;
-use libc::{S_IFDIR, S_IFREG};
+use libc::{S_IFDIR, S_IFLNK, S_IFREG};
 use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt, ReadBuf};
 
 use crc32fast::Hasher;
@@ -75,6 +75,7 @@ fn epoch_to_dos(epoch: i64) -> (u16, u16) {
 pub enum FileType<R> {
     Directory,
     Regular(R),
+    Symlink(OsString),
 }
 
 impl<R: AsyncRead + Unpin + Send> FileType<R> {
@@ -82,6 +83,12 @@ impl<R: AsyncRead + Unpin + Send> FileType<R> {
         match self {
             FileType::Directory => None,
             FileType::Regular(content) => Some(HashWrapper::new(content)),
+            FileType::Symlink(symlink_target) => {
+                let cursor = std::io::Cursor::new(symlink_target.as_bytes());
+                let reader_box = Box::new(cursor) as Box<dyn AsyncRead + Unpin + Send>;
+                let leaked_ref = Box::leak(reader_box);
+                Some(HashWrapper::new(leaked_ref))
+            }
         }
     }
 }
@@ -247,6 +254,7 @@ impl ZipEntry {
         let file_type_attr = match file_type {
             FileType::Regular(_) => S_IFREG,
             FileType::Directory => S_IFDIR,
+            FileType::Symlink(_) => S_IFLNK,
         };
 
         Self {
@@ -699,6 +707,16 @@ where
                     FileType::<&[u8]>::Directory,
                 )
                 .await?;
+        } else if entry.file_type().is_symlink() {
+            let target = std::fs::read_link(entry.path())?;
+            encoder
+                .add_entry(
+                    entry_path_no_base,
+                    mtime,
+                    mode,
+                    FileType::<io::Cursor<&[u8]>>::Symlink(target.into()),
+                )
+                .await?;
         }
     }
 
-- 
2.39.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH v4 proxmox 3/3] compression: Add unit tests for the ZipEncoder
  2024-01-24 10:15 [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore Filip Schauer
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 proxmox 1/3] compression: Refactor ZipEntry creation and add FileType enum Filip Schauer
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 proxmox 2/3] compression: Add support for symlinks in zip files Filip Schauer
@ 2024-01-24 10:15 ` Filip Schauer
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 backup 1/2] pxar: Adopt FileType enum when adding a zip entry Filip Schauer
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2024-01-24 10:15 UTC (permalink / raw)
  To: pbs-devel

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 proxmox-compression/Cargo.toml   |   2 +-
 proxmox-compression/tests/zip.rs | 123 +++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+), 1 deletion(-)
 create mode 100644 proxmox-compression/tests/zip.rs

diff --git a/proxmox-compression/Cargo.toml b/proxmox-compression/Cargo.toml
index 49735cb..8c9c47e 100644
--- a/proxmox-compression/Cargo.toml
+++ b/proxmox-compression/Cargo.toml
@@ -27,5 +27,5 @@ proxmox-io = { workspace = true, features = [ "tokio" ] }
 proxmox-lang.workspace = true
 
 [dev-dependencies]
-tokio = { workspace = true, features = [ "macros" ] }
+tokio = { workspace = true, features = [ "macros", "rt" ] }
 
diff --git a/proxmox-compression/tests/zip.rs b/proxmox-compression/tests/zip.rs
new file mode 100644
index 0000000..1f76037
--- /dev/null
+++ b/proxmox-compression/tests/zip.rs
@@ -0,0 +1,123 @@
+use proxmox_compression::zip::{FileType, ZipEncoder};
+
+use anyhow::{ensure, Error};
+use flate2::{Decompress, FlushDecompress};
+use std::ffi::OsString;
+use tokio::test;
+
+fn check_zip_with_one_file(
+    bytes: Vec<u8>,
+    expected_file_name: &str,
+    expected_content: &[u8],
+    expected_file_attributes: u16,
+    is_file: bool,
+) -> Result<(), Error> {
+    ensure!(
+        &bytes[..4] == b"PK\x03\x04",
+        "ZIP magic number does not match"
+    );
+
+    let general_purpose_flags = &bytes[6..8];
+    let size_compressed = &bytes[18..22];
+    let size_uncompressed = &bytes[22..26];
+    let file_name_len = (bytes[26] as usize) | ((bytes[27] as usize) << 8);
+    let extra_len = (bytes[28] as usize) | ((bytes[29] as usize) << 8);
+    let file_name = &bytes[30..30 + file_name_len];
+    let mut offset = 30 + file_name_len;
+
+    assert_eq!(expected_file_name.as_bytes(), file_name);
+
+    offset += extra_len;
+
+    if is_file {
+        let mut decompress = Decompress::new(false);
+        let mut decompressed_data = vec![0u8; expected_content.len()];
+        decompress.decompress(
+            &bytes[offset..],
+            &mut decompressed_data,
+            FlushDecompress::Finish,
+        )?;
+
+        assert_eq!(decompressed_data, expected_content);
+
+        offset += decompress.total_in() as usize;
+    }
+
+    // Optional data descriptor
+    if &bytes[offset..offset + 4] == b"PK\x07\x08" {
+        offset += 4;
+
+        if (general_purpose_flags[0] & 0x08) == 0x08 {
+            offset += 12;
+
+            if size_compressed == b"\xff\xff\xff\xff" && size_uncompressed == b"\xff\xff\xff\xff" {
+                offset += 8;
+            }
+        }
+    }
+
+    ensure!(
+        &bytes[offset..offset + 4] == b"PK\x01\x02",
+        "Missing central directory file header"
+    );
+
+    let external_file_attributes = &bytes[offset + 38..offset + 42];
+    let file_attributes = u16::from_le_bytes(external_file_attributes[2..4].try_into()?);
+
+    assert_eq!(expected_file_attributes, file_attributes);
+
+    Ok(())
+}
+
+#[test]
+async fn test_zip_text_file() -> Result<(), Error> {
+    let mut target = Vec::new();
+
+    let source_contents = "bar";
+    let source = source_contents.as_bytes();
+
+    let mut zip_encoder = ZipEncoder::new(&mut target);
+    zip_encoder
+        .add_entry("foo.txt", 0, 0o755, FileType::Regular(source))
+        .await?;
+
+    zip_encoder.finish().await?;
+
+    check_zip_with_one_file(target, "foo.txt", b"bar", 0o100755, true)?;
+
+    Ok(())
+}
+
+#[test]
+async fn test_zip_symlink() -> Result<(), Error> {
+    let mut target = Vec::new();
+
+    let link_path = OsString::from("/dev/null");
+
+    let mut zip_encoder = ZipEncoder::new(&mut target);
+    zip_encoder
+        .add_entry("link", 0, 0o755, FileType::<&[u8]>::Symlink(link_path))
+        .await?;
+
+    zip_encoder.finish().await?;
+
+    check_zip_with_one_file(target, "link", b"/dev/null", 0o120755, true)?;
+
+    Ok(())
+}
+
+#[test]
+async fn test_zip_directory() -> Result<(), Error> {
+    let mut target = Vec::new();
+
+    let mut zip_encoder = ZipEncoder::new(&mut target);
+    zip_encoder
+        .add_entry("directory", 0, 0o755, FileType::<&[u8]>::Directory)
+        .await?;
+
+    zip_encoder.finish().await?;
+
+    check_zip_with_one_file(target, "directory/", b"", 0o40755, false)?;
+
+    Ok(())
+}
-- 
2.39.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH v4 backup 1/2] pxar: Adopt FileType enum when adding a zip entry
  2024-01-24 10:15 [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore Filip Schauer
                   ` (2 preceding siblings ...)
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 proxmox 3/3] compression: Add unit tests for the ZipEncoder Filip Schauer
@ 2024-01-24 10:15 ` Filip Schauer
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 backup 2/2] fix #4995: pxar: Include symlinks in zip file creation Filip Schauer
  2024-01-29 12:44 ` [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore Folke Gleumes
  5 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2024-01-24 10:15 UTC (permalink / raw)
  To: pbs-devel

Use a FileType enum instead of a boolean to specify whether a ZipEntry
is a directory or a regular file.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 pbs-client/src/pxar/extract.rs | 48 ++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
index af18ecfc..a7f94bf6 100644
--- a/pbs-client/src/pxar/extract.rs
+++ b/pbs-client/src/pxar/extract.rs
@@ -24,7 +24,7 @@ use proxmox_io::{sparse_copy, sparse_copy_async};
 use proxmox_sys::c_result;
 use proxmox_sys::fs::{create_path, CreateOptions};
 
-use proxmox_compression::zip::{ZipEncoder, ZipEntry};
+use proxmox_compression::zip::{FileType, ZipEncoder};
 
 use crate::pxar::dir_stack::PxarDirStack;
 use crate::pxar::metadata;
@@ -993,13 +993,13 @@ where
         let path = entry.path().strip_prefix(&prefix)?;
         if path != Path::new("/") {
             let metadata = entry.metadata();
-            let entry = ZipEntry::new(
+            zip.add_entry(
                 path,
                 metadata.stat.mtime.secs,
                 metadata.stat.mode as u16,
-                false,
-            );
-            zip.add_entry::<FileContents<T>>(entry, None).await?;
+                FileType::<FileContents<T>>::Directory,
+            )
+            .await?;
         }
 
         let mut decoder = dir.decode_full().await?;
@@ -1012,15 +1012,16 @@ where
             match entry.kind() {
                 EntryKind::File { .. } => {
                     log::debug!("adding '{}' to zip", path.display());
-                    let entry = ZipEntry::new(
-                        path,
-                        metadata.stat.mtime.secs,
-                        metadata.stat.mode as u16,
-                        true,
-                    );
-                    zip.add_entry(entry, decoder.contents())
+                    if let Some(contents) = decoder.contents() {
+                        zip.add_entry(
+                            path,
+                            metadata.stat.mtime.secs,
+                            metadata.stat.mode as u16,
+                            FileType::Regular(contents),
+                        )
                         .await
                         .context("could not send file entry")?;
+                    }
                 }
                 EntryKind::Hardlink(_) => {
                     let entry = root
@@ -1030,25 +1031,26 @@ where
                     let realfile = accessor.follow_hardlink(&entry).await?;
                     let metadata = realfile.entry().metadata();
                     log::debug!("adding '{}' to zip", path.display());
-                    let entry = ZipEntry::new(
-                        path,
-                        metadata.stat.mtime.secs,
-                        metadata.stat.mode as u16,
-                        true,
-                    );
-                    zip.add_entry(entry, decoder.contents())
+                    if let Some(contents) = decoder.contents() {
+                        zip.add_entry(
+                            path,
+                            metadata.stat.mtime.secs,
+                            metadata.stat.mode as u16,
+                            FileType::Regular(contents),
+                        )
                         .await
                         .context("could not send file entry")?;
+                    }
                 }
                 EntryKind::Directory => {
                     log::debug!("adding '{}' to zip", path.display());
-                    let entry = ZipEntry::new(
+                    zip.add_entry(
                         path,
                         metadata.stat.mtime.secs,
                         metadata.stat.mode as u16,
-                        false,
-                    );
-                    zip.add_entry::<FileContents<T>>(entry, None).await?;
+                        FileType::<FileContents<T>>::Directory,
+                    )
+                    .await?;
                 }
                 _ => {} // ignore all else
             };
-- 
2.39.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* [pbs-devel] [PATCH v4 backup 2/2] fix #4995: pxar: Include symlinks in zip file creation
  2024-01-24 10:15 [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore Filip Schauer
                   ` (3 preceding siblings ...)
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 backup 1/2] pxar: Adopt FileType enum when adding a zip entry Filip Schauer
@ 2024-01-24 10:15 ` Filip Schauer
  2024-01-29 12:44 ` [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore Folke Gleumes
  5 siblings, 0 replies; 7+ messages in thread
From: Filip Schauer @ 2024-01-24 10:15 UTC (permalink / raw)
  To: pbs-devel

Include symlinks when restoring a backup as a zip file.

Signed-off-by: Filip Schauer <f.schauer@proxmox.com>
---
 pbs-client/src/pxar/extract.rs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/pbs-client/src/pxar/extract.rs b/pbs-client/src/pxar/extract.rs
index a7f94bf6..96053e42 100644
--- a/pbs-client/src/pxar/extract.rs
+++ b/pbs-client/src/pxar/extract.rs
@@ -1052,6 +1052,17 @@ where
                     )
                     .await?;
                 }
+                EntryKind::Symlink(link) => {
+                    log::debug!("adding '{}' to zip", path.display());
+                    let realpath = Path::new(link);
+                    zip.add_entry(
+                        path,
+                        metadata.stat.mtime.secs,
+                        metadata.stat.mode as u16,
+                        FileType::<FileContents<T>>::Symlink(realpath.into()),
+                    )
+                    .await?;
+                }
                 _ => {} // ignore all else
             };
         }
-- 
2.39.2





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore
  2024-01-24 10:15 [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore Filip Schauer
                   ` (4 preceding siblings ...)
  2024-01-24 10:15 ` [pbs-devel] [PATCH v4 backup 2/2] fix #4995: pxar: Include symlinks in zip file creation Filip Schauer
@ 2024-01-29 12:44 ` Folke Gleumes
  5 siblings, 0 replies; 7+ messages in thread
From: Folke Gleumes @ 2024-01-29 12:44 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion



On Wed, 2024-01-24 at 11:15 +0100, Filip Schauer wrote:
> Include symlinks when restoring files from a backup as a zip file.
> 
> Resulting ZIP files were successfully tested on Linux with: zipinfo,
> unzip, unar
> 
> On Windows the extracted symlinks show up as regular files containing
> the path to the destination. As far as I am aware Windows Explorer
> does
> not support symlinks in ZIP files.
> 
> Changes since v3:
> * Move content to Regular FileType enum
> * Make ZipEntry private
> * Add take_reader function to FileType enum
> 
> Changes since v2:
> * Add a FileType enum and embed the symlink target into the enum.
> * Add unit tests for ZipEncoder to the proxmox-compression crate
> 
> Changes since v1:
> * Use P instead of &Path
> * Fix compile error due to misplaced comma
> * Check content before symlink_target, since regular files are more
>   common than symlinks
> 
> proxmox:
> 
> Filip Schauer (3):
>   compression: Refactor ZipEntry creation and add FileType enum
>   compression: Add support for symlinks in zip files
>   compression: Add unit tests for the ZipEncoder
> 
>  proxmox-compression/Cargo.toml   |   2 +-
>  proxmox-compression/src/zip.rs   | 159 +++++++++++++++++++----------
> --
>  proxmox-compression/tests/zip.rs | 123 ++++++++++++++++++++++++
>  3 files changed, 223 insertions(+), 61 deletions(-)
>  create mode 100644 proxmox-compression/tests/zip.rs
> 
> proxmox-backup:
> 
> Filip Schauer (2):
>   pxar: Adopt FileType enum when adding a zip entry
>   fix #4995: pxar: Include symlinks in zip file creation
> 
>  pbs-client/src/pxar/extract.rs | 59 +++++++++++++++++++++-----------
> --
>  1 file changed, 36 insertions(+), 23 deletions(-)
> 

Tested this with some handcrafted directory/symlink structures and my
home directory for some organic data. Restore was tested on Windows 11
and Debian 12.

Everything worked as intended.

Tested-by: Folke Gleumes <f.gleumes@proxmox.com>




^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-01-29 12:44 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-24 10:15 [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore Filip Schauer
2024-01-24 10:15 ` [pbs-devel] [PATCH v4 proxmox 1/3] compression: Refactor ZipEntry creation and add FileType enum Filip Schauer
2024-01-24 10:15 ` [pbs-devel] [PATCH v4 proxmox 2/3] compression: Add support for symlinks in zip files Filip Schauer
2024-01-24 10:15 ` [pbs-devel] [PATCH v4 proxmox 3/3] compression: Add unit tests for the ZipEncoder Filip Schauer
2024-01-24 10:15 ` [pbs-devel] [PATCH v4 backup 1/2] pxar: Adopt FileType enum when adding a zip entry Filip Schauer
2024-01-24 10:15 ` [pbs-devel] [PATCH v4 backup 2/2] fix #4995: pxar: Include symlinks in zip file creation Filip Schauer
2024-01-29 12:44 ` [pbs-devel] [PATCH v4 many] fix #4995: Include symlinks in zip file restore Folke Gleumes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal