From e183e5d87435f73baa83024e937a0bcddf807757 Mon Sep 17 00:00:00 2001 From: Mary Marchini Date: Tue, 28 Sep 2021 20:42:36 -0700 Subject: [PATCH 1/9] fix: use more reliable close event The close event from the request object is not guaranteed to fire on the same order across major versions of Node.js, the more accurate way to look if the connection was closed is to listen to the event on the socket. Fixes tests on v16. Ref: https://github.com/nodejs/node/issues/38924 --- lib/plugins/bodyReader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/plugins/bodyReader.js b/lib/plugins/bodyReader.js index a71cfb0be..82c9d1016 100644 --- a/lib/plugins/bodyReader.js +++ b/lib/plugins/bodyReader.js @@ -186,7 +186,7 @@ function bodyReader(options) { // add 'close and 'aborted' event handlers so that requests (and their // corresponding memory) don't leak if client stops sending data half // way through a POST request - req.once('close', next); + req.socket.once('close', next); req.once('aborted', next); req.resume(); } From 604ea331171daeb0afae26133a0abd700e25cac5 Mon Sep 17 00:00:00 2001 From: Mary Marchini Date: Tue, 28 Sep 2021 20:46:28 -0700 Subject: [PATCH 2/9] chore: fix flaky test --- test/plugins/static.test.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/plugins/static.test.js b/test/plugins/static.test.js index b80bff32b..52b88c4f2 100644 --- a/test/plugins/static.test.js +++ b/test/plugins/static.test.js @@ -350,11 +350,14 @@ describe('static resource plugin', function() { directory: TMP_PATH }); + var socket = new net.Socket(); SERVER.get('/index.html', function(req, res, next) { // closed before serve - serve(req, res, function(nextRoute) { - assert.strictEqual(nextRoute, false); - done(); + socket.on('end', function() { + serve(req, res, function(nextRoute) { + assert.strictEqual(nextRoute, false); + done(); + }); }); }); SERVER.on('after', function(req, res, route, afterErr) { @@ -362,7 +365,6 @@ describe('static resource plugin', function() { done(); }); - var socket = new net.Socket(); socket.connect({ host: '127.0.0.1', port: PORT }, function() { socket.write(RAW_REQUEST, 'utf-8', function(err2, data) { assert.ifError(err2); From 5b4f68e69dc97a7e4cff6bf425a6ef76258739f4 Mon Sep 17 00:00:00 2001 From: Mary Marchini Date: Tue, 28 Sep 2021 20:51:02 -0700 Subject: [PATCH 3/9] chore: Actions CI for 16 --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 61ed0beb6..4030abb7e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -31,6 +31,7 @@ jobs: - 10.x - 12.x - 14.x + - 16.x runs-on: ${{matrix.os}} steps: - uses: actions/checkout@v2 From 71502f8a4ec57889afb9ac3f815740c293c6bf6c Mon Sep 17 00:00:00 2001 From: Mary Marchini Date: Tue, 28 Sep 2021 21:16:39 -0700 Subject: [PATCH 4/9] chore: run ci on 8.x --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4030abb7e..f04558ab5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,6 +2,7 @@ on: push: branches: - master + - 8.x pull_request: branches: - master From 6ce3e3d95c96fcd4deeeefc10cc339360003fefc Mon Sep 17 00:00:00 2001 From: Mary Marchini Date: Tue, 28 Sep 2021 23:22:08 -0700 Subject: [PATCH 5/9] docs(CHANGELOG): Update changelog --- CHANGELOG.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 73a962d1d..0409ad136 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,20 @@ + +## 8.6.0 (2021-09-28) + + +#### Bug Fixes + +* use more reliable close event ([e183e5d8](git://github.com/restify/node-restify.git/commit/e183e5d8)) +* examples/todoapp/package.json to reduce vulnerabilities (#1832) ([d9b27c60](git://github.com/restify/node-restify.git/commit/d9b27c60)) +* **npm:** exclude extraneous files (#1818) ([e8516c37](git://github.com/restify/node-restify.git/commit/e8516c37)) + + +#### Features + +* send 500s for unhandled requests (#1777) ([885cecd7](git://github.com/restify/node-restify.git/commit/885cecd7)) +* **deps:** replace cover/istanbul with nyc (#1823) ([361f83e5](git://github.com/restify/node-restify.git/commit/361f83e5)) + + ### 8.5.1 (2019-12-13) From 8861e02eb38f6c2a5bdc614050d3687b54deb578 Mon Sep 17 00:00:00 2001 From: Mary Marchini Date: Tue, 28 Sep 2021 23:22:35 -0700 Subject: [PATCH 6/9] chore(release): release 8.6.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 05c1d1fc2..ae0c295a5 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,7 @@ "express", "DTrace" ], - "version": "8.5.1", + "version": "8.6.0", "repository": { "type": "git", "url": "git://github.com/restify/node-restify.git" From 91a42d4463e7343678875b44eb441f17e4a97b57 Mon Sep 17 00:00:00 2001 From: Joey Harrington Date: Wed, 9 Feb 2022 11:49:27 -0800 Subject: [PATCH 7/9] fix: use close event on response instead of socket (#1892) --- lib/plugins/bodyReader.js | 2 +- test/plugins/bodyReader.test.js | 136 ++++++++++++++++++++++++++++---- 2 files changed, 123 insertions(+), 15 deletions(-) diff --git a/lib/plugins/bodyReader.js b/lib/plugins/bodyReader.js index 82c9d1016..60ff1f462 100644 --- a/lib/plugins/bodyReader.js +++ b/lib/plugins/bodyReader.js @@ -186,7 +186,7 @@ function bodyReader(options) { // add 'close and 'aborted' event handlers so that requests (and their // corresponding memory) don't leak if client stops sending data half // way through a POST request - req.socket.once('close', next); + res.once('close', next); req.once('aborted', next); req.resume(); } diff --git a/test/plugins/bodyReader.test.js b/test/plugins/bodyReader.test.js index b799fef90..3e48a4298 100644 --- a/test/plugins/bodyReader.test.js +++ b/test/plugins/bodyReader.test.js @@ -18,25 +18,25 @@ var CLIENT; var PORT; describe('body reader', function() { - describe('gzip content encoding', function() { - beforeEach(function(done) { - SERVER = restify.createServer({ - dtrace: helper.dtrace, - log: helper.getLog('server') - }); + beforeEach(function(done) { + SERVER = restify.createServer({ + dtrace: helper.dtrace, + log: helper.getLog('server') + }); - SERVER.listen(0, '127.0.0.1', function() { - PORT = SERVER.address().port; + SERVER.listen(0, '127.0.0.1', function() { + PORT = SERVER.address().port; - done(); - }); + done(); }); + }); - afterEach(function(done) { - CLIENT.close(); - SERVER.close(done); - }); + afterEach(function(done) { + CLIENT.close(); + SERVER.close(done); + }); + describe('gzip content encoding', function() { it('should parse gzip encoded content', function(done) { SERVER.use(restify.plugins.bodyParser()); @@ -187,4 +187,112 @@ describe('body reader', function() { req.write(postData); }); }); + + it('should not add a listener for each call on same socket', done => { + SERVER.use(restify.plugins.bodyParser()); + + let serverReq, serverRes, serverReqSocket; + SERVER.post('/meals', function(req, res, next) { + serverReq = req; + serverRes = res; + serverReqSocket = req.socket; + res.send(); + next(); + }); + + CLIENT = restifyClients.createJsonClient({ + url: 'http://127.0.0.1:' + PORT, + retry: false, + agent: new http.Agent({ keepAlive: true }) + }); + + CLIENT.post('/meals', { breakfast: 'pancakes' }, (err, _, res) => { + assert.ifError(err); + assert.equal(res.statusCode, 200); + + const firstReqSocket = serverReqSocket; + const numReqListeners = listenerCount(serverReq); + const numResListeners = listenerCount(serverRes); + const numReqSocketListeners = listenerCount(serverReq.socket); + + // Without setImmediate, the second request will not reuse the socket. + setImmediate(() => { + CLIENT.post('/meals', { lunch: 'salad' }, (err2, __, res2) => { + assert.ifError(err2); + assert.equal(res2.statusCode, 200); + assert.equal( + serverReqSocket, + firstReqSocket, + 'This test should issue two requests that share the ' + + 'same socket.' + ); + // The number of listeners on each emitter should not have + // increased since the first request. + assert.equal(listenerCount(serverReq), numReqListeners); + assert.equal(listenerCount(serverRes), numResListeners); + assert.equal( + listenerCount(serverReq.socket), + numReqSocketListeners + ); + done(); + }); + }); + }); + }); + + it('should call next for each successful request on same socket', done => { + let nextCallCount = 0; + SERVER.use(restify.plugins.bodyParser()); + SERVER.use((req, res, next) => { + nextCallCount += 1; + next(); + }); + + let serverReqSocket; + SERVER.post('/meals', function(req, res, next) { + res.send(); + next(); + }); + + CLIENT = restifyClients.createJsonClient({ + url: 'http://127.0.0.1:' + PORT, + retry: false, + agent: new http.Agent({ keepAlive: true }) + }); + + CLIENT.post('/meals', { breakfast: 'waffles' }, (err, _, res) => { + assert.ifError(err); + assert.equal(res.statusCode, 200); + const firstReqSocket = serverReqSocket; + assert.equal(nextCallCount, 1); + + // Without setImmediate, the second request will not reuse the socket. + setImmediate(() => { + CLIENT.post('/meals', { lunch: 'candy' }, (err2, __, res2) => { + assert.ifError(err2); + assert.equal(res2.statusCode, 200); + assert.equal( + serverReqSocket, + firstReqSocket, + 'This test should issue two requests that share the ' + + 'same socket.' + ); + assert.equal(nextCallCount, 2); + done(); + }); + }); + }); + }); }); + +/** + * @param {EventEmitter} emitter - An emitter + * @returns {number} - The total number of listeners across all events + */ +function listenerCount(emitter) { + let numListeners = 0; + for (const eventName of emitter.eventNames()) { + numListeners += emitter.listenerCount(eventName); + } + return numListeners; +} From 1482a76f97b65b2e07dbdc164140ec38b9581702 Mon Sep 17 00:00:00 2001 From: Joey Harrington Date: Thu, 10 Feb 2022 14:56:54 -0800 Subject: [PATCH 8/9] docs(CHANGELOG): Update changelog --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0409ad136..297834a75 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,12 @@ + +### 8.6.1 (2022-02-10) + + +#### Bug Fixes + +* use close event on response instead of socket (#1892) ([91a42d44](git://github.com/restify/node-restify.git/commit/91a42d44)) + + ## 8.6.0 (2021-09-28) From 5bb5e68d368e2cc286134657a2aa3606711515b0 Mon Sep 17 00:00:00 2001 From: Joey Harrington Date: Thu, 10 Feb 2022 14:57:27 -0800 Subject: [PATCH 9/9] chore(release): release 8.6.1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index ae0c295a5..503b32498 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,7 @@ "express", "DTrace" ], - "version": "8.6.0", + "version": "8.6.1", "repository": { "type": "git", "url": "git://github.com/restify/node-restify.git"