Commit c589b6b
http2: fix FileHandle leak in respondWithFile
Ensure that the file handle is closed if header validation fails in
respondWithFile. This prevents ERR_INVALID_STATE errors where a
FileHandle object is closed during garbage collection.
PR-URL: #61707
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tim Perry <pimterry@gmail.com>1 parent e7173a0 commit c589b6b
2 files changed
Lines changed: 55 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -2680,6 +2680,8 @@ function processRespondWithFD(self, fd, headers, offset = 0, length = -1, | |||
| 2680 | 2680 | try { | |
| 2681 | 2681 | headersList = buildNgHeaderString(headers, assertValidPseudoHeaderResponse); | |
| 2682 | 2682 | } catch (err) { | |
| 2683 | + if (self.ownsFd) | ||
| 2684 | + tryClose(fd); | ||
| 2683 | 2685 | self.destroy(err); | |
| 2684 | 2686 | return; | |
| 2685 | 2687 | } | |
@@ -2693,6 +2695,8 @@ function processRespondWithFD(self, fd, headers, offset = 0, length = -1, | |||
| 2693 | 2695 | const ret = self[kHandle].respond(headersList, streamOptions); | |
| 2694 | 2696 | ||
| 2695 | 2697 | if (ret < 0) { | |
| 2698 | + if (self.ownsFd) | ||
| 2699 | + tryClose(fd); | ||
| 2696 | 2700 | self.destroy(new NghttpError(ret)); | |
| 2697 | 2701 | return; | |
| 2698 | 2702 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,51 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + const common = require('../common'); | ||
| 3 | + if (!common.hasCrypto) | ||
| 4 | + common.skip('missing crypto'); | ||
| 5 | + const fixtures = require('../common/fixtures'); | ||
| 6 | + const assert = require('assert'); | ||
| 7 | + const http2 = require('http2'); | ||
| 8 | + const fs = require('fs'); | ||
| 9 | + | ||
| 10 | + const fname = fixtures.path('elipses.txt'); | ||
| 11 | + | ||
| 12 | + const server = http2.createServer(); | ||
| 13 | + | ||
| 14 | + server.on('stream', common.mustCall((stream) => { | ||
| 15 | + const originalClose = fs.close; | ||
| 16 | + let fdClosed = false; | ||
| 17 | + | ||
| 18 | + fs.close = common.mustCall(function(fd, cb) { | ||
| 19 | + fdClosed = true; | ||
| 20 | + return originalClose.apply(this, arguments); | ||
| 21 | + }); | ||
| 22 | + | ||
| 23 | + const headers = { | ||
| 24 | + ':method': 'GET', | ||
| 25 | + 'content-type': 'text/plain' | ||
| 26 | + }; | ||
| 27 | + | ||
| 28 | + stream.respondWithFile(fname, headers); | ||
| 29 | + | ||
| 30 | + stream.on('error', common.mustCall((err) => { | ||
| 31 | + assert.strictEqual(err.code, 'ERR_HTTP2_INVALID_PSEUDOHEADER'); | ||
| 32 | + })); | ||
| 33 | + | ||
| 34 | + stream.on('close', common.mustCall(() => { | ||
| 35 | + fs.close = originalClose; | ||
| 36 | + assert.strictEqual(fdClosed, true); | ||
| 37 | + })); | ||
| 38 | + })); | ||
| 39 | + | ||
| 40 | + server.listen(0, common.mustCall(() => { | ||
| 41 | + const client = http2.connect(`http://localhost:${server.address().port}`); | ||
| 42 | + const req = client.request(); | ||
| 43 | + | ||
| 44 | + req.on('close', common.mustCall(() => { | ||
| 45 | + client.close(); | ||
| 46 | + server.close(); | ||
| 47 | + })); | ||
| 48 | + | ||
| 49 | + req.on('error', common.mustCall()); | ||
| 50 | + req.end(); | ||
| 51 | + })); | ||
0 commit comments