fix: s3 list object versions next marker #1328
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…s from disk - Replace get_user with check_key in IAMAuth::get_secret_key - check_key will automatically load users from disk if not in cache - This fixes the issue where newly created users cannot be authenticated immediately - Add detailed comments explaining the change
Ensure next_key_marker is filtered like next_version_id_marker to avoid passing None or empty values to boto3, which causes ParamValidationError when s3tests tries to use the response in subsequent requests. This fixes the error: Invalid type for parameter VersionIdMarker, value: None, type: <class 'NoneType'>, valid types: <class 'str'> which occurs in test teardown when nuke_prefixed_buckets calls list_versions and receives a response with None values that are then passed to boto3.
Main user (rustfsadmin) is a system user handled by SimpleAuth via environment variables, so it doesn't need to be created via admin API.
1. Fix envsubst: export all required variables (S3_ACCESS_KEY, S3_SECRET_KEY, S3_ALT_ACCESS_KEY, S3_ALT_SECRET_KEY) for s3tests config generation 2. Fix next_version_idmarker: return 'null' string for non-versioned objects instead of None, matching AWS S3 API behavior 3. Handle 'null' VersionIdMarker: skip UUID parsing when client passes 'null' as the version marker (used for non-versioned objects) This fixes test_bucket_list_delimiter_not_skip_special teardown error: - ParamValidationError: Invalid type for parameter VersionIdMarker, value: None
Add comprehensive unit tests for: - next_key_marker and next_version_id_marker filtering (empty string -> None) - version_id formatting (None -> "null" string for non-versioned objects) - "null" version marker parsing (skip UUID parsing for non-versioned) - Round-trip behavior: server response -> client request -> server parsing These tests verify the fix for boto3 ParamValidationError: 'Invalid type for parameter VersionIdMarker, value: None'
Copilot AI
added a commit
that referenced
this pull request
Jan 1, 2026
Co-authored-by: overtrue <1472352+overtrue@users.noreply.github.com>
15 tasks
Copilot AI
added a commit
that referenced
this pull request
Jan 1, 2026
- Fix typo: _forced_elete -> _forced_delete in api_remove.rs - Update auth.rs to use check_key for better user loading - Handle "null" version markers in store_list_objects.rs - Filter empty markers in ListObjectVersions (ecfs.rs) - Update s3-tests provisioning script comments - Add comprehensive tests for marker handling All changes are properly formatted with cargo fmt. Co-authored-by: overtrue <1472352+overtrue@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the S3 ListObjectVersions API was returning empty strings for pagination markers instead of omitting them, causing boto3 client validation errors during pagination.
Key Changes:
- Filter empty
next_key_markerandnext_version_id_markervalues to comply with AWS S3 API specification - Handle "null" string representation for non-versioned objects in version ID markers
- Add comprehensive unit tests for marker filtering and round-trip behavior
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| rustfs/src/storage/ecfs.rs | Filters empty marker strings before returning in ListObjectVersionsOutput; adds unit tests for marker filtering logic |
| crates/ecstore/src/store_list_objects.rs | Returns "null" string for non-versioned objects and parses "null" markers correctly; adds unit tests for round-trip marker handling |
| rustfs/src/auth.rs | Changes authentication from get_user to check_key (unrelated to PR purpose) |
| scripts/s3-tests/run.sh | Adds environment variable exports and user provisioning comments (unrelated to PR purpose) |
| scripts/s3-tests/cleanup.sh | New cleanup script for test environment (unrelated to PR purpose) |
| Cargo.lock | Updates openssl-probe dependency versions (unrelated to PR purpose) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
houseme
added a commit
that referenced
this pull request
Jan 3, 2026
…-proxies * 'main' of github.com:rustfs/rustfs: Add workflow to mark stale issues automatically fix: remove nginx-ingress default body size limit (#1335) feat:Permission verification for deleting versions (#1341) chore: upgrade GitHub Actions artifact actions (#1339) chore: replace native-tls with pure rustls for FTPS/SFTP e2e tests (#1334) chore: upgrade dependencies and migrate to aws-lc-rs (#1333) fix: s3 list object versions next marker (#1328) fix(tagging): fix e2e test_object_tagging failure (#1327) Feat/ftps&sftp (#1308) # Conflicts: # Cargo.lock # Cargo.toml # crates/config/src/constants/mod.rs # crates/config/src/lib.rs # rustfs/Cargo.toml
houseme
added a commit
that referenced
this pull request
Jan 7, 2026
* 'main' of github.com:rustfs/rustfs: fix:correct RemoteAddr extension type to enable IP-based policy evaluation (#1356) Add workflow to mark stale issues automatically fix: remove nginx-ingress default body size limit (#1335) feat:Permission verification for deleting versions (#1341) chore: upgrade GitHub Actions artifact actions (#1339) chore: replace native-tls with pure rustls for FTPS/SFTP e2e tests (#1334) chore: upgrade dependencies and migrate to aws-lc-rs (#1333) fix: s3 list object versions next marker (#1328) fix(tagging): fix e2e test_object_tagging failure (#1327) Feat/ftps&sftp (#1308) fix(iam): preserve decrypt-failed credentials instead of deleting them (#1312) Restore globals and add unified TLS/mTLS loading from RUSTFS_TLS_PATH (#1309) fix: correctly handle aws:SourceIp in policy evaluation (#1301) (#1306) Add trendshift feat: add local s3-tests script with configurable options and improvements (#1300)
reatang
pushed a commit
that referenced
this pull request
Jan 11, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of Change
Related Issues
Summary of Changes
Fix: ListObjectVersions NextMarker Parameter Validation Error
Problem
The
test_bucket_list_delimiter_not_skip_specials3tests case failed during teardown with:Root Cause
AWS S3 API spec:
NextKeyMarkerandNextVersionIdMarkerfields should be omitted entirely when empty, not set toNoneor empty string.Our implementation:
next_key_markerwasn't filtering empty values, causing empty strings to appear in XML responses.boto3 strict validation: When boto3 parses an empty marker from XML and passes it to the next pagination request, it fails validation since
VersionIdMarkeronly accepts non-empty strings.Fix
Filter empty values for both marker fields in
rustfs/src/storage/ecfs.rs:Also fixed
crates/ecstore/src/store_list_objects.rsto return"null"string (per S3 spec) instead ofNonefor non-versioned objects, and handle"null"marker parsing correctly.Changes
rustfs/src/storage/ecfs.rs: Filter emptynext_key_markerandnext_version_id_markercrates/ecstore/src/store_list_objects.rs: Return"null"string for non-versioned objects; handle"null"marker parsingTesting
test_bucket_list_delimiter_not_skip_specialnow passes including teardownChecklist
make pre-commitImpact
Additional Notes
Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.