fix: align multipart listing response with S3 - #1384
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The changes are small, match the stated S3-compatibility goal, and include direct test coverage verifying the updated response shape.
Pull request overview
This PR is a compatibility fix to make the ListMultipartUploads response produced by the S3 protocol handler match the AWS S3 API shape more closely, following up on earlier listing changes from #1346.
Changes:
- Renamed
ListMultipartUploadsResult.NametoListMultipartUploadsResult.Bucket. - Removed
ListMultipartUploadsResult.KeyCountfrom the multipart listing response. - Updated unit tests to assert the new response shape (and the absence of the old fields).
File summaries
| File | Description |
|---|---|
| src/storage/protocols/s3/s3-handler.ts | Adjusts the multipart listing response payload to use Bucket and omit KeyCount to align with S3. |
| src/storage/protocols/s3/s3-handler.test.ts | Adds assertions validating Bucket is present and Name/KeyCount are not returned. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward fix aligning the ListMultipartUploads response shape with the real S3 API.
Reviewed the listMultipartUploads response construction in s3-handler.ts: confirmed Name → Bucket rename and removal of the non-existent KeyCount field match AWS's ListMultipartUploadsResult schema (KeyCount is only part of ListObjectsV2, not multipart listing). Checked that the accompanying test assertions correctly verify Bucket is set and Name/KeyCount are absent.
Extended reasoning...
Overview
The PR touches a single response-shaping code path in src/storage/protocols/s3/s3-handler.ts: the object literal returned by listMultipartUploads. It renames Name to Bucket and deletes an erroneous KeyCount field, with a matching test update in s3-handler.test.ts.
Security risks
None. This is purely a response schema correction for an S3-compatibility endpoint — no auth, crypto, or data-exposure logic is touched. The field values themselves (bucket name, counts) were already being computed correctly; only the field naming/presence changes.
Level of scrutiny
Low. This is a small, mechanical, self-contained diff with clear intent (align with the real AWS S3 XML schema for ListMultipartUploadsResult), and it's accompanied by targeted test assertions that lock in the new behavior. No CODEOWNERS file was found restricting this path, and the bug hunter reported zero findings.
Other factors
The change is easily verifiable against public AWS API documentation (ListMultipartUploadsResult uses Bucket, not Name, and has no KeyCount field — that's specific to ListObjectsV2). Test coverage was updated in the same commit to assert the new field and the absence of the removed ones, which is appropriate for a fix of this size.
Coverage Report for CI Build 34509609861Warning Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes. Coverage remained the same at 82.684%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
What kind of change does this PR introduce?
Small follow-up to #1346 that corrects the ListMultipartUploads response shape to match the S3 API by returning Bucket instead of Name and removing KeyCount.