[PATCH proxmox-backup] fix #8186: pxar: fix off-by-one error in dynamic entry range end check

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

With change-detection-mode metadata, the pxar archiver tires to reuse
unchanged files instead of re-encoding them. This happens by
comparing the file metadata against the matching file from the
previous metadata archive. If reusable, the range of dynamic entries
to reuse from the previous payload archive are determined. To avoid
reusing many partial chunks with only little actual payload, the
archiver uses a lookahead cache and keeps a continuous range of
reusable dynamic entries in order to decide whether to reuse or
re-encode cached entries.

The check for dynamic entries included in the files payload range was
however off-by-one, checking only if the end of the dynamic entry is
smaller than the range end. This however leads to unnecessary
inclusion of the next entry in case the end was equal, with the
consequence that files which potentially could have been reused might
get re-encoded, if the payload padding exceeds the padding threshold.

Both, dynamic entries end and range end are exclusive, so check for
equality in order to only include strictly required dynamic entries.

Fixes: https://bugzilla.proxmox.com/show_bug.cgi?id=8186
Reported-by: <moamarch@zoho.com>
Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 pbs-client/src/pxar/create.rs | 44 ++++++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/pbs-client/src/pxar/create.rs b/pbs-client/src/pxar/create.rs
index 4d25341a7..b1266b1fc 100644
--- a/pbs-client/src/pxar/create.rs
+++ b/pbs-client/src/pxar/create.rs
@@ -1468,7 +1468,7 @@ fn lookup_dynamic_entries(
         };
         indices.push(reusable_dynamic_entry);
 
-        if range.end < end {
+        if range.end <= end {
             padding_end = end - range.end;
             break;
         }
@@ -2187,6 +2187,48 @@ mod tests {
         let _ = std::fs::remove_file(&path);
     }
 
+    #[test]
+    fn lookup_dynamic_entries_range_end_check() {
+        use pbs_datastore::dynamic_index::{DynamicIndexReader, DynamicIndexWriter};
+
+        let testdir = create_testdir("lookup_dynamic_entries_range_end_check")
+            .expect("Should be able to create test dir");
+        let path = testdir.join("lookup_dynamic_entries.didx");
+        let _ = std::fs::remove_file(&path);
+
+        let mut writer = DynamicIndexWriter::create(&path).unwrap();
+        writer.add_chunk(1024, &[0xAAu8; 32]).unwrap();
+        writer.add_chunk(2048, &[0xBBu8; 32]).unwrap();
+        writer.add_chunk(3072, &[0xCCu8; 32]).unwrap();
+        let _csum = writer.close().unwrap();
+
+        let reader = DynamicIndexReader::open(&path).unwrap();
+        for (range, count, start_padding, end_padding) in [
+            (0..1024, 1, 0, 0),
+            (16..1024, 1, 16, 0),
+            (0..1000, 1, 0, 24),
+            (1024..2048, 1, 0, 0),
+            (0..2048, 2, 0, 0),
+            (0..2049, 3, 0, 1023),
+        ] {
+            let (indices, start, end) = super::lookup_dynamic_entries(&reader, &range).unwrap();
+            assert_eq!(
+                indices.len(),
+                count,
+                "incorrect chunk count: got {}, expected {count} for {range:?}",
+                indices.len(),
+            );
+            assert_eq!(
+                start, start_padding,
+                "incorrect start padding: got {start}, expected {start_padding} for {range:?}"
+            );
+            assert_eq!(
+                end, end_padding,
+                "incorrect end padding: got {end}, expected {end_padding} for {range:?}"
+            );
+        }
+    }
+
     /// Inversions that span cache ranges must be caught by the global tracking; a per-range
     /// implementation would have reset on flush and accepted the second offset, after which
     /// the encoder strict check would still abort the backup.
-- 
2.47.3