Skip to content

S3: preserve repeated object path slashes with a per-driver option - #2189

Merged
micafer merged 8 commits into
apache:trunkfrom
Shubham-Padkonde:fix/s3-preserve-object-slashes
Nov 10, 2026
Merged

micafer merged 8 commits into
apache:trunkfrom
Shubham-Padkonde:fix/s3-preserve-object-slashes

Conversation

@Shubham-Padkonde

@Shubham-Padkonde Shubham-Padkonde commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Description

Addresses #1654 with the per-driver opt-in discussed in the issue. S3 object names may contain leading or repeated slashes, but the default connection URL normalization can collapse them. Deleting such a key can return success for a different path while leaving the intended object in the bucket.

Add S3StorageDriver(..., ex_allow_path_double_slashes=True) to preserve the request path before signing and sending it. The default remains unchanged, the existing global setting remains supported, and enabling the option does not affect another driver instance. Includes documentation and a changelog entry.

Regression tests inspect the outgoing DELETE request for /object, path//object, and //path///object, and verify per-driver isolation and global-setting compatibility. Before the fix, all three request-path assertions and the per-driver assertion fail.

Status

Ready for review. Prepared with OpenAI Codex assistance.

Validation:

  • Linux / Python 3.14: python -m pytest libcloud/test/storage/test_s3.py libcloud/test/common/test_base.py libcloud/test/test_connection.py -q — 89 passed, 1 skipped, 3 subtests passed.
  • Black, isort, flake8 on both changed Python files; configured Pylint error check on the production file; git diff --check — passed.
  • An initial Windows / Python 3.12 run had two existing download-test temporary-file failures; both pass on Linux. Existing datetime deprecation warnings remain.
  • No live AWS account testing or full provider suite run. HTTP responses are mocked; request construction and signing use the real driver.

Checklist

  • Code linting
  • Documentation
  • Tests
  • ICLA (no signature claimed; this is a small driver option and regression fix)

November 8 validation after merging trunk: 53 S3 tests passed, one skipped, with three passing subtests. Two Windows download tests failed on both this branch and unchanged trunk (which had 51 passes). The S3 implementation and regression tests match the previously published version; the conflict was in the release notes.

November 10: merged trunk at 0317e58 and retained both upstream storage release notes and the S3 entry. The conflict only affected CHANGES.rst; our S3 implementation and tests are unchanged. Targeted S3/common-base/connection tests: 85 pass, one skip, three passing subtests. Four failures reproduce on unchanged trunk (83 pass): two Windows temporary-file download failures and two proxy-environment assertions. Whitespace validation passes. No live AWS or full provider-suite run. Published as 774f842.

@codecov-commenter

codecov-commenter commented Nov 10, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.64%. Comparing base (0317e58) to head (55e4085).
⚠️ Report is 11 commits behind head on trunk.

Additional details and impacted files
@@            Coverage Diff             @@
##            trunk    #2189      +/-   ##
==========================================
+ Coverage   83.62%   83.64%   +0.02%     
==========================================
  Files         352      352              
  Lines       82233    82321      +88     
  Branches     8840     8868      +28     
==========================================
+ Hits        68767    68853      +86     
- Misses      10593    10594       +1     
- Partials     2873     2874       +1     
Files with missing lines Coverage Δ
libcloud/storage/drivers/s3.py 89.71% <100.00%> (+0.09%) ⬆️
libcloud/test/storage/test_s3.py 94.54% <100.00%> (+0.32%) ⬆️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@micafer micafer left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@micafer
micafer merged commit d3bb996 into apache:trunk Nov 10, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants