From c8c71e34b6bb4f757f2a47b49f1bc2a807ce846c Mon Sep 17 00:00:00 2001 From: Mohamed Zenadi Date: Sat, 11 Apr 2026 18:20:15 +0200 Subject: [PATCH] fix(storage): fix critical bug in range calculation (#2493) Signed-off-by: Mohamed Zenadi Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- crates/e2e_test/src/range_request_test.rs | 99 ++++++++++++++++++++++- crates/ecstore/src/set_disk/read.rs | 6 +- 2 files changed, 102 insertions(+), 3 deletions(-) diff --git a/crates/e2e_test/src/range_request_test.rs b/crates/e2e_test/src/range_request_test.rs index b5283129c..df0f1f4ed 100644 --- a/crates/e2e_test/src/range_request_test.rs +++ b/crates/e2e_test/src/range_request_test.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! End-to-end regression test for invalid GET object ranges. +//! End-to-end regression tests for invalid and suffix GET object ranges. #[cfg(test)] mod tests { @@ -77,4 +77,101 @@ mod tests { other_err => panic!("Expected S3 service error, got: {other_err:?}"), } } + + #[tokio::test] + #[serial] + async fn test_get_object_suffix_byte_range_returns_correct_body() { + init_logging(); + info!("TEST: GetObject suffix byte-range should return correct body"); + + let mut env = RustFSTestEnvironment::new().await.expect("Failed to create test environment"); + env.start_rustfs_server(vec![]).await.expect("Failed to start RustFS"); + + let client = create_s3_client(&env); + let bucket = "test-suffix-range"; + let key = "range-suffix.bin"; + + // ~3 MB so the object spans multiple erasure blocks (block_size = 1 MB). + // Suffix ranges on single-block objects never hit the bug. + let file_size: usize = 3_095_910; + let content: Vec = (0..file_size).map(|i| (i % 256) as u8).collect(); + + create_bucket(&client, bucket).await.expect("Failed to create bucket"); + client + .put_object() + .bucket(bucket) + .key(key) + .body(ByteStream::from(content.clone())) + .send() + .await + .expect("PutObject should succeed"); + + // bytes=-8 — last 8 bytes (parquet footer read) + let result = client + .get_object() + .bucket(bucket) + .key(key) + .range("bytes=-8") + .send() + .await + .expect("bytes=-8 should succeed"); + let body = result.body.collect().await.expect("bytes=-8 body").into_bytes(); + assert_eq!(body.len(), 8, "bytes=-8 body length"); + assert_eq!(&body[..], &content[file_size - 8..], "bytes=-8 body content"); + + // bytes=-96 — last 96 bytes + let result = client + .get_object() + .bucket(bucket) + .key(key) + .range("bytes=-96") + .send() + .await + .expect("bytes=-96 should succeed"); + let body = result.body.collect().await.expect("bytes=-96 body").into_bytes(); + assert_eq!(body.len(), 96, "bytes=-96 body length"); + assert_eq!(&body[..], &content[file_size - 96..], "bytes=-96 body content"); + + // bytes=-1 — last byte + let result = client + .get_object() + .bucket(bucket) + .key(key) + .range("bytes=-1") + .send() + .await + .expect("bytes=-1 should succeed"); + let body = result.body.collect().await.expect("bytes=-1 body").into_bytes(); + assert_eq!(body.len(), 1, "bytes=-1 body length"); + assert_eq!(body[0], content[file_size - 1], "bytes=-1 body content"); + + // bytes=0-7 — absolute range (regression guard) + let result = client + .get_object() + .bucket(bucket) + .key(key) + .range("bytes=0-7") + .send() + .await + .expect("bytes=0-7 should succeed"); + let body = result.body.collect().await.expect("bytes=0-7 body").into_bytes(); + assert_eq!(body.len(), 8, "bytes=0-7 body length"); + assert_eq!(&body[..], &content[0..8], "bytes=0-7 body content"); + + // Equivalent absolute range for the same last-8-bytes window + let start = file_size - 8; + let end = file_size - 1; + let range = format!("bytes={start}-{end}"); + let result = client + .get_object() + .bucket(bucket) + .key(key) + .range(&range) + .send() + .await + .expect("absolute tail range should succeed"); + let body = result.body.collect().await.expect("absolute tail body").into_bytes(); + assert_eq!(body.len(), 8, "absolute tail body length"); + assert_eq!(&body[..], &content[start..], "absolute tail body content"); + } } diff --git a/crates/ecstore/src/set_disk/read.rs b/crates/ecstore/src/set_disk/read.rs index c527a7769..383a74687 100644 --- a/crates/ecstore/src/set_disk/read.rs +++ b/crates/ecstore/src/set_disk/read.rs @@ -419,6 +419,7 @@ async fn build_reconstructed_part_stream( skip_verify_bitrot: bool, use_zero_copy: bool, ) -> Result> { + let shard_length = till_offset.saturating_sub(read_offset); let mut readers = Vec::with_capacity(disks.len()); let mut errors = Vec::with_capacity(disks.len()); for (idx, disk_op) in disks.iter().enumerate() { @@ -428,7 +429,7 @@ async fn build_reconstructed_part_stream( bucket, &format!("{}/{}/part.{}", object, files[idx].data_dir.unwrap_or_default(), part_number), read_offset, - till_offset, + shard_length, erasure.shard_size(), checksum_algo.clone(), skip_verify_bitrot, @@ -1563,6 +1564,7 @@ impl SetDisks { let mut readers = Vec::with_capacity(disks.len()); let mut errors = Vec::with_capacity(disks.len()); + let shard_length = till_offset.saturating_sub(read_offset); for (idx, disk_op) in disks.iter().enumerate() { match create_bitrot_reader( files[idx].data.as_deref(), @@ -1570,7 +1572,7 @@ impl SetDisks { bucket, &format!("{}/{}/part.{}", object, files[idx].data_dir.unwrap_or_default(), part_number), read_offset, - till_offset, + shard_length, erasure.shard_size(), checksum_algo.clone(), skip_verify_bitrot,