http: fix drain event with cork/uncork

[davidje13]

Fixes #60432

This is a copy of PR #60437, so credit for the fix goes to @ThierryMT. Sadly that PR was abandoned and I wasn't able to get a response from the author. I've just applied their changes to the latest main, patched up the lint errors, and applied the suggested change to the tests. I've signed the certificate of origin on the basis that the test is based on the reproduction I provided in the original bug report #60432 (hence, created in part by me), but let me know if this is too tenuous and I can write a new test from scratch instead. The fix itself is so trivial that I'm not sure there's a way to write it which isn't effectively the same code.

---

Original PR description:

When using cork() and uncork() with ServerResponse, the drain event was not reliably emitted after uncorking. This occurred because the uncork() method did not check if a drain was pending (kNeedDrain flag) after flushing the chunked buffer.

The Problem:

• When corked, write() buffers data in kChunkedBuffer instead of sending immediately
• If write() returns false, kNeedDrain is set to true
• When uncork() is called, it flushes the buffer by calling _send()
• After flushing, uncork() never checked if drain was needed or emitted the event
• This left the response waiting for a drain event that would never fire

The Fix:
This commit ensures that when uncork() successfully flushes buffered data and a drain was needed, the drain event is emitted immediately.

Changes:

• Modified OutgoingMessage.prototype.uncork() in lib/_http_outgoing.js
• Capture return value from final _send() call
• Check if drain was pending after successful flush
• Emit drain event when appropriate

Testing:
The issue could be reproduced by using cork() with multiple large writes and checking for drain events. With this fix, the drain event now fires correctly after uncork().

[nodejs-github-bot]

Review requested:

• @nodejs/http
• @nodejs/net

[Copilot]

Pull request overview

This pull request fixes a reliability issue where ServerResponse could get stuck with writableNeedDrain === true when using cork()/uncork(), because uncork() flushed the internal chunked buffer without emitting 'drain' when no socket-level backpressure occurred.

Changes:

• Update OutgoingMessage.prototype.uncork() to emit 'drain' after flushing the chunked buffer when a drain was pending.
• Add a regression test covering ServerResponse 'drain' behavior when corked writes hit backpressure.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
lib/_http_outgoing.js	Emits 'drain' from uncork() when the buffered chunked payload is flushed successfully and kNeedDrain was set.
test/parallel/test-http-response-drain-cork.js	Adds a regression test for the cork/uncork drain behavior on ServerResponse.

---

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

[ronag]

Please fix copilot feedback.

[davidje13]

Applied the requested changes, and simplified the test a little since setting an explicit high water mark means we don't need to be so extreme with the volume of data.

I also removed the '3' part of the test, which I don't think was testing anything useful here, and is likely just a remnant of how I originally diagnosed the issue.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74331/

[mcollina]

lgtm

[davidje13]

Amended first commit message to fix commit lint error (also rebased on latest main)

[mcollina]

lgtm

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74353/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74358/

[davidje13]

Is anybody able to tell me what the CI errors are? (I just get "Access Denied / davidje13 is missing the Overall/Read permission" for all 3 failed runs)

Looks like a Windows-specific issue? Or a test flake?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74505/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/74508/

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/64038
✔  Done loading data for nodejs/node/pull/64038
----------------------------------- PR info ------------------------------------
Title      http: fix drain event with cork/uncork (#64038)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     davidje13:cork-drain -> nodejs:main
Labels     http, author ready, needs-ci
Commits    2
 - http: fix drain event with cork/uncork
 - http: Simplify drain-on-uncork logic and test
Committers 1
 - David Evans <davidje13@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/64038
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/64038
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 21 Jun 2026 01:03:22 GMT
   ✔  Approvals: 3
   ✔  - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/64038#pullrequestreview-4539742987
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/64038#pullrequestreview-4541802163
   ✔  - Tim Perry (@pimterry): https://github.com/nodejs/node/pull/64038#pullrequestreview-4544466402
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2026-07-02T11:35:53Z: https://ci.nodejs.org/job/node-test-pull-request/74508/
- Querying data for job/node-test-pull-request/74508/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 64038
From https://github.com/nodejs/node
 * branch                  refs/pull/64038/merge -> FETCH_HEAD
✔  Fetched commits as 85c31dfae1f7..daed51b4a858
--------------------------------------------------------------------------------
[main ae85ef5f20] http: fix drain event with cork/uncork
 Author: David Evans <davidje13@users.noreply.github.com>
 Date: Sun Jun 21 01:36:14 2026 +0100
 2 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 test/parallel/test-http-response-drain-cork.js
[main d22a21f2dc] http: Simplify drain-on-uncork logic and test
 Author: David Evans <davidje13@users.noreply.github.com>
 Date: Sun Jun 21 13:55:20 2026 +0100
 2 files changed, 19 insertions(+), 22 deletions(-)
   ✔  Patches applied
There are 2 commits in the PR. Attempting autorebase.
(node:388) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.
(Use `node --trace-deprecation ...` to show where the warning was created)
Rebasing (2/4)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
http: fix drain event with cork/uncork
When using cork() and uncork() with ServerResponse, the drain

event was not reliably emitted after uncorking. This occurred

because the uncork() method did not check if a drain was pending

(kNeedDrain flag) after flushing the chunked buffer.
This fix ensures that when uncork() successfully flushes buffered

data and a drain was needed, the drain event is emitted

immediately.
This commit is a copy of PR #60437 (abandoned) with minor linting

fixes.
Fixes: #60432

Signed-off-by: David Evans <davidje13@users.noreply.github.com>

PR-URL: #64038

Reviewed-By: Robert Nagy <ronagy@icloud.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Reviewed-By: Tim Perry <pimterry@gmail.com>
[detached HEAD 2b27a24fdf] http: fix drain event with cork/uncork

Author: David Evans <davidje13@users.noreply.github.com>

Date: Sun Jun 21 01:36:14 2026 +0100

2 files changed, 74 insertions(+), 1 deletion(-)

create mode 100644 test/parallel/test-http-response-drain-cork.js

Rebasing (3/4)

Rebasing (4/4)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

http: Simplify drain-on-uncork logic and test
Check internal state when deciding whether to send a drain event

on uncork() - instead of relying on the return value of _send - as

suggested in the PR comments.
The test is updated to set an explicit high water mark for better

reliability, and since this means we can use a much lower value,

the size of data in the test has been reduced significantly.
PR-URL: #64038

Reviewed-By: Robert Nagy <ronagy@icloud.com>

Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

Reviewed-By: Tim Perry <pimterry@gmail.com>
[detached HEAD ec37800699] http: Simplify drain-on-uncork logic and test

Author: David Evans <davidje13@users.noreply.github.com>

Date: Sun Jun 21 13:55:20 2026 +0100

2 files changed, 19 insertions(+), 22 deletions(-)

Successfully rebased and updated refs/heads/main.
ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/28598122034

[nodejs-github-bot]

Landed in 615749b