fix: stop buffering data after SOCKS5 connect#5118
Conversation
Assisted-by: openai:gpt-5.5 Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5118 +/- ##
==========================================
- Coverage 93.14% 93.14% -0.01%
==========================================
Files 110 110
Lines 35908 36109 +201
==========================================
+ Hits 33446 33633 +187
- Misses 2462 2476 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| const boundPort = this.buffer.readUInt16BE(offset) | ||
|
|
||
| this.buffer = this.buffer.subarray(responseLength) | ||
| this.buffer = Buffer.alloc(0) |
There was a problem hiding this comment.
Does this need to be reallocated? We could just keep one around, they are just empty.
There was a problem hiding this comment.
The SOCKS5 client removes its data listener, but the client object can still be retained by the socket’s error/close listeners. If this.buffer is not cleared, it can retain the SOCKS5 response buffer.
We don't need fresh allocation every time, as it's replaced using Buffer.concat(...)
I replaced this with EMPTY_BUFFER constant in f8a876f
(cherry picked from commit 74e8706)
This relates to...
Fixes: #5117
Rationale
After a SOCKS5 CONNECT succeeds, the socket carries tunneled HTTP/TLS data and should no longer be parsed by
Socks5Client. The existing permanentdatalistener kept appending tunneled response chunks to the internal protocol buffer, causing duplicate retained memory for the lifetime of the socket.Changes
datahandler so it can be removed after CONNECT succeeds.socket.unshift().Features
N/A
Bug Fixes
Stop buffering data after SOCKS5 connect
Breaking Changes and Deprecations
N/A
Status
Assisted-by: openai:gpt-5.5