https: fix readable listener leak during proxy CONNECT - #62913
thisalihassan wants to merge 2 commits into
Conversation
|
Review requested:
|
nodejs-github-bot
added
https
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #62913 +/- ##
==========================================
- Coverage 89.65% 89.62% -0.03%
==========================================
Files 708 708
Lines 220410 220410
Branches 42270 42269 -1
==========================================
- Hits 197605 197551 -54
- Misses 14657 14704 +47
- Partials 8148 8155 +7
🚀 New features to boost your workflow:
|
| // Proxy server that writes the CONNECT response one byte at a time. Using the | ||
| // built-in HTTP server avoids having to parse the CONNECT request ourselves | ||
| // (which cannot rely on a single TCP chunk containing the whole request line). | ||
| const proxy = http.createServer(); |
There was a problem hiding this comment.
Can you move the proxy to a separate process? I recall there can be problems otherwise if the environment contains proxy variables.
EDIT: looking at the patterns of other files I think it should be starting the request from a different process instead. Or at least avoid having the requester and the proxy in the same process.
There was a problem hiding this comment.
@joyeecheung done, moved the requester to a subprocess via runProxiedRequest
establishTunnel() reused the same function as both the initial reader and the 'readable' listener, and re-subscribed it at the end of every call. Fixes: nodejs#62904 Signed-off-by: Ali Hassan <ali-hassan27@outlook.com>
Signed-off-by: Ali Hassan <ali-hassan27@outlook.com>
44d675e to
12f0f8d
Compare
|
This pull request has been marked as stale due to 90 days of inactivity. |
github-actions
Bot
added
the
stale
4 participants
establishTunnel() reused the same function as both the initial reader and the
'readable'listener, and re-subscribed it at the end of every call. BecauseEventEmitteriterates a snapshot of its listeners per emit, each'readable'event fired every already-attached copy, and each copy re-subscribedFixes: #62904