Visitar URL original
fix(s3): use raw multipart upload pagination markers by hsusul · Pull Request #1487 · supabase/storage · GitHub
Skip to content

fix(s3): use raw multipart upload pagination markers - #1487

Open
hsusul wants to merge 1 commit into
supabase:masterfrom
hsusul:fix/storage-correctness-oct7
Open

hsusul wants to merge 1 commit into
supabase:masterfrom
hsusul:fix/storage-correctness-oct7

Conversation

@hsusul

@hsusul hsusul commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix. Fixes #1486.

What is the current behavior?

ListMultipartUploads rejects valid object-key/upload-ID markers with Invalid Parameter continuation token, because it expects private base64 tokens. Truncated responses also wrap both next markers in those tokens, contrary to S3's marker contract.

What is the new behavior?

Pass raw keys and upload IDs to the existing database pagination path. Return the last listed key/upload ID as next markers, URL-encoding key response fields when EncodingType: 'url' is requested. Remove the two now-unused multipart-only token helpers.

Regression coverage exercises raw marker input, a two-page same-key listing, URL-encoded keys containing Unicode/colon/slash/space, echoed markers, and the final untruncated page. The existing colon-preservation test now uses an S3 key marker directly.

Additional context

Validation on upstream bc38cb97:

  • Existing handler/database baseline: 121 tests passed.
  • All three new regression cases fail with the unchanged upstream handler; all three pass with the fix.
  • Focused handler/database/router tests: 279 passed (npm run test:unit -- --maxWorkers=2 src/storage/protocols/s3/s3-handler.test.ts src/storage/database/pg.test.ts src/http/routes/s3/router.test.ts).
  • Full unit suite: 2,782 tests / 166 files passed (npm run test:unit -- --maxWorkers=2).
  • npm run build, npm run lint (Biome and Prettier), and git diff --check passed.
  • Docker integration tests were not run: the isolated Compose stack could not bind port 9000, which was already allocated. Only this task's Compose resources were removed. An initial unbounded unit run was interrupted after unrelated pool-test timeouts under resource contention; the bounded full run passed.

Compatibility: previously saved private base64 multipart markers must be discarded and listings restarted. Markers now follow the S3 API contract; object-list continuation tokens are unaffected. No database or dependency changes.

Related merged #1346 addresses multipart ordering and #1384 addresses response shape; this fixes the distinct wire-marker format. Searches of issues/PRs/comments, discussions, commits, and local history found no blocking overlap.

@hsusul
hsusul requested a review from a team as a code owner October 7, 2026 20:03
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 37679183507

Coverage decreased (-0.07%) to 84.872%

Details

  • Coverage decreased (-0.07%) from the base build.
  • Patch coverage: 2 of 2 lines across 1 file are fully covered (100%).
  • 17 coverage regressions across 1 file.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

17 previously-covered lines in 1 file lost coverage.

File Lines Losing Coverage Coverage
src/internal/http/agent.ts 17 32.56%

Coverage Stats

Coverage Status
Relevant Lines: 14520
Covered Lines: 12768
Line Coverage: 87.93%
Relevant Branches: 8967
Covered Branches: 7166
Branch Coverage: 79.92%
Branches in Coverage %: Yes
Coverage Strength: 782.07 hits per line

💛 - Coveralls

This branch has not been deployed

No deployments
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.

S3 ListMultipartUploads rejects raw key and upload ID markers

2 participants