[pbs-devel] [PATCH proxmox-backup v3 3/6] chunk store: return chunk extension and restrict chunk filename check

[Christian Ebner]

Clearly distinguish the cases for regular, bad and chunk marker files
by returning the respective extension as ChunkExt enum variant in the
chunk iterator.

Restrict the filenames to not only consist of 64 hexdigits and match
size and ending, but rather check that each individual byte in the
filename matches, including the bad chunk number and the marker file
extension.

Directory entries which no longer match the stricter criteria are now
skipped over.

Signed-off-by: Christian Ebner <c.ebner at proxmox.com>
---
changes since version 2:
- refactor to not duplicate the is_ascii_hexdigit check for better
  readability
- stricter extension checks to truly match all expected bytes in
  fileaname
- inline trivial extension type checks instead of introducing helpers

 pbs-datastore/src/chunk_store.rs | 58 ++++++++++++++++++++++++++------
 1 file changed, 47 insertions(+), 11 deletions(-)

diff --git a/pbs-datastore/src/chunk_store.rs b/pbs-datastore/src/chunk_store.rs
index 084f62b80..5148d6c46 100644
--- a/pbs-datastore/src/chunk_store.rs
+++ b/pbs-datastore/src/chunk_store.rs
@@ -280,7 +280,11 @@ impl ChunkStore {
         &self,
     ) -> Result<
         impl std::iter::FusedIterator<
-            Item = (Result<proxmox_sys::fs::ReadDirEntry, Error>, usize, bool),
+            Item = (
+                Result<proxmox_sys::fs::ReadDirEntry, Error>,
+                usize,
+                ChunkExt,
+            ),
         >,
         Error,
     > {
@@ -315,21 +319,47 @@ impl ChunkStore {
                         Some(Ok(entry)) => {
                             // skip files if they're not a hash
                             let bytes = entry.file_name().to_bytes();
-                            if bytes.len() != 64 && bytes.len() != 64 + ".0.bad".len() {
+
+                            if bytes.len() < 64 {
                                 continue;
                             }
+
                             if !bytes.iter().take(64).all(u8::is_ascii_hexdigit) {
                                 continue;
                             }
 
-                            let bad = bytes.ends_with(b".bad");
-                            return Some((Ok(entry), percentage, bad));
+                            // regular chunk
+                            if bytes.len() == 64 {
+                                return Some((Ok(entry), percentage, ChunkExt::None));
+                            }
+
+                            // i-th bad chunk
+                            if bytes.len() == 64 + ".i.bad".len()
+                                && bytes[64] == b'.'
+                                && bytes[65] >= b'0'
+                                && bytes[65] <= b'9'
+                                && bytes[66] == b'.'
+                                && bytes.ends_with(b"bad")
+                            {
+                                return Some((Ok(entry), percentage, ChunkExt::Bad));
+                            }
+
+                            // chunk marker file
+                            let marker_ext_bytes = USING_MARKER_FILENAME_EXT.as_bytes();
+                            if bytes.len() == 64 + 1 + marker_ext_bytes.len()
+                                && bytes[64] == b'.'
+                                && bytes.ends_with(marker_ext_bytes)
+                            {
+                                return Some((Ok(entry), percentage, ChunkExt::UsedMarker));
+                            }
+
+                            continue;
                         }
                         Some(Err(err)) => {
                             // stop after first error
                             done = true;
                             // and pass the error through:
-                            return Some((Err(err), percentage, false));
+                            return Some((Err(err), percentage, ChunkExt::None));
                         }
                         None => (), // open next directory
                     }
@@ -362,7 +392,7 @@ impl ChunkStore {
                         return Some((
                             Err(format_err!("unable to read subdir '{subdir}' - {err}")),
                             percentage,
-                            false,
+                            ChunkExt::None,
                         ));
                     }
                 }
@@ -397,7 +427,8 @@ impl ChunkStore {
         let mut last_percentage = 0;
         let mut chunk_count = 0;
 
-        for (entry, percentage, bad) in self.get_chunk_store_iterator()? {
+        for (entry, percentage, chunk_ext) in self.get_chunk_store_iterator()? {
+            let bad = chunk_ext == ChunkExt::Bad;
             if last_percentage != percentage {
                 last_percentage = percentage;
                 info!("processed {percentage}% ({chunk_count} chunks)");
@@ -428,10 +459,7 @@ impl ChunkStore {
                     drop(lock);
                     continue;
                 }
-                if filename
-                    .to_bytes()
-                    .ends_with(USING_MARKER_FILENAME_EXT.as_bytes())
-                {
+                if chunk_ext == ChunkExt::UsedMarker {
                     unlinkat(Some(dirfd), filename, UnlinkatFlags::NoRemoveDir).map_err(|err| {
                         format_err!("unlinking chunk using marker {filename:?} failed - {err}")
                     })?;
@@ -903,6 +931,14 @@ impl ChunkStore {
     }
 }
 
+#[derive(PartialEq)]
+/// Chunk iterator directory entry filename extension
+enum ChunkExt {
+    None,
+    Bad,
+    UsedMarker,
+}
+
 #[test]
 fn test_chunk_store1() {
     let mut path = std::fs::canonicalize(".").unwrap(); // we need absolute path
-- 
2.47.3