[pbs-devel] [PATCH v5 pxar 8/28] fix #3174: enc/dec: introduce pxar format version 2

[Christian Ebner <c.ebner@proxmox.com>]

Prefix pxar archives with format version 2 with a header containing the
corresponding version 2 hash.

The main intention for this is to early detect the incompatible version
for older pxar binaries, not compatible with this format version.

Further, encoder and decoder states are extended to include the version
and check consistency accordingly.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
Changes since version 4:
- not present in version 4

 examples/mk-format-hashes.rs |  5 +++++
 src/decoder/mod.rs           | 30 +++++++++++++++++++++++++++++-
 src/encoder/aio.rs           | 22 ++++++++++++++++------
 src/encoder/mod.rs           | 29 +++++++++++++++++++++++++++++
 src/encoder/sync.rs          | 11 ++++++++---
 src/format/mod.rs            | 11 ++++++++++-
 6 files changed, 97 insertions(+), 11 deletions(-)

diff --git a/examples/mk-format-hashes.rs b/examples/mk-format-hashes.rs
index 7fb938d..61f4773 100644
--- a/examples/mk-format-hashes.rs
+++ b/examples/mk-format-hashes.rs
@@ -1,6 +1,11 @@
 use pxar::format::hash_filename;
 
 const CONSTANTS: &[(&str, &str, &str)] = &[
+    (
+        "Pxar format version 2 entry, fallback to version 1 if not present",
+        "PXAR_FORMAT_VERSION_2",
+        "__PROXMOX_FORMAT_VERSION_V2__",
+    ),
     (
         "Beginning of an entry (current version).",
         "PXAR_ENTRY",
diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs
index 4eea633..b7f6c39 100644
--- a/src/decoder/mod.rs
+++ b/src/decoder/mod.rs
@@ -160,6 +160,7 @@ pub(crate) struct DecoderImpl<T> {
     /// The random access code uses decoders for sub-ranges which may not end in a `PAYLOAD` for
     /// entries like FIFOs or sockets, so there we explicitly allow an item to terminate with EOF.
     eof_after_entry: bool,
+    version: format::FormatVersion,
 }
 
 enum State {
@@ -220,6 +221,7 @@ impl<I: SeqRead> DecoderImpl<I> {
             state: State::Begin,
             with_goodbye_tables: false,
             eof_after_entry,
+            version: format::FormatVersion::default(),
         };
 
         // this.read_next_entry().await?;
@@ -236,7 +238,16 @@ impl<I: SeqRead> DecoderImpl<I> {
         loop {
             match self.state {
                 State::Eof => return Ok(None),
-                State::Begin => return self.read_next_entry().await.map(Some),
+                State::Begin => {
+                    match self.read_next_entry_header_or_eof().await? {
+                        Some(header) if header.htype == format::PXAR_FORMAT_VERSION_2 => {
+                            self.version = format::FormatVersion::V2;
+                            return self.read_next_entry().await.map(Some);
+                        }
+                        Some(header) => return self.read_next_entry_payload_or_eof(header).await,
+                        None => return Err(io_format_err!("unexpected EOF")),
+                    }
+                }
                 State::Default => {
                     // we completely finished an entry, so now we're going "up" in the directory
                     // hierarchy and parse the next PXAR_FILENAME or the PXAR_GOODBYE:
@@ -277,6 +288,9 @@ impl<I: SeqRead> DecoderImpl<I> {
             match self.current_header.htype {
                 format::PXAR_FILENAME => return self.handle_file_entry().await,
                 format::PXAR_APPENDIX_REF => {
+                    if self.version == format::FormatVersion::Default {
+                        io_bail!("unsupported appendix reference in default version");
+                    }
                     self.state = State::Default;
                     return self.handle_appendix_ref_entry().await
                 }
@@ -296,6 +310,9 @@ impl<I: SeqRead> DecoderImpl<I> {
                     }
                 }
                 format::PXAR_APPENDIX => {
+                    if self.version == format::FormatVersion::Default {
+                        io_bail!("unsupported appendix in default version");
+                    }
                     self.state = State::Default;
                     return Ok(Some(self.entry.take()));
                 }
@@ -378,6 +395,14 @@ impl<I: SeqRead> DecoderImpl<I> {
     }
 
     async fn read_next_entry_or_eof(&mut self) -> io::Result<Option<Entry>> {
+        if let Some(header) = self.read_next_entry_header_or_eof().await? {
+            self.read_next_entry_payload_or_eof(header).await
+        } else {
+            Ok(None)
+        }
+    }
+
+    async fn read_next_entry_header_or_eof(&mut self) -> io::Result<Option<Header>> {
         self.state = State::Default;
         self.entry.clear_data();
 
@@ -387,7 +412,10 @@ impl<I: SeqRead> DecoderImpl<I> {
         };
 
         header.check_header_size()?;
+        Ok(Some(header))
+    }
 
+    async fn read_next_entry_payload_or_eof(&mut self, header: Header) -> io::Result<Option<Entry>> {
         if header.htype == format::PXAR_HARDLINK {
             // The only "dangling" header without an 'Entry' in front of it because it does not
             // carry its own metadata.
diff --git a/src/encoder/aio.rs b/src/encoder/aio.rs
index 5a833c5..b750c8d 100644
--- a/src/encoder/aio.rs
+++ b/src/encoder/aio.rs
@@ -24,8 +24,9 @@ impl<'a, T: tokio::io::AsyncWrite + 'a> Encoder<'a, TokioWriter<T>> {
     pub async fn from_tokio(
         output: T,
         metadata: &Metadata,
+        version: format::FormatVersion,
     ) -> io::Result<Encoder<'a, TokioWriter<T>>> {
-        Encoder::new(TokioWriter::new(output), metadata).await
+        Encoder::new(TokioWriter::new(output), metadata, version).await
     }
 }
 
@@ -46,9 +47,13 @@ impl<'a> Encoder<'a, TokioWriter<tokio::fs::File>> {
 
 impl<'a, T: SeqWrite + 'a> Encoder<'a, T> {
     /// Create an asynchronous encoder for an output implementing our internal write interface.
-    pub async fn new(output: T, metadata: &Metadata) -> io::Result<Encoder<'a, T>> {
+    pub async fn new(
+        output: T,
+        metadata: &Metadata,
+        version: format::FormatVersion,
+    ) -> io::Result<Encoder<'a, T>> {
         Ok(Self {
-            inner: encoder::EncoderImpl::new(output.into(), metadata).await?,
+            inner: encoder::EncoderImpl::new(output.into(), metadata, version).await?,
         })
     }
 
@@ -299,6 +304,7 @@ mod test {
     use std::task::{Context, Poll};
 
     use super::Encoder;
+    use crate::format;
     use crate::Metadata;
 
     struct DummyOutput;
@@ -321,9 +327,13 @@ mod test {
     /// Assert that `Encoder` is `Send`
     fn send_test() {
         let test = async {
-            let mut encoder = Encoder::new(DummyOutput, &Metadata::dir_builder(0o700).build())
-                .await
-                .unwrap();
+            let mut encoder = Encoder::new(
+                DummyOutput,
+                &Metadata::dir_builder(0o700).build(),
+                format::FormatVersion::Default,
+            )
+            .await
+            .unwrap();
             {
                 let mut dir = encoder
                     .create_directory("baba", &Metadata::dir_builder(0o700).build())
diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs
index c33b2c3..b3c1a89 100644
--- a/src/encoder/mod.rs
+++ b/src/encoder/mod.rs
@@ -247,6 +247,7 @@ pub async fn encoded_size(filename: &std::ffi::CStr, metadata: &Metadata) -> io:
         file_copy_buffer: Arc::new(Mutex::new(unsafe {
             crate::util::vec_new_uninitialized(1024 * 1024)
         })),
+        version: format::FormatVersion::Default,
     };
 
     this.start_file_do(Some(metadata), filename.to_bytes())
@@ -356,6 +357,8 @@ pub(crate) struct EncoderImpl<'a, T: SeqWrite + 'a> {
     /// Since only the "current" entry can be actively writing files, we share the file copy
     /// buffer.
     file_copy_buffer: Arc<Mutex<Vec<u8>>>,
+    /// Pxar format version to encode
+    version: format::FormatVersion,
 }
 
 impl<'a, T: SeqWrite + 'a> Drop for EncoderImpl<'a, T> {
@@ -377,6 +380,7 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
     pub async fn new(
         output: EncoderOutput<'a, T>,
         metadata: &Metadata,
+        version: format::FormatVersion,
     ) -> io::Result<EncoderImpl<'a, T>> {
         if !metadata.is_dir() {
             io_bail!("directory metadata must contain the directory mode flag");
@@ -389,8 +393,10 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
             file_copy_buffer: Arc::new(Mutex::new(unsafe {
                 crate::util::vec_new_uninitialized(1024 * 1024)
             })),
+            version,
         };
 
+        this.encode_format_version().await?;
         this.encode_metadata(metadata).await?;
         this.state.files_offset = this.position();
 
@@ -509,6 +515,9 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
         appendix_ref_offset: AppendixRefOffset,
         file_size: u64,
     ) -> io::Result<()> {
+        if self.version == format::FormatVersion::Default {
+            io_bail!("unable to add appendix reference for default format version");
+        }
         self.check()?;
 
         let offset = self.position();
@@ -544,6 +553,9 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
         &mut self,
         full_size: AppendixRefOffset,
     ) -> io::Result<AppendixStartOffset> {
+        if self.version == format::FormatVersion::Default {
+            io_bail!("unable to add appendix for default format version");
+        }
         self.check()?;
 
         let data = &full_size.raw().to_le_bytes().to_vec();
@@ -740,6 +752,7 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
             parent: Some(&mut self.state),
             finished: false,
             file_copy_buffer,
+            version: self.version.clone(),
         })
     }
 
@@ -755,6 +768,22 @@ impl<'a, T: SeqWrite + 'a> EncoderImpl<'a, T> {
         Ok(())
     }
 
+    async fn encode_format_version(&mut self) -> io::Result<()> {
+        if self.state.write_position != 0 {
+            io_bail!("format version must be encoded at the beginning of an archive");
+        }
+
+        let version = match self.version {
+            format::FormatVersion::Default => return Ok(()),
+            format::FormatVersion::V2 => format::PXAR_FORMAT_VERSION_2,
+        };
+
+        let header = format::Header::with_content_size(version, 0);
+        header.check_header_size()?;
+
+        seq_write_struct(self.output.as_mut(), header, &mut self.state.write_position).await
+    }
+
     async fn encode_metadata(&mut self, metadata: &Metadata) -> io::Result<()> {
         seq_write_pxar_struct_entry(
             self.output.as_mut(),
diff --git a/src/encoder/sync.rs b/src/encoder/sync.rs
index 5ede554..f25afb7 100644
--- a/src/encoder/sync.rs
+++ b/src/encoder/sync.rs
@@ -28,7 +28,11 @@ impl<'a, T: io::Write + 'a> Encoder<'a, StandardWriter<T>> {
     /// Encode a `pxar` archive into a regular `std::io::Write` output.
     #[inline]
     pub fn from_std(output: T, metadata: &Metadata) -> io::Result<Encoder<'a, StandardWriter<T>>> {
-        Encoder::new(StandardWriter::new(output), metadata)
+        Encoder::new(
+            StandardWriter::new(output),
+            metadata,
+            format::FormatVersion::Default,
+        )
     }
 }
 
@@ -41,6 +45,7 @@ impl<'a> Encoder<'a, StandardWriter<std::fs::File>> {
         Encoder::new(
             StandardWriter::new(std::fs::File::create(path.as_ref())?),
             metadata,
+            format::FormatVersion::Default,
         )
     }
 }
@@ -50,9 +55,9 @@ impl<'a, T: SeqWrite + 'a> Encoder<'a, T> {
     ///
     /// Note that the `output`'s `SeqWrite` implementation must always return `Poll::Ready` and is
     /// not allowed to use the `Waker`, as this will cause a `panic!`.
-    pub fn new(output: T, metadata: &Metadata) -> io::Result<Self> {
+    pub fn new(output: T, metadata: &Metadata, version: format::FormatVersion) -> io::Result<Self> {
         Ok(Self {
-            inner: poll_result_once(encoder::EncoderImpl::new(output.into(), metadata))?,
+            inner: poll_result_once(encoder::EncoderImpl::new(output.into(), metadata, version))?,
         })
     }
 
diff --git a/src/format/mod.rs b/src/format/mod.rs
index 8016ab1..7bffe98 100644
--- a/src/format/mod.rs
+++ b/src/format/mod.rs
@@ -44,7 +44,7 @@
 //!   * final goodbye table
 //!   * `APPENDIX_TAIL`     -- marks the end of an archive containing a APPENDIX section
 
-use std::cmp::Ordering;
+use std::cmp::{Ordering, PartialEq};
 use std::ffi::{CStr, OsStr};
 use std::fmt;
 use std::fmt::Display;
@@ -88,6 +88,8 @@ pub mod mode {
 }
 
 // Generated by `cargo run --example mk-format-hashes`
+/// Pxar format version 2 entry, fallback to version 1 if not present
+pub const PXAR_FORMAT_VERSION_2: u64 = 0xa0c3af8478917dbb;
 /// Beginning of an entry (current version).
 pub const PXAR_ENTRY: u64 = 0xd5956474e588acef;
 /// Previous version of the entry struct
@@ -118,6 +120,13 @@ pub const PXAR_GOODBYE_TAIL_MARKER: u64 = 0xef5eed5b753e1555;
 /// Marks the end of an archive containing an appendix section
 pub const PXAR_APPENDIX_TAIL: u64 = 0x5b1b9abb7ae454f1;
 
+#[derive(Clone, Default, PartialEq)]
+pub enum FormatVersion {
+    #[default]
+    Default,
+    V2,
+}
+
 #[derive(Debug, Endian)]
 #[repr(C)]
 pub struct Header {
-- 
2.39.2