[pbs-devel] [PATCH pxar] fix `decode_entry` on special files

Wolfgang Bumiller w.bumiller at proxmox.com
Tue Dec 15 11:37:37 CET 2020


When using the random accessor to access FIFOs or sockets,
the ranged reader limits the data to only that entry, and
the `decode_entry` will never see a `PAYLOAD` or
`GOODBYE_TABLE` item to finish the entry.
Instead, it'll reach EOF and we need to handle this.
The accessor now tells the decoder to expect EOF as a valid
condition for ending the entry.

Signed-off-by: Wolfgang Bumiller <w.bumiller at proxmox.com>
---

The included testcase previously failed with
    "failed to decode entry for fifo0"

 src/accessor/mod.rs  |  2 +-
 src/decoder/mod.rs   | 52 +++++++++++++++++++++++++++++++++++---------
 tests/simple/main.rs | 46 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 89 insertions(+), 11 deletions(-)

diff --git a/src/accessor/mod.rs b/src/accessor/mod.rs
index 0ab03a6..d02dc13 100644
--- a/src/accessor/mod.rs
+++ b/src/accessor/mod.rs
@@ -229,7 +229,7 @@ async fn get_decoder<T: ReadAt>(
     entry_range: Range<u64>,
     path: PathBuf,
 ) -> io::Result<DecoderImpl<SeqReadAtAdapter<T>>> {
-    Ok(DecoderImpl::new_full(SeqReadAtAdapter::new(input, entry_range), path).await?)
+    Ok(DecoderImpl::new_full(SeqReadAtAdapter::new(input, entry_range), path, true).await?)
 }
 
 // NOTE: This performs the Decoder::read_next_item() behavior! Keep in mind when changing!
diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs
index fcc2dd9..2a5e79a 100644
--- a/src/decoder/mod.rs
+++ b/src/decoder/mod.rs
@@ -155,6 +155,10 @@ pub(crate) struct DecoderImpl<T> {
     path_lengths: Vec<usize>,
     state: State,
     with_goodbye_tables: bool,
+
+    /// 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,
 }
 
 enum State {
@@ -191,14 +195,18 @@ pub(crate) enum ItemResult {
 
 impl<I: SeqRead> DecoderImpl<I> {
     pub async fn new(input: I) -> io::Result<Self> {
-        Self::new_full(input, "/".into()).await
+        Self::new_full(input, "/".into(), false).await
     }
 
     pub(crate) fn input(&self) -> &I {
         &self.input
     }
 
-    pub(crate) async fn new_full(input: I, path: PathBuf) -> io::Result<Self> {
+    pub(crate) async fn new_full(
+        input: I,
+        path: PathBuf,
+        eof_after_entry: bool,
+    ) -> io::Result<Self> {
         let this = DecoderImpl {
             input,
             current_header: unsafe { mem::zeroed() },
@@ -210,6 +218,7 @@ impl<I: SeqRead> DecoderImpl<I> {
             path_lengths: Vec::new(),
             state: State::Begin,
             with_goodbye_tables: false,
+            eof_after_entry,
         };
 
         // this.read_next_entry().await?;
@@ -383,7 +392,14 @@ impl<I: SeqRead> DecoderImpl<I> {
 
             self.current_header = unsafe { mem::zeroed() };
 
-            while self.read_next_item().await? != ItemResult::Entry {}
+            loop {
+                match self.read_next_item_or_eof().await? {
+                    Some(ItemResult::Entry) => break,
+                    Some(ItemResult::Attribute) => continue,
+                    None if self.eof_after_entry => break,
+                    None => io_bail!("unexpected EOF in entry"),
+                }
+            }
 
             if self.entry.is_dir() {
                 self.path_lengths
@@ -402,24 +418,40 @@ impl<I: SeqRead> DecoderImpl<I> {
             .ok_or_else(|| io_format_err!("unexpected EOF"))
     }
 
+    async fn read_next_item(&mut self) -> io::Result<ItemResult> {
+        match self.read_next_item_or_eof().await? {
+            Some(item) => Ok(item),
+            None => io_bail!("unexpected EOF"),
+        }
+    }
+
+    // NOTE: The random accessor will decode FIFOs and Sockets in a decoder instance with a ranged
+    // reader so there is no PAYLOAD or GOODBYE TABLE to "end" an entry.
+    //
     // NOTE: This behavior method is also recreated in the accessor's `get_decoder_at_filename`
     // function! Keep in mind when changing!
-    async fn read_next_item(&mut self) -> io::Result<ItemResult> {
-        self.read_next_header().await?;
-        self.read_current_item().await
+    async fn read_next_item_or_eof(&mut self) -> io::Result<Option<ItemResult>> {
+        match self.read_next_header_or_eof().await? {
+            Some(()) => self.read_current_item().await.map(Some),
+            None => Ok(None),
+        }
     }
 
-    async fn read_next_header(&mut self) -> io::Result<()> {
+    async fn read_next_header_or_eof(&mut self) -> io::Result<Option<()>> {
         let dest = unsafe {
             std::slice::from_raw_parts_mut(
                 &mut self.current_header as *mut Header as *mut u8,
                 size_of_val(&self.current_header),
             )
         };
-        seq_read_exact(&mut self.input, dest).await?;
-        self.current_header.check_header_size()?;
 
-        Ok(())
+        match seq_read_exact_or_eof(&mut self.input, dest).await? {
+            Some(()) => {
+                self.current_header.check_header_size()?;
+                Ok(Some(()))
+            }
+            None => Ok(None),
+        }
     }
 
     /// Read the next item, the header is already loaded.
diff --git a/tests/simple/main.rs b/tests/simple/main.rs
index c73ca10..f15a0f5 100644
--- a/tests/simple/main.rs
+++ b/tests/simple/main.rs
@@ -42,6 +42,9 @@ fn test1() {
 
     assert!(!file.is_empty(), "encoder did not write any data");
 
+    // may be useful for testing...
+    // std::fs::write("myarchive.pxar", &file).expect("failed to write out test archive");
+
     let mut input = &file[..];
     let mut decoder = decoder::Decoder::from_std(&mut input).expect("failed to create decoder");
     let decoded_fs =
@@ -53,6 +56,7 @@ fn test1() {
         .expect("failed to create random access reader for encoded archive");
 
     check_bunzip2(&accessor);
+    check_run_special_files(&accessor);
 }
 
 fn check_bunzip2(accessor: &accessor::Accessor<&[u8]>) {
@@ -85,3 +89,45 @@ fn check_bunzip2(accessor: &accessor::Accessor<&[u8]>) {
 
     assert_eq!(content, "This is the bzip2 executable");
 }
+
+fn check_run_special_files(accessor: &accessor::Accessor<&[u8]>) {
+    let rundir = accessor
+        .open_root()
+        .expect("failed to open root of encoded archive")
+        .lookup("run")
+        .expect("failed to open /run in encoded archive")
+        .expect("missing /run in encoded archive")
+        .enter_directory()
+        .expect("expected /run to be a directory in the test archive");
+
+    assert_eq!(rundir.entry_count(), 2, "expected 2 entries in /run");
+
+    let mut rd = rundir.read_dir();
+    let fifo0 = rd
+        .next()
+        .expect("expected 'fifo0' entry in rundir")
+        .expect("failed to get first (fifo0) entry in test archive /run directory");
+    assert_eq!(
+        fifo0.file_name(),
+        Path::new("fifo0"),
+        "expected first file in /run to be fifo0"
+    );
+
+    let _entry = fifo0
+        .decode_entry()
+        .expect("failed to decode entry for fifo0");
+
+    let sock0 = rd
+        .next()
+        .expect("expected 'sock0' entry in rundir")
+        .expect("failed to get second (sock0) entry in test archive /run directory");
+    assert_eq!(
+        sock0.file_name(),
+        Path::new("sock0"),
+        "expected second file in /run to be sock0"
+    );
+
+    let _entry = sock0
+        .decode_entry()
+        .expect("failed to decode entry for sock0");
+}
-- 
2.20.1





More information about the pbs-devel mailing list