Skip to content

fix: leak in iceberg upload part#1123

Open
ferhatelmas wants to merge 2 commits into
masterfrom
ferhat/iceberg-leak
Open

fix: leak in iceberg upload part#1123
ferhatelmas wants to merge 2 commits into
masterfrom
ferhat/iceberg-leak

Conversation

@ferhatelmas
Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Bug fix

What is the current behavior?

Upload part is using passthrough but not consuming it.

What is the new behavior?

Passthrough is passed correctly.

Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
Copilot AI review requested due to automatic review settings May 21, 2026 12:47
@ferhatelmas ferhatelmas requested a review from a team as a code owner May 21, 2026 12:47
Copy link
Copy Markdown
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

This PR fixes a stream leak in the Iceberg-specific S3 UploadPart route by ensuring the PassThrough stream that the request body is piped into is actually the stream consumed by the upload implementation.

Changes:

  • Always create and pipe ctx.req.raw into a PassThrough for the Iceberg UploadPart handler.
  • Use the PassThrough as the Body stream for non-streaming-signature uploads (previously, ctx.req.raw was used while the PassThrough remained unconsumed).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coveralls
Copy link
Copy Markdown

coveralls commented May 21, 2026

Coverage Report for CI Build 26506685018

Coverage increased (+0.02%) to 75.19%

Details

  • Coverage increased (+0.02%) from the base build.
  • Patch coverage: 6 uncovered changes across 1 file (0 of 6 lines covered, 0.0%).
  • 1 coverage regression across 1 file.

Uncovered Changes

File Changed Covered %
src/http/routes/s3/commands/upload-part.ts 6 0 0.0%

Coverage Regressions

1 previously-covered line in 1 file lost coverage.

File Lines Losing Coverage Coverage
src/http/routes/s3/commands/upload-part.ts 1 40.0%

Coverage Stats

Coverage Status
Relevant Lines: 10639
Covered Lines: 8433
Line Coverage: 79.26%
Relevant Branches: 6229
Covered Branches: 4250
Branch Coverage: 68.23%
Branches in Coverage %: Yes
Coverage Strength: 363.46 hits per line

💛 - Coveralls

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.

4 participants