From 9f46fb699a68c2d8b721b0b1d0e3419a830a4052 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Sat, 5 Dec 2020 16:37:59 +0100 Subject: [PATCH 1/3] Content-range fix --- src/api/s3_get.rs | 44 +++++++++++++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/src/api/s3_get.rs b/src/api/s3_get.rs index a68c485b..a5e7b342 100644 --- a/src/api/s3_get.rs +++ b/src/api/s3_get.rs @@ -161,7 +161,7 @@ pub async fn handle_get( } }) .buffered(2); - //let body: Body = Box::new(StreamBody::new(Box::pin(body_stream))); + let body = hyper::body::Body::wrap_stream(body_stream); Ok(resp_builder.body(body)?) } @@ -183,7 +183,7 @@ pub async fn handle_get_range( let resp_builder = object_headers(version, version_meta) .header( "Content-Range", - format!("bytes {}-{}/{}", begin, end, version_meta.size), + format!("bytes {}-{}/{}", begin, end - 1, version_meta.size), ) .status(StatusCode::PARTIAL_CONTENT); @@ -206,35 +206,49 @@ pub async fn handle_get_range( None => return Err(Error::NotFound), }; - let blocks = version - .blocks() - .iter() - .cloned() - .filter(|block| block.offset + block.size > begin && block.offset < end) - .collect::>(); + // We will store here the list of blocks that have an intersection with the requested + // range, as well as their "true offset", which is their actual offset in the complete + // file (whereas block.offset designates the offset of the block WITHIN THE PART + // block.part_number, which is not the same in the case of a multipart upload) + let mut blocks = Vec::with_capacity(std::cmp::min( + version.blocks().len(), + 4 + ((end - begin) / std::cmp::max(version.blocks()[0].size as u64, 1024)) as usize, + )); + let mut true_offset = 0; + for b in version.blocks().iter() { + if true_offset >= end { + break; + } + // Keep only blocks that have an intersection with the requested range + if true_offset < end && true_offset + b.size > begin { + blocks.push((b.clone(), true_offset)); + } + true_offset += b.size; + } let body_stream = futures::stream::iter(blocks) - .map(move |block| { + .map(move |(block, true_offset)| { let garage = garage.clone(); async move { let data = garage.block_manager.rpc_get_block(&block.hash).await?; - let start_in_block = if block.offset > begin { + let data = Bytes::from(data); + let start_in_block = if true_offset > begin { 0 } else { - begin - block.offset + begin - true_offset }; - let end_in_block = if block.offset + block.size < end { + let end_in_block = if true_offset + block.size < end { block.size } else { - end - block.offset + end - true_offset }; Result::::Ok(Bytes::from( - data[start_in_block as usize..end_in_block as usize].to_vec(), + data.slice(start_in_block as usize..end_in_block as usize), )) } }) .buffered(2); - //let body: Body = Box::new(StreamBody::new(Box::pin(body_stream))); + let body = hyper::body::Body::wrap_stream(body_stream); Ok(resp_builder.body(body)?) } From bd7e3d1bd1c4df4f2848e0f8c49878dacb330b50 Mon Sep 17 00:00:00 2001 From: Quentin Date: Sat, 5 Dec 2020 18:57:22 +0100 Subject: [PATCH 2/3] Fix Content-Length --- src/api/s3_get.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/api/s3_get.rs b/src/api/s3_get.rs index a5e7b342..a52222a0 100644 --- a/src/api/s3_get.rs +++ b/src/api/s3_get.rs @@ -15,6 +15,7 @@ use crate::error::*; fn object_headers( version: &ObjectVersion, version_meta: &ObjectVersionMeta, + partial_size: Option ) -> http::response::Builder { let date = UNIX_EPOCH + Duration::from_millis(version.timestamp); let date_str = httpdate::fmt_http_date(date); @@ -24,7 +25,7 @@ fn object_headers( "Content-Type", version_meta.headers.content_type.to_string(), ) - .header("Content-Length", format!("{}", version_meta.size)) + .header("Content-Length", format!("{}", partial_size.unwrap_or(version_meta.size))) .header("ETag", version_meta.etag.to_string()) .header("Last-Modified", date_str) .header("Accept-Ranges", format!("bytes")); @@ -62,7 +63,7 @@ pub async fn handle_head( }; let body: Body = Body::from(vec![]); - let response = object_headers(&version, version_meta) + let response = object_headers(&version, version_meta, None) .status(StatusCode::OK) .body(body) .unwrap(); @@ -123,7 +124,7 @@ pub async fn handle_get( .await; } - let resp_builder = object_headers(&last_v, last_v_meta).status(StatusCode::OK); + let resp_builder = object_headers(&last_v, last_v_meta, None).status(StatusCode::OK); match &last_v_data { ObjectVersionData::DeleteMarker => unreachable!(), @@ -180,7 +181,7 @@ pub async fn handle_get_range( return Err(Error::BadRequest(format!("Range not included in file"))); } - let resp_builder = object_headers(version, version_meta) + let resp_builder = object_headers(version, version_meta, Some(end - begin)) .header( "Content-Range", format!("bytes {}-{}/{}", begin, end - 1, version_meta.size), From 76b489f3d374c5a793929ce70654538d07268a6e Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Sat, 5 Dec 2020 19:20:07 +0100 Subject: [PATCH 3/3] Reformulate patch --- src/api/s3_get.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/api/s3_get.rs b/src/api/s3_get.rs index a52222a0..43215923 100644 --- a/src/api/s3_get.rs +++ b/src/api/s3_get.rs @@ -15,7 +15,6 @@ use crate::error::*; fn object_headers( version: &ObjectVersion, version_meta: &ObjectVersionMeta, - partial_size: Option ) -> http::response::Builder { let date = UNIX_EPOCH + Duration::from_millis(version.timestamp); let date_str = httpdate::fmt_http_date(date); @@ -25,7 +24,6 @@ fn object_headers( "Content-Type", version_meta.headers.content_type.to_string(), ) - .header("Content-Length", format!("{}", partial_size.unwrap_or(version_meta.size))) .header("ETag", version_meta.etag.to_string()) .header("Last-Modified", date_str) .header("Accept-Ranges", format!("bytes")); @@ -63,7 +61,8 @@ pub async fn handle_head( }; let body: Body = Body::from(vec![]); - let response = object_headers(&version, version_meta, None) + let response = object_headers(&version, version_meta) + .header("Content-Length", format!("{}", version_meta.size)) .status(StatusCode::OK) .body(body) .unwrap(); @@ -124,7 +123,9 @@ pub async fn handle_get( .await; } - let resp_builder = object_headers(&last_v, last_v_meta, None).status(StatusCode::OK); + let resp_builder = object_headers(&last_v, last_v_meta) + .header("Content-Length", format!("{}", last_v_meta.size)) + .status(StatusCode::OK); match &last_v_data { ObjectVersionData::DeleteMarker => unreachable!(), @@ -181,7 +182,8 @@ pub async fn handle_get_range( return Err(Error::BadRequest(format!("Range not included in file"))); } - let resp_builder = object_headers(version, version_meta, Some(end - begin)) + let resp_builder = object_headers(version, version_meta) + .header("Content-Length", format!("{}", end - begin)) .header( "Content-Range", format!("bytes {}-{}/{}", begin, end - 1, version_meta.size),