fix: s3 list object versions next marker (#1328)

Cargo.lock

@@ -5820,7 +5820,7 @@ dependencies = [
  "libc",
  "log",
  "openssl",
- "openssl-probe",
+ "openssl-probe 0.1.6",
  "openssl-sys",
  "schannel",
  "security-framework 2.11.1",
@@ -6251,6 +6251,12 @@ dependencies = [
  "syn 2.0.111",
 ]
 
+[[package]]
+name = "openssl-probe"
+version = "0.1.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d05e27ee213611ffe7d6348b942e8f942b37114c00cc03cec254295a4a17852e"
+
 [[package]]
 name = "openssl-probe"
 version = "0.2.0"
@@ -8835,7 +8841,7 @@ version = "0.8.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "612460d5f7bea540c490b2b6395d8e34a953e52b491accd6c86c8164c5932a63"
 dependencies = [
- "openssl-probe",
+ "openssl-probe 0.2.0",
  "rustls-pki-types",
  "schannel",
  "security-framework 3.5.1",

crates/ecstore/src/store_list_objects.rs

@@ -387,7 +387,12 @@ impl ECStore {
         }
 
         let version_marker = if let Some(marker) = version_marker {
-            Some(Uuid::parse_str(&marker)?)
+            // "null" is used for non-versioned objects in AWS S3 API
+            if marker == "null" {
+                None
+            } else {
+                Some(Uuid::parse_str(&marker)?)
+            }
         } else {
             None
         };
@@ -445,7 +450,13 @@ impl ECStore {
             if is_truncated {
                 get_objects
                     .last()
-                    .map(|last| (Some(last.name.clone()), last.version_id.map(|v| v.to_string())))
+                    .map(|last| {
+                        (
+                            Some(last.name.clone()),
+                            // AWS S3 API returns "null" for non-versioned objects
+                            Some(last.version_id.map(|v| v.to_string()).unwrap_or_else(|| "null".to_string())),
+                        )
+                    })
                     .unwrap_or_default()
             } else {
                 (None, None)
@@ -1374,6 +1385,78 @@ fn calc_common_counter(infos: &[DiskInfo], read_quorum: usize) -> u64 {
 
 #[cfg(test)]
 mod test {
+    use uuid::Uuid;
+
+    /// Test that "null" version marker is handled correctly
+    /// AWS S3 API uses "null" string to represent non-versioned objects
+    #[test]
+    fn test_null_version_marker_handling() {
+        // "null" should be treated as None (non-versioned)
+        let version_marker = "null";
+        let parsed: Option<Uuid> = if version_marker == "null" {
+            None
+        } else {
+            Uuid::parse_str(version_marker).ok()
+        };
+        assert!(parsed.is_none(), "\"null\" should be parsed as None");
+
+        // Valid UUID should be parsed correctly
+        let valid_uuid = "550e8400-e29b-41d4-a716-446655440000";
+        let parsed: Option<Uuid> = if valid_uuid == "null" {
+            None
+        } else {
+            Uuid::parse_str(valid_uuid).ok()
+        };
+        assert!(parsed.is_some(), "Valid UUID should be parsed correctly");
+        assert_eq!(parsed.unwrap().to_string(), "550e8400-e29b-41d4-a716-446655440000");
+    }
+
+    /// Test that next_version_idmarker returns "null" for non-versioned objects
+    #[test]
+    fn test_next_version_idmarker_null_string() {
+        // When version_id is None, next_version_idmarker should be "null"
+        let version_id: Option<Uuid> = None;
+        let next_version_idmarker = version_id.map(|v| v.to_string()).unwrap_or_else(|| "null".to_string());
+        assert_eq!(next_version_idmarker, "null");
+
+        // When version_id is Some, next_version_idmarker should be the UUID string
+        let version_id: Option<Uuid> = Some(Uuid::parse_str("550e8400-e29b-41d4-a716-446655440000").unwrap());
+        let next_version_idmarker = version_id.map(|v| v.to_string()).unwrap_or_else(|| "null".to_string());
+        assert_eq!(next_version_idmarker, "550e8400-e29b-41d4-a716-446655440000");
+    }
+
+    /// Test the round-trip: next_version_idmarker -> VersionIdMarker parameter -> parsing
+    #[test]
+    fn test_version_marker_round_trip() {
+        // Scenario 1: Non-versioned object
+        // Server returns "null" as NextVersionIdMarker
+        // Client sends "null" as VersionIdMarker
+        // Server parses "null" as None
+        let server_response = "null";
+        let client_request = server_response;
+        let parsed: Option<Uuid> = if client_request == "null" {
+            None
+        } else {
+            Uuid::parse_str(client_request).ok()
+        };
+        assert!(parsed.is_none());
+
+        // Scenario 2: Versioned object
+        // Server returns UUID as NextVersionIdMarker
+        // Client sends UUID as VersionIdMarker
+        // Server parses UUID correctly
+        let uuid_str = "550e8400-e29b-41d4-a716-446655440000";
+        let server_response = uuid_str;
+        let client_request = server_response;
+        let parsed: Option<Uuid> = if client_request == "null" {
+            None
+        } else {
+            Uuid::parse_str(client_request).ok()
+        };
+        assert!(parsed.is_some());
+        assert_eq!(parsed.unwrap().to_string(), uuid_str);
+    }
+
     // use std::sync::Arc;
 
     // use crate::cache_value::metacache_set::list_path_raw;

rustfs/src/auth.rs

@@ -126,10 +126,21 @@ impl S3Auth for IAMAuth {
         }
 
         if let Ok(iam_store) = rustfs_iam::get() {
-            if let Some(id) = iam_store.get_user(access_key).await {
-                return Ok(SecretKey::from(id.credentials.secret_key.clone()));
-            } else {
-                tracing::warn!("get_user failed: no such user, access_key: {access_key}");
+            // Use check_key instead of get_user to ensure user is loaded from disk if not in cache
+            // This is important for newly created users that may not be in cache yet.
+            // check_key will automatically attempt to load the user from disk if not found in cache.
+            match iam_store.check_key(access_key).await {
+                Ok((Some(id), _valid)) => {
+                    // Return secret key for signature verification regardless of user status.
+                    // Authorization will be checked separately in the authorization phase.
+                    return Ok(SecretKey::from(id.credentials.secret_key.clone()));
+                }
+                Ok((None, _)) => {
+                    tracing::warn!("get_secret_key failed: no such user, access_key: {access_key}");
+                }
+                Err(e) => {
+                    tracing::warn!("get_secret_key failed: check_key error, access_key: {access_key}, error: {e:?}");
+                }
             }
         } else {
             tracing::warn!("get_secret_key failed: iam not initialized, access_key: {access_key}");

rustfs/src/storage/ecfs.rs

@@ -3044,8 +3044,9 @@ impl S3 for FS {
             })
             .collect::<Vec<_>>();
 
-        // Only set next_version_id_marker if it has a value, per AWS S3 API spec
-        // boto3 expects it to be a string or omitted, not None
+        // Only set next_key_marker and next_version_id_marker if they have values, per AWS S3 API spec
+        // boto3 expects them to be strings or omitted, not None or empty strings
+        let next_key_marker = object_infos.next_marker.filter(|v| !v.is_empty());
         let next_version_id_marker = object_infos.next_version_idmarker.filter(|v| !v.is_empty());
 
         let output = ListObjectVersionsOutput {
@@ -3057,7 +3058,7 @@ impl S3 for FS {
             common_prefixes: Some(common_prefixes),
             versions: Some(objects),
             delete_markers: Some(delete_markers),
-            next_key_marker: object_infos.next_marker,
+            next_key_marker,
             next_version_id_marker,
             ..Default::default()
         };
@@ -6191,4 +6192,83 @@ mod tests {
     //
     // These are better suited for integration tests rather than unit tests.
     // The current tests focus on the testable parts without external dependencies.
+
+    /// Test that next_key_marker and next_version_id_marker are filtered correctly
+    /// AWS S3 API requires these fields to be omitted when empty, not set to None or ""
+    #[test]
+    fn test_next_marker_filtering() {
+        // Test filter behavior for empty strings
+        let empty_string = Some(String::new());
+        let filtered = empty_string.filter(|v| !v.is_empty());
+        assert!(filtered.is_none(), "Empty string should be filtered to None");
+
+        // Test filter behavior for non-empty strings
+        let non_empty = Some("some-marker".to_string());
+        let filtered = non_empty.filter(|v| !v.is_empty());
+        assert!(filtered.is_some(), "Non-empty string should not be filtered");
+        assert_eq!(filtered.unwrap(), "some-marker");
+
+        // Test filter behavior for None
+        let none_value: Option<String> = None;
+        let filtered = none_value.filter(|v| !v.is_empty());
+        assert!(filtered.is_none(), "None should remain None");
+    }
+
+    /// Test version_id handling for ListObjectVersions response
+    /// Per AWS S3 API spec:
+    /// - Versioned objects: version_id is a UUID string
+    /// - Non-versioned objects: version_id should be "null" string
+    #[test]
+    fn test_version_id_formatting() {
+        use uuid::Uuid;
+
+        // Non-versioned object: version_id is None, should format as "null"
+        let version_id: Option<Uuid> = None;
+        let formatted = version_id.map(|v| v.to_string()).unwrap_or_else(|| "null".to_string());
+        assert_eq!(formatted, "null");
+
+        // Versioned object: version_id is Some(UUID), should format as UUID string
+        let uuid = Uuid::parse_str("550e8400-e29b-41d4-a716-446655440000").unwrap();
+        let version_id: Option<Uuid> = Some(uuid);
+        let formatted = version_id.map(|v| v.to_string()).unwrap_or_else(|| "null".to_string());
+        assert_eq!(formatted, "550e8400-e29b-41d4-a716-446655440000");
+    }
+
+    /// Test that ListObjectVersionsOutput markers are correctly set
+    /// This verifies the fix for boto3 ParamValidationError
+    #[test]
+    fn test_list_object_versions_markers_handling() {
+        // Simulate the marker filtering logic from list_object_versions
+
+        // Case 1: Both markers have values (truncated result with versioned object)
+        let next_marker = Some("object-key".to_string());
+        let next_version_idmarker = Some("550e8400-e29b-41d4-a716-446655440000".to_string());
+
+        let filtered_key_marker = next_marker.filter(|v| !v.is_empty());
+        let filtered_version_marker = next_version_idmarker.filter(|v| !v.is_empty());
+
+        assert!(filtered_key_marker.is_some());
+        assert!(filtered_version_marker.is_some());
+
+        // Case 2: Markers are empty strings (non-truncated result)
+        let next_marker = Some(String::new());
+        let next_version_idmarker = Some(String::new());
+
+        let filtered_key_marker = next_marker.filter(|v| !v.is_empty());
+        let filtered_version_marker = next_version_idmarker.filter(|v| !v.is_empty());
+
+        assert!(filtered_key_marker.is_none(), "Empty key marker should be filtered to None");
+        assert!(filtered_version_marker.is_none(), "Empty version marker should be filtered to None");
+
+        // Case 3: Truncated result with non-versioned object (version_id is "null")
+        let next_marker = Some("object-key".to_string());
+        let next_version_idmarker = Some("null".to_string());
+
+        let filtered_key_marker = next_marker.filter(|v| !v.is_empty());
+        let filtered_version_marker = next_version_idmarker.filter(|v| !v.is_empty());
+
+        assert!(filtered_key_marker.is_some());
+        assert!(filtered_version_marker.is_some());
+        assert_eq!(filtered_version_marker.unwrap(), "null");
+    }
 }

scripts/s3-tests/cleanup.sh

@@ -0,0 +1,128 @@
+#!/bin/bash
+# Copyright 2024 RustFS Team
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Cleanup script for port 9000 and test data directory
+# This script kills any process using port 9000 and cleans the test data directory
+
+set -euo pipefail
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+PROJECT_ROOT="$(cd "${SCRIPT_DIR}/../.." && pwd)"
+DATA_DIR="${PROJECT_ROOT}/target/test-data/rustfs-single"
+PORT="${S3_PORT:-9000}"
+HOST="${S3_HOST:-127.0.0.1}"
+
+log_info() {
+    echo "[INFO] $*"
+}
+
+log_warn() {
+    echo "[WARN] $*"
+}
+
+cleanup_port_9000() {
+    log_info "Checking for processes using port ${PORT}..."
+    
+    # Try to find process using port 9000
+    PID=""
+    
+    # Try lsof first (works on macOS and Linux)
+    if command -v lsof >/dev/null 2>&1; then
+        PID=$(lsof -ti:${PORT} 2>/dev/null || true)
+    fi
+    
+    # Fallback: try using netstat or ss
+    if [ -z "$PID" ]; then
+        if command -v netstat >/dev/null 2>&1; then
+            PID=$(netstat -tuln 2>/dev/null | grep ":${PORT} " | awk '{print $7}' | cut -d'/' -f1 | head -1 || true)
+        elif command -v ss >/dev/null 2>&1; then
+            PID=$(ss -tuln 2>/dev/null | grep ":${PORT} " | awk '{print $6}' | cut -d',' -f2 | cut -d'=' -f2 | head -1 || true)
+        fi
+    fi
+    
+    # If we found a PID, kill it
+    if [ -n "$PID" ] && [ "$PID" != "-" ]; then
+        log_info "Found process ${PID} using port ${PORT}, killing it..."
+        kill -9 "$PID" 2>/dev/null || true
+        sleep 2
+        
+        # Verify the process is gone
+        if kill -0 "$PID" 2>/dev/null; then
+            log_warn "Process ${PID} is still running, trying force kill..."
+            kill -9 "$PID" 2>/dev/null || true
+            sleep 1
+        fi
+    else
+        log_info "No process found using port ${PORT}"
+    fi
+    
+    # Also try to kill any rustfs processes (more aggressive cleanup)
+    if pgrep -f "rustfs.*${PORT}" >/dev/null 2>&1; then
+        log_info "Killing any remaining rustfs processes using port ${PORT}..."
+        pkill -f "rustfs.*${PORT}" 2>/dev/null || true
+        sleep 1
+    fi
+    
+    # Verify port is released
+    log_info "Verifying port ${PORT} is released..."
+    for i in {1..10}; do
+        if command -v nc >/dev/null 2>&1; then
+            if ! nc -z "${HOST}" "${PORT}" 2>/dev/null; then
+                log_info "Port ${PORT} is now available"
+                break
+            fi
+        elif timeout 1 bash -c "cat < /dev/null > /dev/tcp/${HOST}/${PORT}" 2>/dev/null; then
+            # Port is still in use
+            if [ $i -eq 10 ]; then
+                log_warn "Port ${PORT} may still be in use after cleanup attempts"
+            fi
+        else
+            log_info "Port ${PORT} is now available"
+            break
+        fi
+        sleep 1
+    done
+}
+
+cleanup_data_directory() {
+    log_info "Cleaning test data directory: ${DATA_DIR}"
+    
+    if [ -d "${DATA_DIR}" ]; then
+        # Remove all contents but keep the directory structure
+        find "${DATA_DIR}" -mindepth 1 -delete 2>/dev/null || true
+        
+        # Recreate subdirectories if they were deleted
+        for i in {0..3}; do
+            mkdir -p "${DATA_DIR}/rustfs${i}"
+        done
+        
+        log_info "Test data directory cleaned"
+    else
+        log_info "Test data directory does not exist, creating it..."
+        mkdir -p "${DATA_DIR}"
+        for i in {0..3}; do
+            mkdir -p "${DATA_DIR}/rustfs${i}"
+        done
+    fi
+}
+
+main() {
+    log_info "Starting cleanup..."
+    cleanup_port_9000
+    cleanup_data_directory
+    log_info "Cleanup completed"
+}
+
+main "$@"

scripts/s3-tests/run.sh

@@ -504,13 +504,15 @@ fi
 log_info "Using template: ${TEMPLATE_PATH}"
 log_info "Generating config: ${CONF_OUTPUT_PATH}"
 
-export S3_HOST
+# Export all required variables for envsubst
+export S3_HOST S3_ACCESS_KEY S3_SECRET_KEY S3_ALT_ACCESS_KEY S3_ALT_SECRET_KEY
 envsubst < "${TEMPLATE_PATH}" > "${CONF_OUTPUT_PATH}" || {
     log_error "Failed to generate s3tests config"
     exit 1
 }
 
 # Step 7: Provision s3-tests alt user
+# Note: Main user (rustfsadmin) is a system user and doesn't need to be created via API
 log_info "Provisioning s3-tests alt user..."
 if ! command -v awscurl >/dev/null 2>&1; then
     python3 -m pip install --user --upgrade pip awscurl || {
@@ -520,7 +522,8 @@ if ! command -v awscurl >/dev/null 2>&1; then
     export PATH="$HOME/.local/bin:$PATH"
 fi
 
-# Admin API requires AWS SigV4 signing
+# Provision alt user (required by suite)
+log_info "Provisioning alt user (${S3_ALT_ACCESS_KEY})..."
 awscurl \
     --service s3 \
     --region "${S3_REGION}" \