Skip to content

fix: policy StringNotEquals double negation and delete_objects version mapping#2015

Merged
overtrue merged 2 commits intomainfrom
fix/policy-string-not-equals-double-negation
Feb 28, 2026
Merged

fix: policy StringNotEquals double negation and delete_objects version mapping#2015
overtrue merged 2 commits intomainfrom
fix/policy-string-not-equals-double-negation

Conversation

@overtrue
Copy link
Collaborator

Type of Change

  • Bug Fix

Related Issues

Summary of Changes

Fix three S3 compatibility bugs that caused 3 s3-tests to fail:

1. Policy StringNotEquals double negation (condition.rs)

StringNotEquals and StringNotEqualsIgnoreCase conditions were being negated twice:

  • Once via the negate=true parameter passed to evaluate_with_resolver
  • Again via is_negate() in Condition::evaluate_with_resolver

This double negation made StringNotEquals behave identically to StringEquals, breaking any bucket policy that used StringNotEquals conditions (e.g., deny PutObject unless encryption matches a specific algorithm).

Fix: Remove StringNotEquals and StringNotEqualsIgnoreCase from is_negate(), since they already handle negation via the negate parameter. NotIpAddress still correctly uses is_negate() as its sole negation mechanism.

Tests fixed: test_bucket_policy_put_obj_kms_s3, test_bucket_policy_put_obj_s3_kms

2. delete_objects version mapping bug (object_usecase.rs)

execute_delete_objects used HashMap<object_name, index> to map deletion results back to the original request indices. When deleting multiple versions of the same key, only the last version's index was stored — previous versions were silently lost from the response.

Fix: Replace HashMap<String, usize> with Vec<usize> for position-based index mapping, correctly tracking each version independently.

Test fixed: test_versioning_concurrent_multi_object_delete

3. Missing S3 policy condition keys (key_name.rs)

Added s3:x-amz-grant-full-control, s3:x-amz-grant-read, s3:x-amz-grant-write, s3:x-amz-grant-read-acp, s3:x-amz-grant-write-acp condition key definitions to support ACL grant policy conditions.

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:

Additional Notes

  • Added unit tests for StringNotEquals condition evaluation in condition.rs (3 tests)
  • Added integration test for BucketPolicy deny with StringNotEquals and Null conditions in policy.rs
  • All 228 existing s3-tests pass (no regressions), plus 3 new tests now pass (total: 231)
  • Updated implemented_tests.txt (228 → 231) and unimplemented_tests.txt (17 → 11 failed)

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.

Made with Cursor

…n mapping

Fix three S3 compatibility bugs:

1. Policy StringNotEquals/StringNotEqualsIgnoreCase conditions were
   double-negated: once via the `negate` parameter in evaluate_with_resolver
   and again via is_negate(), making them behave like StringEquals. Remove
   the redundant negation in is_negate() for string conditions (NotIpAddress
   still correctly uses is_negate() as its sole negation mechanism).

2. delete_objects used a HashMap<object_name, index> to map results back,
   causing only the last version per key to be recorded when deleting
   multiple versions of the same object. Replace with a Vec<usize> for
   position-based mapping.

3. Add missing S3 policy condition keys (x-amz-grant-*) for ACL grant
   conditions.

Made-with: Cursor
Copilot AI review requested due to automatic review settings February 28, 2026 18:55
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

Fixes S3-compatibility issues in policy condition evaluation and multi-object delete responses, and updates the s3-tests tracking lists accordingly.

Changes:

  • Correct StringNotEquals* condition evaluation by removing unintended double-negation.
  • Fix DeleteObjects response index mapping for multiple versions of the same key.
  • Add missing S3 policy condition keys for ACL grant headers and expand policy test coverage.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
scripts/s3-tests/unimplemented_tests.txt Updates the “failed tests” list/count after fixing previously failing cases.
scripts/s3-tests/implemented_tests.txt Moves newly passing tests into the implemented list and updates totals/notes.
rustfs/src/app/object_usecase.rs Reworks DeleteObjects index mapping to handle duplicate keys (multiple versions) correctly.
crates/policy/src/policy/policy.rs Adds an integration-style test for deny logic using StringNotEquals + Null.
crates/policy/src/policy/function/key_name.rs Adds missing s3:x-amz-grant-* condition key definitions.
crates/policy/src/policy/function/condition.rs Fixes negation logic for StringNotEquals* and adds unit tests for condition evaluation.

Address review: object_sizes was keyed by object_name alone, causing
size overwrites when deleting multiple versions of the same key. Replace
HashMap<String, i64> with Vec<i64> parallel to object_to_delete so each
entry tracks its own size independently.

Made-with: Cursor
@overtrue overtrue merged commit e7466eb into main Feb 28, 2026
13 checks passed
@overtrue overtrue deleted the fix/policy-string-not-equals-double-negation branch February 28, 2026 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants