]> git.proxmox.com Git - proxmox-backup.git/commitdiff
datastore: data blob: allow checking for zstd internal buffer-to-small error
authorDominik Csapak <d.csapak@proxmox.com>
Mon, 5 Aug 2024 09:24:12 +0000 (11:24 +0200)
committerThomas Lamprecht <t.lamprecht@proxmox.com>
Wed, 7 Aug 2024 16:54:04 +0000 (18:54 +0200)
We want to check the error code of zstd not to be 'Destination buffer
to small' (dstSize_tooSmall),  but currently there is no practical API
that is also public. So we introduce a helper that uses the internal
logic of zstd to determine the error.

Since this is not guaranteed to be a stable api, add a test for that
so we catch that error early on build. This should be fine, as long as
the zstd behavior only changes with e.g. major debian upgrades, which
is normally the only time where the zstd version is updated.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
 [ TL: re-order fn, rename test and reword comments ]
Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Cargo.toml
debian/control
pbs-datastore/Cargo.toml
pbs-datastore/src/data_blob.rs

index 2b51ec829ec46a8dffddbb1199737b6423f05ded..275e3c959c05fa9701a9e3f04b093a51bebfa413 100644 (file)
@@ -158,6 +158,7 @@ url = "2.1"
 walkdir = "2"
 xdg = "2.2"
 zstd = { version = "0.12", features = [ "bindgen" ] }
+zstd-safe = "6.0"
 
 [dependencies]
 anyhow.workspace = true
index cc8f759605ddf205da29553a56b13392b7f6fe04..18fab1d1f6e8e643737c5cf6ce523b0be1bd78ab 100644 (file)
@@ -145,6 +145,7 @@ Build-Depends: bash-completion,
                librust-xdg-2+default-dev (>= 2.2-~~),
                librust-zstd-0.12+bindgen-dev,
                librust-zstd-0.12+default-dev,
+               librust-zstd-safe-6-dev,
                libsgutils2-dev,
                libstd-rust-dev,
                libsystemd-dev (>= 246-~~),
index d8997e1dcb06282ec13e05dd65b752d01eb18cbd..494c231b02bcc2982390c6e691bccbfb4c44c064 100644 (file)
@@ -23,6 +23,7 @@ tokio = { workspace = true, features = [] }
 tracing.workspace = true
 walkdir.workspace = true
 zstd.workspace = true
+zstd-safe.workspace = true
 
 pathpatterns.workspace = true
 pxar.workspace = true
index a3a41c5e55ea9bcadd26db550c2b5106c76c13ee..ec948bfe18e8b8ed34ed658ba163b3d45969c74d 100644 (file)
@@ -563,11 +563,23 @@ impl<'a, 'b> DataChunkBuilder<'a, 'b> {
     }
 }
 
+/// Check if the error code returned by `zstd_safe::compress`, or anything else that does FFI calls
+/// into zstd code, was `70` 'Destination buffer is too small' by subtracting the error code from
+/// `0` (with underflow), see `ERR_getErrorCode` in
+/// https://github.com/facebook/zstd/blob/dev/lib/common/error_private.h
+///
+/// There is a test below to ensure we catch any change in the interface or internal value.
+fn zstd_error_is_target_too_small(err: usize) -> bool {
+    let (real_code, _) = 0usize.overflowing_sub(err);
+    // see ZSTD_ErrorCode in https://github.com/facebook/zstd/blob/dev/lib/zstd_errors.h
+    real_code == 70 // ZSTD_error_dstSize_tooSmall
+}
+
 #[cfg(test)]
 mod test {
     use pbs_tools::crypt_config::CryptConfig;
 
-    use super::DataChunkBuilder;
+    use super::{zstd_error_is_target_too_small, DataChunkBuilder};
 
     const TEST_DATA_LEN: usize = 50;
 
@@ -640,4 +652,20 @@ mod test {
             .expect("cannot decode encrypted, compressed chunk");
         assert_eq!(data, data_decoded);
     }
+
+    #[test]
+    /// test for the error code internal logic of zstd so we catch any interface/value changes on
+    /// (package) compile time
+    fn zstd_assert_dst_size_to_small_error_code_abi() {
+        let data = &build_test_data();
+        let mut target = Vec::new();
+        match zstd_safe::compress(&mut target, data, 1) {
+            Ok(_) => panic!("unexpected success with zero-sized buffer"),
+            Err(err) => {
+                if !zstd_error_is_target_too_small(err) {
+                    panic!("unexpected error code {err}, check test validity and zstd for changes!");
+                }
+            }
+        }
+    }
 }