From 007d9c0b21914302be6393fe4530cbf1b3cc44b3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= <anzhengchao@gmail.com>
Date: Wed, 8 Oct 2025 21:19:57 +0800
Subject: [PATCH] fix: normalize ETag comparison in multipart upload and
 replication (#627)

- Normalize ETags by removing quotes before comparison in complete_multipart_upload
- Fix ETag comparison in replication logic to handle quoted ETags from API responses
- Fix ETag comparison in transition object logic
- Add unit tests for trim_etag function

This fixes the ETag mismatch error when uploading large files (5GB+) via multipart upload,
which was caused by PR #592 adding quotes to ETag responses while internal storage remains unquoted.

Fixes #625
---
 .../replication/replication_resyncer.rs       |  6 +++++-
 crates/ecstore/src/set_disk.rs                | 14 +++++++++----
 crates/utils/src/path.rs                      | 21 +++++++++++++++++++
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/crates/ecstore/src/bucket/replication/replication_resyncer.rs b/crates/ecstore/src/bucket/replication/replication_resyncer.rs
index f3578955..e3a4b90d 100644
--- a/crates/ecstore/src/bucket/replication/replication_resyncer.rs
+++ b/crates/ecstore/src/bucket/replication/replication_resyncer.rs
@@ -2312,7 +2312,11 @@ fn get_replication_action(oi1: &ObjectInfo, oi2: &HeadObjectOutput, op_type: Rep
 
     let size = oi1.get_actual_size().unwrap_or_default();
 
-    if oi1.etag != oi2.e_tag
+    // Normalize ETags by removing quotes before comparison (PR #592 compatibility)
+    let oi1_etag = oi1.etag.as_ref().map(|e| rustfs_utils::path::trim_etag(e));
+    let oi2_etag = oi2.e_tag.as_ref().map(|e| rustfs_utils::path::trim_etag(e));
+
+    if oi1_etag != oi2_etag
         || oi1.version_id.map(|v| v.to_string()) != oi2.version_id
         || size != oi2.content_length.unwrap_or_default()
         || oi1.delete_marker != oi2.delete_marker.unwrap_or_default()
diff --git a/crates/ecstore/src/set_disk.rs b/crates/ecstore/src/set_disk.rs
index 572a8183..93366e4a 100644
--- a/crates/ecstore/src/set_disk.rs
+++ b/crates/ecstore/src/set_disk.rs
@@ -5668,8 +5668,11 @@ impl StorageAPI for SetDisks {
             }
             return Err(to_object_err(ERR_METHOD_NOT_ALLOWED, vec![bucket, object]));
         }*/
+        // Normalize ETags by removing quotes before comparison (PR #592 compatibility)
+        let transition_etag = rustfs_utils::path::trim_etag(&opts.transition.etag);
+        let stored_etag = rustfs_utils::path::trim_etag(&extract_etag(&fi.metadata));
         if !opts.mod_time.expect("err").unix_timestamp() == fi.mod_time.as_ref().expect("err").unix_timestamp()
-            || opts.transition.etag != extract_etag(&fi.metadata)
+            || transition_etag != stored_etag
         {
             return Err(to_object_err(Error::from(DiskError::FileNotFound), vec![bucket, object]));
         }
@@ -6598,10 +6601,13 @@ impl StorageAPI for SetDisks {
             let ext_part = &curr_fi.parts[i];
             tracing::info!(target:"rustfs_ecstore::set_disk", part_number = p.part_num, part_size = ext_part.size, part_actual_size = ext_part.actual_size, "Completing multipart part");
 
-            if p.etag != Some(ext_part.etag.clone()) {
+            // Normalize ETags by removing quotes before comparison (PR #592 compatibility)
+            let client_etag = p.etag.as_ref().map(|e| rustfs_utils::path::trim_etag(e));
+            let stored_etag = Some(rustfs_utils::path::trim_etag(&ext_part.etag));
+            if client_etag != stored_etag {
                 error!(
-                    "complete_multipart_upload etag err {:?}, part_id={}, bucket={}, object={}",
-                    p.etag, p.part_num, bucket, object
+                    "complete_multipart_upload etag err client={:?}, stored={:?}, part_id={}, bucket={}, object={}",
+                    p.etag, ext_part.etag, p.part_num, bucket, object
                 );
                 return Err(Error::InvalidPart(p.part_num, ext_part.etag.clone(), p.etag.clone().unwrap_or_default()));
             }
diff --git a/crates/utils/src/path.rs b/crates/utils/src/path.rs
index 07f158b2..e263033c 100644
--- a/crates/utils/src/path.rs
+++ b/crates/utils/src/path.rs
@@ -276,6 +276,27 @@ pub fn trim_etag(etag: &str) -> String {
 mod tests {
     use super::*;
 
+    #[test]
+    fn test_trim_etag() {
+        // Test with quoted ETag
+        assert_eq!(trim_etag("\"abc123\""), "abc123");
+
+        // Test with unquoted ETag
+        assert_eq!(trim_etag("abc123"), "abc123");
+
+        // Test with empty string
+        assert_eq!(trim_etag(""), "");
+
+        // Test with only quotes
+        assert_eq!(trim_etag("\"\""), "");
+
+        // Test with MD5 hash
+        assert_eq!(trim_etag("\"2c7ab85a893283e98c931e9511add182\""), "2c7ab85a893283e98c931e9511add182");
+
+        // Test with multipart ETag format
+        assert_eq!(trim_etag("\"098f6bcd4621d373cade4e832627b4f6-2\""), "098f6bcd4621d373cade4e832627b4f6-2");
+    }
+
     #[test]
     fn test_base_dir_from_prefix() {
         let a = "da/";