Fix/correctly handle get object ifmatch&ifnonematch (#1563)

Co-authored-by: loverustfs <hello@rustfs.com>
Co-authored-by: heihutu <30542132+heihutu@users.noreply.github.com>

rustfs/src/storage/ecfs.rs

@@ -137,7 +137,7 @@ use std::{
     str::FromStr,
     sync::{Arc, LazyLock},
 };
-use time::{OffsetDateTime, format_description::well_known::Rfc3339};
+use time::{OffsetDateTime, format_description::well_known::Rfc2822, format_description::well_known::Rfc3339};
 use tokio::{
     io::{AsyncRead, AsyncSeek},
     sync::mpsc,
@@ -622,6 +622,176 @@ async fn validate_bucket_object_lock_enabled(bucket: &str) -> S3Result<()> {
     Ok(())
 }
 
+/// Validates HTTP conditional request headers for a single object according to
+/// RFC 7232 and S3 API semantics.
+///
+/// This function evaluates the following headers, if present, in the standard
+/// conditional request order:
+/// - `If-Match`
+/// - `If-None-Match`
+/// - `If-Modified-Since`
+/// - `If-Unmodified-Since`
+///
+/// The `headers` parameter provides the incoming HTTP request headers, and
+/// `info` supplies the object's current metadata (ETag and modification time)
+/// used to evaluate those preconditions.
+///
+/// The function returns `Ok(())` if either no relevant conditional headers are
+/// set or all specified preconditions are satisfied. If any precondition fails,
+/// it returns an `Err(S3Error)` with an appropriate S3 error code (for example,
+/// `PreconditionFailed` for failed `If-Match` / `If-Unmodified-Since`, or
+/// `NotModified` for `If-None-Match` / `If-Modified-Since` when the resource
+/// has not changed), allowing the caller to translate this into the correct
+/// HTTP response.
+fn check_preconditions(headers: &HeaderMap, info: &ObjectInfo) -> S3Result<()> {
+    let mod_time = info.mod_time;
+    let etag = info.etag.as_deref();
+
+    if mod_time.is_none() && etag.is_none() {
+        return Ok(());
+    }
+
+    // If-Match: requires ETag to exist
+    if let Some(if_match_val) = headers.get("if-match").and_then(|v| v.to_str().ok()) {
+        match etag {
+            Some(e) if is_etag_equal(e, if_match_val) => {}
+            _ => return Err(S3Error::new(S3ErrorCode::PreconditionFailed)),
+        }
+    }
+
+    // If-Unmodified-Since (only when If-Match is absent)
+    if headers.get("if-match").is_none()
+        && let Some(t) = mod_time
+        && let Some(if_unmodified_since) = headers.get("if-unmodified-since").and_then(|v| v.to_str().ok())
+        && let Ok(given_time) = OffsetDateTime::parse(if_unmodified_since, &Rfc2822)
+        && t > given_time.add(time::Duration::seconds(1))
+    {
+        return Err(S3Error::new(S3ErrorCode::PreconditionFailed));
+    }
+
+    // If-None-Match
+    if let Some(if_none_match) = headers.get("if-none-match").and_then(|v| v.to_str().ok())
+        && let Some(e) = etag
+        && is_etag_equal(e, if_none_match)
+    {
+        let mut error_headers = HeaderMap::new();
+        if let Ok(etag_header) = parse_etag(e) {
+            error_headers.insert("etag", etag_header);
+        }
+        if let Some(t) = mod_time
+            && let Ok(last_modified_str) = t.format(&Rfc2822)
+            && let Ok(last_modified_header) = HeaderValue::from_str(&last_modified_str)
+        {
+            error_headers.insert("last-modified", last_modified_header);
+        }
+
+        let mut s3_error = S3Error::new(S3ErrorCode::NotModified);
+        s3_error.set_message("Not Modified".to_string());
+        s3_error.set_status_code(StatusCode::NOT_MODIFIED);
+        s3_error.set_headers(error_headers);
+        return Err(s3_error);
+    }
+
+    // If-Modified-Since (only when If-None-Match is absent - RFC 7232)
+    if headers.get("if-none-match").is_none()
+        && let Some(t) = mod_time
+        && let Some(if_modified_since) = headers.get("if-modified-since").and_then(|v| v.to_str().ok())
+        && let Ok(given_time) = OffsetDateTime::parse(if_modified_since, &Rfc2822)
+        && t < given_time.add(time::Duration::seconds(1))
+    {
+        let mut error_headers = HeaderMap::new();
+        if let Some(e) = etag
+            && let Ok(etag_header) = parse_etag(e)
+        {
+            error_headers.insert("etag", etag_header);
+        }
+        if let Ok(last_modified_str) = t.format(&Rfc2822)
+            && let Ok(last_modified_header) = HeaderValue::from_str(&last_modified_str)
+        {
+            error_headers.insert("last-modified", last_modified_header);
+        }
+
+        let mut s3_error = S3Error::new(S3ErrorCode::NotModified);
+        s3_error.set_message("Not Modified".to_string());
+        s3_error.set_status_code(StatusCode::NOT_MODIFIED);
+        s3_error.set_headers(error_headers);
+
+        return Err(s3_error);
+    }
+
+    Ok(())
+}
+
+/// Compares an object ETag with an ETag value from an HTTP header.
+///
+/// This helper implements HTTP ETag comparison semantics for headers such as
+/// `If-Match` and `If-None-Match`:
+/// - Supports the wildcard `*`, which matches any `object_etag`.
+/// - Supports comma-separated ETag lists (e.g., `"etag1", "etag2"`), returning
+///   `true` if any entry matches `object_etag`.
+/// - Automatically trims surrounding whitespace and double quotes from both the
+///   header entries and `object_etag` before comparison.
+///
+/// # Parameters
+/// - `object_etag`: The ETag associated with the stored object.
+/// - `header_etag`: The raw ETag header value received in the request, which may
+///   be a wildcard, a single ETag, or a comma-separated list of ETags.
+///
+/// # Returns
+/// `true` if the header value matches the object ETag according to the above
+/// HTTP ETag comparison rules, otherwise `false`.
+fn is_etag_equal(object_etag: &str, header_etag: &str) -> bool {
+    let header_etag = header_etag.trim();
+    if header_etag == "*" {
+        return true;
+    }
+    header_etag
+        .split(',')
+        .map(|s| s.trim().trim_matches('"'))
+        .any(|e| e == object_etag.trim_matches('"'))
+}
+
+/// Converts an object ETag string into an HTTP `HeaderValue` for use in response headers.
+///
+/// This function normalizes ETag values by ensuring they are wrapped in double quotes,
+/// which is the standard HTTP ETag format according to RFC 7232. If the input ETag
+/// already contains quotes, they are removed before adding new quotes to avoid duplication.
+///
+/// # Parameters
+/// - `object_etag`: The ETag string, which may or may not already be wrapped in quotes.
+///   Must not be empty after removing quotes.
+///
+/// # Returns
+/// - `Ok(HeaderValue)`: The normalized ETag value wrapped in double quotes, ready for use
+///   in HTTP response headers.
+/// - `Err(S3Error)`: Returns `InvalidArgument` if the ETag value is empty (after removing quotes)
+///   or contains characters that cannot be represented in an HTTP header value.
+///
+/// # Note
+/// When this function returns an error, callers should log the error but continue with
+/// the operation (e.g., return NotModified without the ETag header) rather than propagating
+/// the InvalidArgument error, as the original operation semantics should be preserved.
+pub fn parse_etag(object_etag: &str) -> Result<HeaderValue, S3Error> {
+    let etag_trimmed = object_etag.trim_matches('"');
+
+    if etag_trimmed.trim().is_empty() {
+        return Err(S3Error::with_message(S3ErrorCode::InvalidArgument, "ETag cannot be empty".to_string()));
+    }
+
+    let etag_quoted = format!("\"{}\"", etag_trimmed);
+
+    HeaderValue::from_str(&etag_quoted).map_err(|e| {
+        warn!(
+            "Failed to convert ETag to HeaderValue (ETag: {}): {}. The ETag header will be omitted from the response.",
+            etag_quoted, e
+        );
+        S3Error::with_message(
+            S3ErrorCode::InvalidArgument,
+            format!("Invalid ETag header value (ETag: {}): {}", etag_quoted, e),
+        )
+    })
+}
+
 impl FS {
     pub fn new() -> Self {
         // let store: ECStore = ECStore::new(address, endpoint_pools).await?;
@@ -2459,6 +2629,8 @@ impl S3 for FS {
 
         let info = reader.object_info;
 
+        check_preconditions(&req.headers, &info)?;
+
         debug!(object_size = info.size, part_count = info.parts.len(), "GET object metadata snapshot");
         for part in &info.parts {
             debug!(
@@ -7067,6 +7239,273 @@ mod tests {
         assert_eq!(err.message(), Some("Bucket is missing ObjectLockConfiguration"));
     }
 
+    #[test]
+    fn test_is_etag_equal() {
+        // [1] Header ETag is "*", should return true (match any object ETag)
+        assert!(is_etag_equal("\"d41d8cd98f00b204e9800998ecf8427e\"", "*"));
+
+        // [2] Exact match (both with double quotes)
+        assert!(is_etag_equal(
+            "\"d41d8cd98f00b204e9800998ecf8427e\"",
+            "\"d41d8cd98f00b204e9800998ecf8427e\""
+        ));
+
+        // [3] Exact match (object ETag with quotes, header ETag without)
+        assert!(is_etag_equal("\"d41d8cd98f00b204e9800998ecf8427e\"", "d41d8cd98f00b204e9800998ecf8427e"));
+
+        // [4] Header ETag has multiple values (comma-separated), one matches
+        assert!(is_etag_equal("\"12345\"", "\"67890\", \"12345\", \"abcde\""));
+
+        // [5] Header ETag has multiple values with spaces, one matches after trim
+        assert!(is_etag_equal("\"12345\"", "  \"67890\" , \"12345\"  , \"abcde\"  "));
+
+        // [6] No match (different ETag)
+        assert!(!is_etag_equal("\"12345\"", "\"67890\""));
+
+        // [7] No match in multiple values
+        assert!(!is_etag_equal("\"12345\"", "\"67890\", \"abcde\""));
+    }
+
+    #[test]
+    fn test_check_preconditions() {
+        let valid_mod_time = OffsetDateTime::from_unix_timestamp(1700000000).unwrap();
+        let valid_etag = "\"d41d8cd98f00b204e9800998ecf8427e\"";
+        let wrong_etag = "\"wrong-etag-123456\"";
+
+        // [1] Both mod_time and etag are None → early return Ok()
+        let info1 = ObjectInfo {
+            mod_time: None,
+            etag: None,
+            ..Default::default()
+        };
+        let headers1 = HeaderMap::new();
+        assert!(check_preconditions(&headers1, &info1).is_ok());
+
+        // [2] No conditional headers with etag=None → Ok()
+        let info2 = ObjectInfo {
+            mod_time: Some(valid_mod_time),
+            etag: None,
+            ..Default::default()
+        };
+        let headers2 = HeaderMap::new();
+        assert!(check_preconditions(&headers2, &info2).is_ok());
+
+        // [3] If-None-Match matches → return Err(S3Error::NotModified)
+        let mut headers3 = HeaderMap::new();
+        headers3.insert("if-none-match", HeaderValue::from_str(valid_etag).unwrap());
+        let info3 = ObjectInfo {
+            mod_time: Some(valid_mod_time),
+            etag: Some(valid_etag.to_string()),
+            ..Default::default()
+        };
+        let result3 = check_preconditions(&headers3, &info3);
+        assert!(result3.is_err());
+        let err3 = result3.unwrap_err();
+        assert_eq!(err3.code(), &S3ErrorCode::NotModified);
+        assert_eq!(err3.message(), Some("Not Modified"));
+        assert_eq!(err3.status_code(), Some(StatusCode::NOT_MODIFIED));
+
+        // [4] If-None-Match does not match → return Ok()
+        let mut headers4 = HeaderMap::new();
+        headers4.insert("if-none-match", HeaderValue::from_str("\"wrong-etag\"").unwrap());
+        let info4 = info3.clone();
+        assert!(check_preconditions(&headers4, &info4).is_ok());
+
+        // [5] If-Modified-Since >= mod_time → return Err(S3Error::NotModified)
+        let mut headers5 = HeaderMap::new();
+        headers5.insert(
+            "if-modified-since",
+            HeaderValue::from_str(&valid_mod_time.format(&Rfc2822).unwrap()).unwrap(),
+        );
+        let info5 = info3.clone();
+        let result5 = check_preconditions(&headers5, &info5);
+        assert!(result5.is_err());
+        let err5 = result5.unwrap_err();
+        assert_eq!(err5.code(), &S3ErrorCode::NotModified);
+
+        // [6] If-Modified-Since < mod_time → return Ok()
+        let earlier_time = valid_mod_time - time::Duration::hours(1);
+        let mut headers6 = HeaderMap::new();
+        headers6.insert(
+            "if-modified-since",
+            HeaderValue::from_str(&earlier_time.format(&Rfc2822).unwrap()).unwrap(),
+        );
+        let info6 = info3.clone();
+        assert!(check_preconditions(&headers6, &info6).is_ok());
+
+        // [7] If-Match does not match → return Err(S3Error::PreconditionFailed)
+        let mut headers7 = HeaderMap::new();
+        headers7.insert("if-match", HeaderValue::from_str("\"wrong-etag\"").unwrap());
+        let info7 = info3.clone();
+        let result7 = check_preconditions(&headers7, &info7);
+        assert!(result7.is_err());
+        assert_eq!(result7.unwrap_err().code(), &S3ErrorCode::PreconditionFailed);
+
+        // [8] If-Match matches → return Ok()
+        let mut headers8 = HeaderMap::new();
+        headers8.insert("if-match", HeaderValue::from_str(valid_etag).unwrap());
+        let info8 = info3.clone();
+        assert!(check_preconditions(&headers8, &info8).is_ok());
+
+        // [9] If-Unmodified-Since < mod_time (no If-Match) → return Err(S3Error::PreconditionFailed)
+        let mut headers9 = HeaderMap::new();
+        headers9.insert(
+            "if-unmodified-since",
+            HeaderValue::from_str(&earlier_time.format(&Rfc2822).unwrap()).unwrap(),
+        );
+        let info9 = info3.clone();
+        let result9 = check_preconditions(&headers9, &info9);
+        assert!(result9.is_err());
+        assert_eq!(result9.unwrap_err().code(), &S3ErrorCode::PreconditionFailed);
+
+        // [10] If-Unmodified-Since >= mod_time (no If-Match) → return Ok()
+        let mut headers10 = HeaderMap::new();
+        headers10.insert(
+            "if-unmodified-since",
+            HeaderValue::from_str(&valid_mod_time.format(&Rfc2822).unwrap()).unwrap(),
+        );
+        let info10 = info3.clone();
+        assert!(check_preconditions(&headers10, &info10).is_ok());
+
+        // [11] If-Match (mismatch) + If-None-Match (match) → return Err(S3Error::PreconditionFailed)
+        let mut headers11 = HeaderMap::new();
+        headers11.insert("if-match", HeaderValue::from_str(wrong_etag).unwrap());
+        headers11.insert("if-none-match", HeaderValue::from_str(valid_etag).unwrap());
+        let info11 = info3.clone();
+        let result11 = check_preconditions(&headers11, &info11);
+        assert!(result11.is_err());
+        let err11 = result11.unwrap_err();
+        assert_eq!(err11.code(), &S3ErrorCode::PreconditionFailed);
+        assert_ne!(err11.code(), &S3ErrorCode::NotModified);
+
+        // [12] If-Match (match) + If-None-Match (match) → return Err(S3Error::NotModified)
+        let mut headers12 = HeaderMap::new();
+        headers12.insert("if-match", HeaderValue::from_str(valid_etag).unwrap());
+        headers12.insert("if-none-match", HeaderValue::from_str(valid_etag).unwrap());
+        let info12 = info3.clone();
+        let result12 = check_preconditions(&headers12, &info12);
+        assert!(result12.is_err());
+        let err12 = result12.unwrap_err();
+        assert_eq!(err12.code(), &S3ErrorCode::NotModified);
+        assert_ne!(err12.code(), &S3ErrorCode::PreconditionFailed);
+
+        // [13] If-None-Match (match) + If-Modified-Since → NotModified (If-Modified-Since ignored)
+        let mut headers13 = HeaderMap::new();
+        headers13.insert("if-none-match", HeaderValue::from_str(valid_etag).unwrap());
+        headers13.insert(
+            "if-modified-since",
+            HeaderValue::from_str(&earlier_time.format(&Rfc2822).unwrap()).unwrap(),
+        );
+        let info13 = info3.clone();
+        let result13 = check_preconditions(&headers13, &info13);
+        assert!(result13.is_err());
+        assert_eq!(result13.unwrap_err().code(), &S3ErrorCode::NotModified);
+
+        // [14] If-None-Match (no match) + If-Modified-Since → Ok (If-Modified-Since ignored)
+        let mut headers14 = HeaderMap::new();
+        headers14.insert("if-none-match", HeaderValue::from_str("\"wrong-etag\"").unwrap());
+        headers14.insert(
+            "if-modified-since",
+            HeaderValue::from_str(&valid_mod_time.format(&Rfc2822).unwrap()).unwrap(),
+        );
+        let info14 = info3.clone();
+        assert!(check_preconditions(&headers14, &info14).is_ok());
+
+        // [15] If-Match with no ETag → PreconditionFailed
+        let mut headers15 = HeaderMap::new();
+        headers15.insert("if-match", HeaderValue::from_str(valid_etag).unwrap());
+        let info15 = ObjectInfo {
+            mod_time: Some(valid_mod_time),
+            etag: None,
+            ..Default::default()
+        };
+        let result15 = check_preconditions(&headers15, &info15);
+        assert!(result15.is_err());
+        assert_eq!(result15.unwrap_err().code(), &S3ErrorCode::PreconditionFailed);
+
+        // [16] If-None-Match with no ETag → Ok (no match possible)
+        let mut headers16 = HeaderMap::new();
+        headers16.insert("if-none-match", HeaderValue::from_str(valid_etag).unwrap());
+        let info16 = ObjectInfo {
+            mod_time: Some(valid_mod_time),
+            etag: None,
+            ..Default::default()
+        };
+        assert!(check_preconditions(&headers16, &info16).is_ok());
+
+        // [17] mod_time None, etag exists, If-Match matches → Ok
+        let mut headers17 = HeaderMap::new();
+        headers17.insert("if-match", HeaderValue::from_str(valid_etag).unwrap());
+        let info17 = ObjectInfo {
+            mod_time: None,
+            etag: Some(valid_etag.to_string()),
+            ..Default::default()
+        };
+        assert!(check_preconditions(&headers17, &info17).is_ok());
+    }
+
+    #[test]
+    fn test_parse_etag() {
+        // [1] ETag without quotes → adds quotes
+        let result1 = parse_etag("d41d8cd98f00b204e9800998ecf8427e").unwrap();
+        assert_eq!(result1.to_str().unwrap(), "\"d41d8cd98f00b204e9800998ecf8427e\"");
+
+        // [2] ETag with quotes → normalizes to single set of quotes
+        let result2 = parse_etag("\"d41d8cd98f00b204e9800998ecf8427e\"").unwrap();
+        assert_eq!(result2.to_str().unwrap(), "\"d41d8cd98f00b204e9800998ecf8427e\"");
+
+        // [3] ETag with multiple quotes → removes all and adds single set
+        let result3 = parse_etag("\"\"\"d41d8cd98f00b204e9800998ecf8427e\"\"\"").unwrap();
+        assert_eq!(result3.to_str().unwrap(), "\"d41d8cd98f00b204e9800998ecf8427e\"");
+
+        // [4] Empty string → returns error
+        let result4 = parse_etag("");
+        assert!(result4.is_err());
+        assert_eq!(result4.unwrap_err().code(), &S3ErrorCode::InvalidArgument);
+
+        // [5] String with only quotes → returns error
+        let result5 = parse_etag("\"\"");
+        assert!(result5.is_err());
+        assert_eq!(result5.unwrap_err().code(), &S3ErrorCode::InvalidArgument);
+
+        // [6] ETag with quotes only at start → removes and adds quotes
+        let result6 = parse_etag("\"d41d8cd98f00b204e9800998ecf8427e").unwrap();
+        assert_eq!(result6.to_str().unwrap(), "\"d41d8cd98f00b204e9800998ecf8427e\"");
+
+        // [7] ETag with quotes only at end → removes and adds quotes
+        let result7 = parse_etag("d41d8cd98f00b204e9800998ecf8427e\"").unwrap();
+        assert_eq!(result7.to_str().unwrap(), "\"d41d8cd98f00b204e9800998ecf8427e\"");
+
+        // [8] ETag with newline character → returns error (invalid header value)
+        let result8 = parse_etag("d41d8cd98f00b204e9800998ecf8427e\n");
+        assert!(result8.is_err());
+        assert_eq!(result8.unwrap_err().code(), &S3ErrorCode::InvalidArgument);
+
+        // [9] ETag with null byte → returns error (invalid header value)
+        let result9 = parse_etag("d41d8cd98f00b204e9800998ecf8427e\0");
+        assert!(result9.is_err());
+        assert_eq!(result9.unwrap_err().code(), &S3ErrorCode::InvalidArgument);
+
+        // [10] ETag with carriage return → returns error (invalid header value)
+        let result10 = parse_etag("d41d8cd98f00b204e9800998ecf8427e\r");
+        assert!(result10.is_err());
+        assert_eq!(result10.unwrap_err().code(), &S3ErrorCode::InvalidArgument);
+
+        // [11] ETag with whitespace → preserves content, adds quotes
+        let result11 = parse_etag("  d41d8cd98f00b204e9800998ecf8427e  ").unwrap();
+        assert_eq!(result11.to_str().unwrap(), "\"  d41d8cd98f00b204e9800998ecf8427e  \"");
+
+        // [12] ETag with only whitespace (no quotes) → returns error
+        let result12 = parse_etag("   ");
+        assert!(result12.is_err());
+        assert_eq!(result12.unwrap_err().code(), &S3ErrorCode::InvalidArgument);
+
+        // [13] ETag with only whitespace (with quotes) → returns error
+        let result13 = parse_etag("\"   \"");
+        assert!(result13.is_err());
+        assert_eq!(result13.unwrap_err().code(), &S3ErrorCode::InvalidArgument);
+    }
+
     // Note: S3Request structure is complex and requires many fields.
     // For real testing, we would need proper integration test setup.
     // Removing this test as it requires too much S3 infrastructure setup.