Skip to content

fix:ListObjects and ListObjectV2 correctly handles unordered and delimiter #1285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
houseme merged 6 commits into from
Dec 28, 2025
Merged

fix:ListObjects and ListObjectV2 correctly handles unordered and delimiter #1285

houseme merged 6 commits into from
Dec 28, 2025

Conversation

@LeonWang0735
Copy link
Contributor

@LeonWang0735 LeonWang0735 commented Dec 27, 2025

Type of Change

  • New Feature
  • Bug Fix
  • Documentation
  • Performance Improvement
  • Test/CI
  • Refactor
  • Other:

Summary of Changes

Fix S3 compatibility validation: properly reject the allow-unordered=true query parameter when used with Delimiter="/" in ListObjectsV2 operations, returning InvalidArgument error as required by the S3 API specification to fix the failing test_bucket_list_unordered and test_bucket_listv2_unorderedtest case.

Checklist

  • I have read and followed the CONTRIBUTING.md guidelines
  • Passed make pre-commit
  • Added/updated necessary tests
  • Documentation updated (if needed)
  • CI/CD passed (if applicable)

Impact

  • Breaking change (compatibility)
  • Requires doc/config/deployment update
  • Other impact:Improves S3 API compliance by enforcing parameter validation; expected to be backward-compatible for compliant clients.

@CLAassistant
Copy link

CLAassistant commented Dec 27, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@houseme houseme requested review from Copilot and weisd December 27, 2025 15:02
houseme
houseme reviewed Dec 27, 2025
View reviewed changes
Copilot AI reviewed Dec 27, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes S3 API compliance validation for ListObjectsV2 and ListObjects operations by rejecting requests that combine the allow-unordered=true query parameter with a delimiter, returning an InvalidArgument error as required by the S3 specification.

Key changes:

  • Added validation logic in list_objects_v2 to detect and reject the incompatible parameter combination
  • Implemented query string parsing to check for the allow-unordered=true parameter
  • Returns appropriate S3Error when invalid combination is detected

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@houseme
Copy link
Contributor

houseme commented Dec 27, 2025

This PR correctly fixes an S3 API compatibility issue by rejecting the invalid allow-unordered=true parameter when delimiter="/" is specified, aligning with AWS S3 specifications and fixing failing tests. The change is small, targeted, and improves compliance. However, minor issues need addressing for production readiness.

Key Issues and Suggestions

  • Remove debug code: The println!("has_allow_unordered"); statement must be removed as it is inappropriate for production code. Replace with proper logging if debugging is needed.
  • Improve parameter parsing: The manual query string parsing (using split and split_once) is error-prone and could fail on malformed inputs or URL encoding issues. Consider using a URL parsing crate (e.g., url) or the framework's built-in query parameter handling for robustness.
  • Enhance error message: Update the error message to be more descriptive, e.g., "InvalidArgument: The allow-unordered parameter cannot be used when delimiter is specified." Include a reference to the S3 spec for clarity.
  • Add tests: While existing tests are fixed, add unit tests for edge cases (e.g., allow-unordered=false, empty delimiter, malformed queries) to ensure comprehensive coverage. Update the PR checklist to reflect this.
  • Code documentation: Add comments explaining the validation logic and why it's required per S3 specs.

Recommendations

  • Request Changes: Address the debug code removal and parameter parsing improvements before merging to ensure code quality and maintainability.
  • Optional Enhancements: Extract the validation into a reusable function and verify performance impact in high-load scenarios.
  • Approval Status: Pending fixes. Re-review after updates.

Thanks for the contribution! This strengthens S3 compatibility. Let me know if you need help implementing these changes.

@houseme houseme merged commit 71c59d1 into rustfs:main Dec 28, 2025
12 checks passed
@LeonWang0735 LeonWang0735 changed the title fix:ListObjects and ListObjectV2 correctly handles unordered and delimiter fix:ListObjects and ListObjectV2 correctly handles unordered and delimiter #1289 Dec 28, 2025
@LeonWang0735 LeonWang0735 changed the title fix:ListObjects and ListObjectV2 correctly handles unordered and delimiter #1289 fix:ListObjects and ListObjectV2 correctly handles unordered and delimiter Dec 28, 2025
@LeonWang0735
Copy link
Contributor Author

LeonWang0735 commented Dec 28, 2025

Related to #1289

lgpseu pushed a commit to lgpseu/rustfs that referenced this pull request Dec 31, 2025
@LeonWang0735 @lgpseu
fix:ListObjects and ListObjectV2 correctly handles unordered and deli…
d0c2937 
…miter (rustfs#1285)
houseme added a commit that referenced this pull request Jan 7, 2026
@houseme
Merge branch 'main' of github.com:rustfs/rustfs into fix/fix-audit-1222
5e01ebf 
* 'main' of github.com:rustfs/rustfs:
  fix: Prevent panic in GetMetrics gRPC handler on invalid input (#1291)
  Modular Makefile (#1288)
  fix:ListObjects and ListObjectV2 correctly handles unordered and delimiter (#1285)
  fix: ensure version_id is returned in S3 response headers (#1272)
  feat: add function to extract user-defined metadata keys and integrat… (#1281)
  helm: update default Chart.yaml, appVersion version bump, add appVersion as a default image tag (#1247)
  fix:affinity.podAntiAffinity.enabled value not taking effect (#1280)
  fix: prevent PV/PVC deletion during rustfs uninstallation (#1279)

# Conflicts:
#	Cargo.lock
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@houseme houseme houseme left review comments

Copilot code review Copilot Copilot left review comments

@weisd weisd Awaiting requested review from weisd

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@LeonWang0735 @CLAassistant @houseme