From 60244b60dd6b29208cf346a1f54b438c4c367f3b Mon Sep 17 00:00:00 2001 From: trinity-1686a Date: Tue, 17 Mar 2026 18:16:37 +0000 Subject: [PATCH] don't panic on missing checksum (fix #1387) (#1389) fix https://git.deuxfleurs.fr/Deuxfleurs/garage/issues/1387 Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/1389 Reviewed-by: Alex Co-authored-by: trinity-1686a Co-committed-by: trinity-1686a --- src/api/common/signature/checksum.rs | 49 ++++++++++++++++++++++------ src/api/s3/copy.rs | 2 +- src/api/s3/multipart.rs | 2 +- src/api/s3/put.rs | 8 ++--- 4 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/api/common/signature/checksum.rs b/src/api/common/signature/checksum.rs index 88d72347..20f4bd7e 100644 --- a/src/api/common/signature/checksum.rs +++ b/src/api/common/signature/checksum.rs @@ -11,6 +11,7 @@ use http::{HeaderMap, HeaderName, HeaderValue}; use garage_util::data::*; use super::*; +use crate::common_error::CommonError; pub use garage_model::s3::object_table::{ChecksumAlgorithm, ChecksumValue}; @@ -201,7 +202,7 @@ impl Checksums { } if let Some(extra) = expected.extra { let algo = extra.algorithm(); - let calculated = self.extract(Some(algo)); + let calculated = self.extract(Some(algo))?; if calculated != Some(extra) { return Err(Error::InvalidDigest(format!( "Failed to validate checksum for algorithm {:?}: calculated {:?}, expected {:?}", @@ -212,17 +213,45 @@ impl Checksums { Ok(()) } - pub fn extract(&self, algo: Option) -> Option { - match algo { + pub fn extract(&self, algo: Option) -> Result, Error> { + Ok(match algo { None => None, - Some(ChecksumAlgorithm::Crc32) => Some(ChecksumValue::Crc32(self.crc32.unwrap())), - Some(ChecksumAlgorithm::Crc32c) => Some(ChecksumValue::Crc32c(self.crc32c.unwrap())), - Some(ChecksumAlgorithm::Crc64Nvme) => { - Some(ChecksumValue::Crc64Nvme(self.crc64nvme.unwrap())) + Some(ChecksumAlgorithm::Crc32) => { + Some(ChecksumValue::Crc32(self.crc32.ok_or_else(|| { + CommonError::BadRequest( + "Requested checksum verification without providing checksum".to_string(), + ) + })?)) } - Some(ChecksumAlgorithm::Sha1) => Some(ChecksumValue::Sha1(self.sha1.unwrap())), - Some(ChecksumAlgorithm::Sha256) => Some(ChecksumValue::Sha256(self.sha256.unwrap())), - } + Some(ChecksumAlgorithm::Crc32c) => { + Some(ChecksumValue::Crc32c(self.crc32c.ok_or_else(|| { + CommonError::BadRequest( + "Requested checksum verification without providing checksum".to_string(), + ) + })?)) + } + Some(ChecksumAlgorithm::Crc64Nvme) => Some(ChecksumValue::Crc64Nvme( + self.crc64nvme.ok_or_else(|| { + CommonError::BadRequest( + "Requested checksum verification without providing checksum".to_string(), + ) + })?, + )), + Some(ChecksumAlgorithm::Sha1) => { + Some(ChecksumValue::Sha1(self.sha1.ok_or_else(|| { + CommonError::BadRequest( + "Requested checksum verification without providing checksum".to_string(), + ) + })?)) + } + Some(ChecksumAlgorithm::Sha256) => { + Some(ChecksumValue::Sha256(self.sha256.ok_or_else(|| { + CommonError::BadRequest( + "Requested checksum verification without providing checksum".to_string(), + ) + })?)) + } + }) } } diff --git a/src/api/s3/copy.rs b/src/api/s3/copy.rs index 97136096..9d5eed8e 100644 --- a/src/api/s3/copy.rs +++ b/src/api/s3/copy.rs @@ -706,7 +706,7 @@ pub async fn handle_upload_part_copy( let checksums = checksummer.finalize(); let etag = dest_encryption.etag_from_md5(&checksums.md5); - let checksum = checksums.extract(dest_object_checksum_algorithm.map(|(algo, _)| algo)); + let checksum = checksums.extract(dest_object_checksum_algorithm.map(|(algo, _)| algo))?; // Put the part's ETag in the Versiontable dest_mpu.parts.put( diff --git a/src/api/s3/multipart.rs b/src/api/s3/multipart.rs index c5eea912..16c8b6aa 100644 --- a/src/api/s3/multipart.rs +++ b/src/api/s3/multipart.rs @@ -225,7 +225,7 @@ pub async fn handle_put_part( MpuPart { version: version_uuid, etag: Some(etag.clone()), - checksum: checksums.extract(checksum_algorithm.map(|(algo, _)| algo)), + checksum: checksums.extract(checksum_algorithm.map(|(algo, _)| algo))?, size: Some(total_size), }, ); diff --git a/src/api/s3/put.rs b/src/api/s3/put.rs index 303a586b..e6f379fa 100644 --- a/src/api/s3/put.rs +++ b/src/api/s3/put.rs @@ -178,7 +178,7 @@ pub(crate) async fn save_stream> + Unpin>( checksums.verify(&expected)?; } ChecksumMode::Calculate(algo) => { - meta.checksum = checksums.extract(algo); + meta.checksum = checksums.extract(algo)?; } ChecksumMode::VerifyFrom { checksummer, @@ -189,7 +189,7 @@ pub(crate) async fn save_stream> + Unpin>( .await .ok_or_internal_error("checksum calculation")??; if let Some(algo) = trailer_algo { - meta.checksum = checksums.extract(Some(algo)); + meta.checksum = checksums.extract(Some(algo))?; } } } @@ -280,7 +280,7 @@ pub(crate) async fn save_stream> + Unpin>( checksums.verify(&expected)?; } ChecksumMode::Calculate(algo) => { - meta.checksum = checksums.extract(algo); + meta.checksum = checksums.extract(algo)?; } ChecksumMode::VerifyFrom { checksummer, @@ -290,7 +290,7 @@ pub(crate) async fn save_stream> + Unpin>( .await .ok_or_internal_error("checksum calculation")??; if let Some(algo) = trailer_algo { - meta.checksum = checksums.extract(Some(algo)); + meta.checksum = checksums.extract(Some(algo))?; } } }