GeezerVison is showing github.com — tls: fix leak of WriteWrap+TLSWrap combination · nodejs/node@bf25994
File tree
Expand file treeCollapse file tree
Open diff view settings
Expand file treeCollapse file tree
Open diff view settings
Collapse file
Copy file name to clipboardExpand all lines: lib/_tls_wrap.js +20-3Lines changed: 20 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -317,14 +317,31 @@ proxiedMethods.forEach(function(name) { |
317 | 317 | }); |
318 | 318 |
|
319 | 319 | tls_wrap.TLSWrap.prototype.close = function closeProxy(cb) { |
320 | | - if (this.owner) |
| 320 | + let ssl; |
| 321 | + if (this.owner) { |
| 322 | + ssl = this.owner.ssl; |
321 | 323 | this.owner.ssl = null; |
| 324 | + } |
| 325 | + |
| 326 | + // Invoke `destroySSL` on close to clean up possibly pending write requests |
| 327 | + // that may self-reference TLSWrap, leading to leak |
| 328 | + const done = () => { |
| 329 | + if (ssl) { |
| 330 | + ssl.destroySSL(); |
| 331 | + if (ssl._secureContext.singleUse) { |
| 332 | + ssl._secureContext.context.close(); |
| 333 | + ssl._secureContext.context = null; |
| 334 | + } |
| 335 | + } |
| 336 | + if (cb) |
| 337 | + cb(); |
| 338 | + }; |
322 | 339 |
|
323 | 340 | if (this._parentWrap && this._parentWrap._handle === this._parent) { |
324 | | - this._parentWrap.once('close', cb); |
| 341 | + this._parentWrap.once('close', done); |
325 | 342 | return this._parentWrap.destroy(); |
326 | 343 | } |
327 | | - return this._parent.close(cb); |
| 344 | + return this._parent.close(done); |
328 | 345 | }; |
329 | 346 |
|
330 | 347 | TLSSocket.prototype._wrapHandle = function(wrap) { |
|
Collapse file
Copy file name to clipboard +26Lines changed: 26 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
|
| 1 | +'use strict'; |
| 2 | +const common = require('../common'); |
| 3 | + |
| 4 | +if (!common.hasCrypto) { |
| 5 | + common.skip('missing crypto'); |
| 6 | + return; |
| 7 | +} |
| 8 | + |
| 9 | +const assert = require('assert'); |
| 10 | +const net = require('net'); |
| 11 | +const tls = require('tls'); |
| 12 | + |
| 13 | +const server = net.createServer(common.mustCall((c) => { |
| 14 | + c.destroy(); |
| 15 | +})).listen(0, common.mustCall(() => { |
| 16 | + const c = tls.connect({ port: server.address().port }); |
| 17 | + c.on('error', () => { |
| 18 | + // Otherwise `.write()` callback won't be invoked. |
| 19 | + c.destroyed = false; |
| 20 | + }); |
| 21 | + |
| 22 | + c.write('hello', common.mustCall((err) => { |
| 23 | + assert.equal(err.code, 'ECANCELED'); |
| 24 | + server.close(); |
| 25 | + })); |
| 26 | +})); |