Skip to content

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

Open
Shubham-Padkonde wants to merge 4 commits into
apache:trunkfrom
Shubham-Padkonde:fix/s3-preserve-object-slashes
Open

Shubham-Padkonde wants to merge 4 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

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

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.63%. Comparing base (0317e58) to head (774f842).
⚠️ Report is 7 commits behind head on trunk.

Additional details and impacted files
@@            Coverage Diff             @@
##            trunk    #2189      +/-   ##
==========================================
+ Coverage   83.62%   83.63%   +0.01%     
==========================================
  Files         352      352              
  Lines       82233    82262      +29     
  Branches     8840     8846       +6     
==========================================
+ Hits        68767    68796      +29     
  Misses      10593    10593              
  Partials     2873     2873              
Files with missing lines Coverage Δ
libcloud/storage/drivers/s3.py 89.71% <100.00%> (+0.09%) ⬆️
libcloud/test/storage/test_s3.py 94.41% <100.00%> (+0.20%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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