From 4a5bbbb81088c9bd25bbe142f67daf4669b6538e Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Sat, 5 Dec 2020 19:23:46 +0100 Subject: Propose ETag fix --- Cargo.lock | 1 + src/api/Cargo.toml | 1 + src/api/s3_get.rs | 5 ++++- src/api/s3_put.rs | 17 ++++++++++++++++- src/table/table.rs | 3 ++- 5 files changed, 24 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a7cf8b56..1d9525be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -461,6 +461,7 @@ dependencies = [ "log", "md-5", "percent-encoding", + "rand", "roxmltree", "sha2", "tokio", diff --git a/src/api/Cargo.toml b/src/api/Cargo.toml index a366f9b8..079993c3 100644 --- a/src/api/Cargo.toml +++ b/src/api/Cargo.toml @@ -27,6 +27,7 @@ md-5 = "0.9.1" sha2 = "0.8" hmac = "0.7" crypto-mac = "0.7" +rand = "0.7" futures = "0.3" futures-util = "0.3" diff --git a/src/api/s3_get.rs b/src/api/s3_get.rs index 43215923..1a23f476 100644 --- a/src/api/s3_get.rs +++ b/src/api/s3_get.rs @@ -24,10 +24,13 @@ fn object_headers( "Content-Type", version_meta.headers.content_type.to_string(), ) - .header("ETag", version_meta.etag.to_string()) .header("Last-Modified", date_str) .header("Accept-Ranges", format!("bytes")); + if !version_meta.etag.is_empty() { + resp = resp.header("ETag", format!("\"{}\"", version_meta.etag)); + } + for (k, v) in version_meta.headers.other.iter() { resp = resp.header(k, v.to_string()); } diff --git a/src/api/s3_put.rs b/src/api/s3_put.rs index 9c4d625c..c42309b2 100644 --- a/src/api/s3_put.rs +++ b/src/api/s3_put.rs @@ -428,6 +428,21 @@ pub async fn handle_complete_multipart_upload( _ => unreachable!(), }; + // ETag calculation: we produce ETags that have the same form as + // those of S3 multipart uploads, but we don't use their actual + // calculation for the first part (we use random bytes). This + // shouldn't impact compatibility as the S3 docs specify that + // the ETag is an opaque value in case of a multipart upload. + // See also: https://teppen.io/2018/06/23/aws_s3_etags/ + let num_parts = version.blocks().last().unwrap().part_number + - version.blocks().first().unwrap().part_number + + 1; + let etag = format!( + "{}-{}", + hex::encode(&rand::random::<[u8; 16]>()[..]), + num_parts + ); + // TODO: check that all the parts that they pretend they gave us are indeed there // TODO: when we read the XML from _req, remember to check the sha256 sum of the payload // against the signed x-amz-content-sha256 @@ -442,7 +457,7 @@ pub async fn handle_complete_multipart_upload( ObjectVersionMeta { headers, size: total_size, - etag: "".to_string(), // TODO + etag: etag, }, version.blocks()[0].hash, )); diff --git a/src/table/table.rs b/src/table/table.rs index 5dfee3c8..acb46325 100644 --- a/src/table/table.rs +++ b/src/table/table.rs @@ -391,7 +391,8 @@ where let (old_entry, new_entry) = self.store.transaction(|db| { let (old_entry, new_entry) = match db.get(&tree_key)? { Some(prev_bytes) => { - let old_entry = self.decode_entry(&prev_bytes) + let old_entry = self + .decode_entry(&prev_bytes) .map_err(sled::ConflictableTransactionError::Abort)?; let mut new_entry = old_entry.clone(); new_entry.merge(&update); -- cgit v1.2.3