Fix ETag format to comply with HTTP standards by wrapping with quotes (#592)

* Initial plan

* Fix ETag format to comply with HTTP standards by wrapping with quotes

Co-authored-by: overtrue <1472352+overtrue@users.noreply.github.com>

* bufigx

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: overtrue <1472352+overtrue@users.noreply.github.com>
Co-authored-by: overtrue <anzhengchao@gmail.com>

crates/ecstore/src/client/object_api_utils.rs

@@ -148,10 +148,75 @@ pub fn new_getobjectreader(
     Ok((get_fn, off as i64, length as i64))
 }
 
+/// Format an ETag value according to HTTP standards (wrap with quotes if not already wrapped)
+pub fn format_etag(etag: &str) -> String {
+    if etag.starts_with('"') && etag.ends_with('"') {
+        // Already properly formatted
+        etag.to_string()
+    } else if etag.starts_with("W/\"") && etag.ends_with('"') {
+        // Already a weak ETag, properly formatted
+        etag.to_string()
+    } else {
+        // Need to wrap with quotes
+        format!("\"{}\"", etag)
+    }
+}
+
 pub fn extract_etag(metadata: &HashMap<String, String>) -> String {
-    if let Some(etag) = metadata.get("etag") {
+    let etag = if let Some(etag) = metadata.get("etag") {
         etag.clone()
     } else {
         metadata["md5Sum"].clone()
+    };
+    format_etag(&etag)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_format_etag() {
+        // Test unquoted ETag - should add quotes
+        assert_eq!(format_etag("6af8d12c0c74b78094884349f3c8a079"), "\"6af8d12c0c74b78094884349f3c8a079\"");
+
+        // Test already quoted ETag - should not double quote
+        assert_eq!(
+            format_etag("\"6af8d12c0c74b78094884349f3c8a079\""),
+            "\"6af8d12c0c74b78094884349f3c8a079\""
+        );
+
+        // Test weak ETag - should keep as is
+        assert_eq!(
+            format_etag("W/\"6af8d12c0c74b78094884349f3c8a079\""),
+            "W/\"6af8d12c0c74b78094884349f3c8a079\""
+        );
+
+        // Test empty ETag - should add quotes
+        assert_eq!(format_etag(""), "\"\"");
+
+        // Test malformed quote (only starting quote) - should wrap properly
+        assert_eq!(format_etag("\"incomplete"), "\"\"incomplete\"");
+
+        // Test malformed quote (only ending quote) - should wrap properly
+        assert_eq!(format_etag("incomplete\""), "\"incomplete\"\"");
+    }
+
+    #[test]
+    fn test_extract_etag() {
+        let mut metadata = HashMap::new();
+
+        // Test with etag field
+        metadata.insert("etag".to_string(), "abc123".to_string());
+        assert_eq!(extract_etag(&metadata), "\"abc123\"");
+
+        // Test with already quoted etag field
+        metadata.insert("etag".to_string(), "\"def456\"".to_string());
+        assert_eq!(extract_etag(&metadata), "\"def456\"");
+
+        // Test fallback to md5Sum
+        metadata.remove("etag");
+        metadata.insert("md5Sum".to_string(), "xyz789".to_string());
+        assert_eq!(extract_etag(&metadata), "\"xyz789\"");
     }
 }

rustfs/src/storage/ecfs.rs

@@ -76,6 +76,7 @@ use rustfs_ecstore::bucket::tagging::decode_tags;
 use rustfs_ecstore::bucket::tagging::encode_tags;
 use rustfs_ecstore::bucket::utils::serialize;
 use rustfs_ecstore::bucket::versioning_sys::BucketVersioningSys;
+use rustfs_ecstore::client::object_api_utils::format_etag;
 use rustfs_ecstore::compress::MIN_COMPRESSIBLE_SIZE;
 use rustfs_ecstore::compress::is_compressible;
 use rustfs_ecstore::error::StorageError;
@@ -460,7 +461,7 @@ impl FS {
                     .await
                     .map_err(ApiError::from)?;
 
-                let e_tag = _obj_info.clone().etag;
+                let e_tag = _obj_info.etag.clone().map(|etag| format_etag(&etag));
 
                 // // store.put_object(bucket, object, data, opts);
 
@@ -748,7 +749,7 @@ impl S3 for FS {
         // warn!("copy_object oi {:?}", &oi);
         let object_info = oi.clone();
         let copy_object_result = CopyObjectResult {
-            e_tag: oi.etag,
+            e_tag: oi.etag.map(|etag| format_etag(&etag)),
             last_modified: oi.mod_time.map(Timestamp::from),
             ..Default::default()
         };
@@ -1693,7 +1694,7 @@ impl S3 for FS {
             content_type,
             accept_ranges: Some("bytes".to_string()),
             content_range,
-            e_tag: info.etag,
+            e_tag: info.etag.map(|etag| format_etag(&etag)),
             metadata: Some(info.user_defined),
             server_side_encryption,
             sse_customer_algorithm,
@@ -1827,7 +1828,7 @@ impl S3 for FS {
             content_length: Some(content_length),
             content_type,
             last_modified,
-            e_tag: info.etag,
+            e_tag: info.etag.map(|etag| format_etag(&etag)),
             metadata: Some(metadata),
             version_id: info.version_id.map(|v| v.to_string()),
             server_side_encryption,
@@ -1978,7 +1979,7 @@ impl S3 for FS {
                     key: Some(v.name.to_owned()),
                     last_modified: v.mod_time.map(Timestamp::from),
                     size: Some(v.get_actual_size().unwrap_or_default()),
-                    e_tag: v.etag.clone(),
+                    e_tag: v.etag.clone().map(|etag| format_etag(&etag)),
                     ..Default::default()
                 };
 
@@ -2057,7 +2058,7 @@ impl S3 for FS {
                     size: Some(v.size),
                     version_id: v.version_id.map(|v| v.to_string()),
                     is_latest: Some(v.is_latest),
-                    e_tag: v.etag.clone(),
+                    e_tag: v.etag.clone().map(|etag| format_etag(&etag)),
                     ..Default::default() // TODO: another fields
                 }
             })
@@ -2334,7 +2335,7 @@ impl S3 for FS {
             .await
             .map_err(ApiError::from)?;
         let event_info = obj_info.clone();
-        let e_tag = obj_info.etag.clone();
+        let e_tag = obj_info.etag.clone().map(|etag| format_etag(&etag));
 
         let repoptions =
             get_must_replicate_options(&mt2, "".to_string(), ReplicationStatusType::Empty, ReplicationType::Object, opts);
@@ -2697,7 +2698,7 @@ impl S3 for FS {
             .map_err(ApiError::from)?;
 
         let output = UploadPartOutput {
-            e_tag: info.etag,
+            e_tag: info.etag.map(|etag| format_etag(&etag)),
             ..Default::default()
         };
 
@@ -2883,7 +2884,7 @@ impl S3 for FS {
 
         // Create response
         let copy_part_result = CopyPartResult {
-            e_tag: part_info.etag,
+            e_tag: part_info.etag.map(|etag| format_etag(&etag)),
             last_modified: part_info.last_mod.map(Timestamp::from),
             ..Default::default()
         };
@@ -2936,7 +2937,7 @@ impl S3 for FS {
                 res.parts
                     .into_iter()
                     .map(|p| Part {
-                        e_tag: p.etag,
+                        e_tag: p.etag.map(|etag| format_etag(&etag)),
                         last_modified: p.last_mod.map(Timestamp::from),
                         part_number: Some(p.part_num as i32),
                         size: Some(p.size as i64),
@@ -3111,7 +3112,7 @@ impl S3 for FS {
         let output = CompleteMultipartUploadOutput {
             bucket: Some(bucket.clone()),
             key: Some(key.clone()),
-            e_tag: obj_info.etag.clone(),
+            e_tag: obj_info.etag.clone().map(|etag| format_etag(&etag)),
             location: Some("us-east-1".to_string()),
             server_side_encryption, // TDD: Return encryption info
             ssekms_key_id,          // TDD: Return KMS key ID if present