[PATCH v1 proxmox-backup 3/3] datastore: introduce helper function to parse digest from S3ObjectKey

[Robert Obkircher <r.obkircher@proxmox.com>]

Do not rely on the platform-specific `Path::file_name` because S3
object keys are flat identifiers that always use `/` by convention.
It would, for example, be incorrect to ignore trailing slashes (or to
treat backslashes differently on Windows).

Signed-off-by: Robert Obkircher <r.obkircher@proxmox.com>
---
 pbs-datastore/src/datastore.rs | 28 ++++++------------------
 pbs-datastore/src/s3.rs        | 39 +++++++++++++++++++++++++++++++---
 2 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/pbs-datastore/src/datastore.rs b/pbs-datastore/src/datastore.rs
index bac5ce3b4..d106f5b52 100644
--- a/pbs-datastore/src/datastore.rs
+++ b/pbs-datastore/src/datastore.rs
@@ -48,7 +48,7 @@ use crate::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
 use crate::fixed_index::{FixedIndexReader, FixedIndexWriter};
 use crate::hierarchy::{ListGroups, ListGroupsType, ListNamespaces, ListNamespacesRecursive};
 use crate::index::IndexFile;
-use crate::s3::S3_CONTENT_PREFIX;
+use crate::s3::{S3_CONTENT_PREFIX, digest_from_object_key};
 use crate::task_tracking::{self, update_active_operations};
 use crate::{DataBlob, LocalDatastoreLruCache};
 
@@ -2675,36 +2675,22 @@ impl DataStore {
         &self,
         object_key: &S3ObjectKey,
     ) -> Option<(PathBuf, [u8; 32], bool)> {
-        // Check object is actually a chunk
-        let path = Path::new::<str>(object_key);
-        // file_name() should always be Some, as objects will have a filename
-        let file_name = path.file_name()?;
-        let bytes = file_name.as_bytes();
-        let bad_chunk = if is_bad_chunk_suffix(&bytes[64..]) {
+        let (file_name, digest, suffix) = digest_from_object_key(object_key)?;
+
+        let bad_chunk = if is_bad_chunk_suffix(suffix.as_bytes()) {
             true
-        } else if bytes.len() == 64 {
+        } else if suffix.is_empty() {
             false
         } else {
             return None; // unexpected suffix
         };
-        if !bytes.iter().take(64).all(u8::is_ascii_hexdigit) {
-            return None;
-        }
 
-        // Safe since contains valid ascii hexdigits only as checked above.
-        let digest_str = file_name.to_string_lossy();
-        let hexdigit_prefix = unsafe { digest_str.get_unchecked(0..4) };
         let mut chunk_path = self.base_path();
         chunk_path.push(".chunks");
-        chunk_path.push(hexdigit_prefix);
+        chunk_path.push(&file_name[..4]);
         chunk_path.push(file_name);
 
-        let mut digest_bytes = [0u8; 32];
-        let digest = file_name.as_bytes();
-        // safe to unwrap as already checked above
-        hex::decode_to_slice(&digest[..64], &mut digest_bytes).unwrap();
-
-        Some((chunk_path, digest_bytes, bad_chunk))
+        Some((chunk_path, digest, bad_chunk))
     }
 
     pub fn try_shared_chunk_store_lock(&self) -> Result<ProcessLockSharedGuard, Error> {
diff --git a/pbs-datastore/src/s3.rs b/pbs-datastore/src/s3.rs
index 09affc2e5..c4209d60a 100644
--- a/pbs-datastore/src/s3.rs
+++ b/pbs-datastore/src/s3.rs
@@ -1,6 +1,7 @@
 use std::path::{Path, PathBuf};
 
 use anyhow::{Error, bail, format_err};
+use hex::FromHex;
 
 use proxmox_s3_client::{DeleteObjectError, S3ObjectKey};
 
@@ -48,6 +49,16 @@ pub fn object_key_from_digest_with_suffix(
     S3ObjectKey::try_from(object_key_string.as_str())
 }
 
+/// Extract filename, digest, and suffix from the last part of an S3ObjectKey.
+pub fn digest_from_object_key(key: &S3ObjectKey) -> Option<(&str, [u8; 32], &str)> {
+    let filename = key.rsplit('/').next()?;
+    let (hex_digest, suffix) = filename.split_at_checked(64)?;
+
+    let digest = FromHex::from_hex(hex_digest).ok()?;
+
+    Some((filename, digest, suffix))
+}
+
 /// Log errors from delete objects api calls
 pub(crate) fn log_s3_delete_objects_errors(errors: &[DeleteObjectError]) {
     for error in errors {
@@ -96,7 +107,6 @@ fn test_object_key_from_path_incorrect_filename() {
 
 #[test]
 fn test_object_key_from_digest() {
-    use hex::FromHex;
     let digest =
         <[u8; 32]>::from_hex("bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8")
             .unwrap();
@@ -108,7 +118,6 @@ fn test_object_key_from_digest() {
 
 #[test]
 fn test_object_key_from_digest_with_suffix() {
-    use hex::FromHex;
     let digest =
         <[u8; 32]>::from_hex("bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8")
             .unwrap();
@@ -122,9 +131,33 @@ fn test_object_key_from_digest_with_suffix() {
 
 #[test]
 fn test_object_key_from_digest_with_invalid_suffix() {
-    use hex::FromHex;
     let digest =
         <[u8; 32]>::from_hex("bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8")
             .unwrap();
     assert!(object_key_from_digest_with_suffix(&digest, "/.0.bad").is_err());
 }
+
+#[test]
+fn test_digest_from_object_key() {
+    let hex = "bb9f8df61474d25e71fa00722318cd387396ca1736605e1248821cc0de3d3af8";
+    let digest = <[u8; 32]>::from_hex(hex).unwrap();
+    let k = |s: &str| S3ObjectKey::try_from(s).unwrap();
+
+    assert_eq!(digest_from_object_key(&k(&hex[1..])), None);
+    let invalid_hex = hex.replace("b", "X");
+    assert_eq!(digest_from_object_key(&k(&invalid_hex)), None);
+
+    assert_eq!(digest_from_object_key(&k(hex)), Some((hex, digest, "")));
+
+    let k1 = object_key_from_digest_with_suffix(&digest, "suffix1").unwrap();
+    assert_eq!(
+        digest_from_object_key(&k1),
+        Some((format!("{hex}suffix1").as_str(), digest, "suffix1"))
+    );
+
+    let k2 = S3ObjectKey::try_from(format!(".chunks/bb9f/{hex}.0.bad").as_str()).unwrap();
+    assert_eq!(
+        digest_from_object_key(&k2),
+        Some((format!("{hex}.0.bad").as_str(), digest, ".0.bad"))
+    );
+}
-- 
2.47.3