From 73b4dee19c75322b3516e13b8beff9b087befab2 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Thu, 22 Mar 2018 22:47:39 +0530 Subject: [PATCH] test: rename regression tests with descriptive filenames Rename test-dgram-regress-4496 to test-dgram-typeerror-buffer to test-dgram-send-invalid-msg-type Rename test-http-regr-gh-2821 to test-http-request-large-payload Rename test-child-process-fork-regr-gh-2847 to test-child-process-fork-closed-channel-segfault Rename test-http-pipeline-regr-2639 to test-http-pipeline-serverresponse-assertionerror Rename test-http-pipeline-regr-3332 to test-http-pipeline-requests-connection-leak Rename test-http-pipeline-regr-3508 to test-http-pipeline-socket-parser-typeerror Refs: https://github.com/nodejs/node/issues/19105 Refs: https://github.com/nodejs/node/blob/master/doc/guides/writing-tests.md#test-structure --- test/parallel/test-async-wrap-GH13045.js | 55 -------------- .../test-async-wrap-tlssocket-asyncreset.js | 66 ++++++++++++++++ ...ld-process-fork-closed-channel-segfault.js | 75 +++++++++++++++++++ .../test-child-process-fork-regr-gh-2847.js | 69 ----------------- ...est-http-pipeline-assertionerror-finish.js | 34 +++++++++ test/parallel/test-http-pipeline-regr-2639.js | 24 ------ test/parallel/test-http-pipeline-regr-3332.js | 27 ------- test/parallel/test-http-pipeline-regr-3508.js | 55 -------------- ...-http-pipeline-requests-connection-leak.js | 34 +++++++++ ...t-http-pipeline-socket-parser-typeerror.js | 64 ++++++++++++++++ ....js => test-http-request-large-payload.js} | 6 +- 11 files changed, 278 insertions(+), 231 deletions(-) delete mode 100644 test/parallel/test-async-wrap-GH13045.js create mode 100644 test/parallel/test-async-wrap-tlssocket-asyncreset.js create mode 100644 test/parallel/test-child-process-fork-closed-channel-segfault.js delete mode 100644 test/parallel/test-child-process-fork-regr-gh-2847.js create mode 100644 test/parallel/test-http-pipeline-assertionerror-finish.js delete mode 100644 test/parallel/test-http-pipeline-regr-2639.js delete mode 100644 test/parallel/test-http-pipeline-regr-3332.js delete mode 100644 test/parallel/test-http-pipeline-regr-3508.js create mode 100644 test/parallel/test-http-pipeline-requests-connection-leak.js create mode 100644 test/parallel/test-http-pipeline-socket-parser-typeerror.js rename test/parallel/{test-http-regr-gh-2821.js => test-http-request-large-payload.js} (70%) diff --git a/test/parallel/test-async-wrap-GH13045.js b/test/parallel/test-async-wrap-GH13045.js deleted file mode 100644 index bb4e1a0c411666..00000000000000 --- a/test/parallel/test-async-wrap-GH13045.js +++ /dev/null @@ -1,55 +0,0 @@ -'use strict'; -const common = require('../common'); -if (!common.hasCrypto) - common.skip('missing crypto'); - -// Refs: https://github.com/nodejs/node/issues/13045 -// An HTTP Agent reuses a TLSSocket, and makes a failed call to `asyncReset`. - -const assert = require('assert'); -const https = require('https'); -const fixtures = require('../common/fixtures'); - -const serverOptions = { - key: fixtures.readKey('agent1-key.pem'), - cert: fixtures.readKey('agent1-cert.pem'), - ca: fixtures.readKey('ca1-cert.pem') -}; - -const server = https.createServer(serverOptions, common.mustCall((req, res) => { - res.end('hello world\n'); -}, 2)); - -server.listen(0, common.mustCall(function() { - const port = this.address().port; - const clientOptions = { - agent: new https.Agent({ - keepAlive: true, - rejectUnauthorized: false - }), - port: port - }; - - const req = https.get(clientOptions, common.mustCall((res) => { - assert.strictEqual(res.statusCode, 200); - res.on('error', (err) => assert.fail(err)); - res.socket.on('error', (err) => assert.fail(err)); - res.resume(); - // drain the socket and wait for it to be free to reuse - res.socket.once('free', () => { - // This is the pain point. Internally the Agent will call - // `socket._handle.asyncReset()` and if the _handle does not implement - // `asyncReset` this will throw TypeError - const req2 = https.get(clientOptions, common.mustCall((res2) => { - assert.strictEqual(res.statusCode, 200); - res2.on('error', (err) => assert.fail(err)); - res2.socket.on('error', (err) => assert.fail(err)); - // this should be the end of the test - res2.destroy(); - server.close(); - })); - req2.on('error', (err) => assert.fail(err)); - }); - })); - req.on('error', (err) => assert.fail(err)); -})); diff --git a/test/parallel/test-async-wrap-tlssocket-asyncreset.js b/test/parallel/test-async-wrap-tlssocket-asyncreset.js new file mode 100644 index 00000000000000..37160741fcd8e7 --- /dev/null +++ b/test/parallel/test-async-wrap-tlssocket-asyncreset.js @@ -0,0 +1,66 @@ +'use strict'; +const common = require('../common'); +if (!common.hasCrypto) common.skip('missing crypto'); +const fixtures = require('../common/fixtures'); + +// An HTTP Agent reuses a TLSSocket, and makes a failed call to `asyncReset`. +// Refs: https://github.com/nodejs/node/issues/13045 + +const assert = require('assert'); +const https = require('https'); + +const serverOptions = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem'), + ca: fixtures.readKey('ca1-cert.pem') +}; + +const server = https.createServer( + serverOptions, + common.mustCall((req, res) => { + res.end('hello world\n'); + }, 2) +); + +server.listen( + 0, + common.mustCall(function() { + const port = this.address().port; + const clientOptions = { + agent: new https.Agent({ + keepAlive: true, + rejectUnauthorized: false + }), + port: port + }; + + const req = https.get( + clientOptions, + common.mustCall((res) => { + assert.strictEqual(res.statusCode, 200); + res.on('error', (err) => assert.fail(err)); + res.socket.on('error', (err) => assert.fail(err)); + res.resume(); + // drain the socket and wait for it to be free to reuse + res.socket.once('free', () => { + // This is the pain point. Internally the Agent will call + // `socket._handle.asyncReset()` and if the _handle does not implement + // `asyncReset` this will throw TypeError + const req2 = https.get( + clientOptions, + common.mustCall((res2) => { + assert.strictEqual(res.statusCode, 200); + res2.on('error', (err) => assert.fail(err)); + res2.socket.on('error', (err) => assert.fail(err)); + // this should be the end of the test + res2.destroy(); + server.close(); + }) + ); + req2.on('error', (err) => assert.fail(err)); + }); + }) + ); + req.on('error', (err) => assert.fail(err)); + }) +); diff --git a/test/parallel/test-child-process-fork-closed-channel-segfault.js b/test/parallel/test-child-process-fork-closed-channel-segfault.js new file mode 100644 index 00000000000000..87b599c7bb29f2 --- /dev/null +++ b/test/parallel/test-child-process-fork-closed-channel-segfault.js @@ -0,0 +1,75 @@ +'use strict'; +const common = require('../common'); + +// Before https://github.com/nodejs/node/pull/2847 a child process trying +// (asynchronously) to use the closed channel to it's creator caused a segfault. + +const assert = require('assert'); +const cluster = require('cluster'); +const net = require('net'); + +if (!cluster.isMaster) { + // Exit on first received handle to leave the queue non-empty in master + process.on('message', function() { + process.exit(1); + }); + return; +} + +const server = net + .createServer(function(s) { + if (common.isWindows) { + s.on('error', function(err) { + // Prevent possible ECONNRESET errors from popping up + if (err.code !== 'ECONNRESET') throw err; + }); + } + setTimeout(function() { + s.destroy(); + }, 100); + }) + .listen(0, function() { + const worker = cluster.fork(); + + function send(callback) { + const s = net.connect(server.address().port, function() { + worker.send({}, s, callback); + }); + + // https://github.com/nodejs/node/issues/3635#issuecomment-157714683 + // ECONNREFUSED or ECONNRESET errors can happen if this connection is + // still establishing while the server has already closed. + // EMFILE can happen if the worker __and__ the server had already closed. + s.on('error', function(err) { + if ( + err.code !== 'ECONNRESET' && + err.code !== 'ECONNREFUSED' && + err.code !== 'EMFILE' + ) { + throw err; + } + }); + } + + worker.process.once( + 'close', + common.mustCall(function() { + // Otherwise the crash on `channel.fd` access may happen + assert.strictEqual(worker.process.channel, null); + server.close(); + }) + ); + + worker.on('online', function() { + send(function(err) { + assert.ifError(err); + send(function(err) { + // Ignore errors when sending the second handle because the worker + // may already have exited. + if (err && err.message !== 'Channel closed') { + throw err; + } + }); + }); + }); + }); diff --git a/test/parallel/test-child-process-fork-regr-gh-2847.js b/test/parallel/test-child-process-fork-regr-gh-2847.js deleted file mode 100644 index 9e4412d1f73738..00000000000000 --- a/test/parallel/test-child-process-fork-regr-gh-2847.js +++ /dev/null @@ -1,69 +0,0 @@ -// Before https://github.com/nodejs/node/pull/2847 a child process trying -// (asynchronously) to use the closed channel to it's creator caused a segfault. -'use strict'; - -const common = require('../common'); -const assert = require('assert'); - -const cluster = require('cluster'); -const net = require('net'); - -if (!cluster.isMaster) { - // Exit on first received handle to leave the queue non-empty in master - process.on('message', function() { - process.exit(1); - }); - return; -} - -const server = net.createServer(function(s) { - if (common.isWindows) { - s.on('error', function(err) { - // Prevent possible ECONNRESET errors from popping up - if (err.code !== 'ECONNRESET') - throw err; - }); - } - setTimeout(function() { - s.destroy(); - }, 100); -}).listen(0, function() { - const worker = cluster.fork(); - - function send(callback) { - const s = net.connect(server.address().port, function() { - worker.send({}, s, callback); - }); - - // https://github.com/nodejs/node/issues/3635#issuecomment-157714683 - // ECONNREFUSED or ECONNRESET errors can happen if this connection is still - // establishing while the server has already closed. - // EMFILE can happen if the worker __and__ the server had already closed. - s.on('error', function(err) { - if ((err.code !== 'ECONNRESET') && - (err.code !== 'ECONNREFUSED') && - (err.code !== 'EMFILE')) { - throw err; - } - }); - } - - worker.process.once('close', common.mustCall(function() { - // Otherwise the crash on `channel.fd` access may happen - assert.strictEqual(worker.process.channel, null); - server.close(); - })); - - worker.on('online', function() { - send(function(err) { - assert.ifError(err); - send(function(err) { - // Ignore errors when sending the second handle because the worker - // may already have exited. - if (err && err.message !== 'Channel closed') { - throw err; - } - }); - }); - }); -}); diff --git a/test/parallel/test-http-pipeline-assertionerror-finish.js b/test/parallel/test-http-pipeline-assertionerror-finish.js new file mode 100644 index 00000000000000..2780d4bdf676ad --- /dev/null +++ b/test/parallel/test-http-pipeline-assertionerror-finish.js @@ -0,0 +1,34 @@ +'use strict'; +const common = require('../common'); + +// This test ensures that Node.js doesn't crash with an AssertionError at +// `ServerResponse.resOnFinish` because of an out-of-order 'finish' bug in +// pipelining. +// https://github.com/nodejs/node/issues/2639 + +const http = require('http'); +const net = require('net'); + +const COUNT = 10; + +const server = http + .createServer( + common.mustCall((req, res) => { + // Close the server, we have only one TCP connection anyway + server.close(); + res.writeHead(200); + res.write('data'); + + setTimeout(function() { + res.end(); + }, (Math.random() * 100) | 0); + }, COUNT) + ) + .listen(0, function() { + const s = net.connect(this.address().port); + + const big = 'GET / HTTP/1.0\r\n\r\n'.repeat(COUNT); + + s.write(big); + s.resume(); + }); diff --git a/test/parallel/test-http-pipeline-regr-2639.js b/test/parallel/test-http-pipeline-regr-2639.js deleted file mode 100644 index 8eaf5588aaf2bd..00000000000000 --- a/test/parallel/test-http-pipeline-regr-2639.js +++ /dev/null @@ -1,24 +0,0 @@ -'use strict'; -const common = require('../common'); -const http = require('http'); -const net = require('net'); - -const COUNT = 10; - -const server = http.createServer(common.mustCall((req, res) => { - // Close the server, we have only one TCP connection anyway - server.close(); - res.writeHead(200); - res.write('data'); - - setTimeout(function() { - res.end(); - }, (Math.random() * 100) | 0); -}, COUNT)).listen(0, function() { - const s = net.connect(this.address().port); - - const big = 'GET / HTTP/1.0\r\n\r\n'.repeat(COUNT); - - s.write(big); - s.resume(); -}); diff --git a/test/parallel/test-http-pipeline-regr-3332.js b/test/parallel/test-http-pipeline-regr-3332.js deleted file mode 100644 index c940b8d3841b52..00000000000000 --- a/test/parallel/test-http-pipeline-regr-3332.js +++ /dev/null @@ -1,27 +0,0 @@ -'use strict'; -require('../common'); -const http = require('http'); -const net = require('net'); -const Countdown = require('../common/countdown'); - -const big = Buffer.alloc(16 * 1024, 'A'); - -const COUNT = 1e4; - -const countdown = new Countdown(COUNT, () => { - server.close(); - client.end(); -}); - -let client; -const server = http.createServer(function(req, res) { - res.end(big, function() { - countdown.dec(); - }); -}).listen(0, function() { - const req = 'GET / HTTP/1.1\r\n\r\n'.repeat(COUNT); - client = net.connect(this.address().port, function() { - client.write(req); - }); - client.resume(); -}); diff --git a/test/parallel/test-http-pipeline-regr-3508.js b/test/parallel/test-http-pipeline-regr-3508.js deleted file mode 100644 index 02c1e1787be6f3..00000000000000 --- a/test/parallel/test-http-pipeline-regr-3508.js +++ /dev/null @@ -1,55 +0,0 @@ -'use strict'; -require('../common'); -const http = require('http'); -const net = require('net'); - -let once = false; -let first = null; -let second = null; - -const chunk = Buffer.alloc(1024, 'X'); - -let size = 0; - -let more; -let done; - -const server = http.createServer(function(req, res) { - if (!once) - server.close(); - once = true; - - if (first === null) { - first = res; - return; - } - if (second === null) { - second = res; - res.write(chunk); - } else { - res.end(chunk); - } - size += res.outputSize; - if (size <= req.socket.writableHighWaterMark) { - more(); - return; - } - done(); -}).on('upgrade', function(req, socket) { - second.end(chunk, function() { - socket.end(); - }); - first.end('hello'); -}).listen(0, function() { - const s = net.connect(this.address().port); - more = function() { - s.write('GET / HTTP/1.1\r\n\r\n'); - }; - done = function() { - s.write('GET / HTTP/1.1\r\n\r\n' + - 'GET / HTTP/1.1\r\nConnection: upgrade\r\nUpgrade: ws\r\n\r\naaa'); - }; - more(); - more(); - s.resume(); -}); diff --git a/test/parallel/test-http-pipeline-requests-connection-leak.js b/test/parallel/test-http-pipeline-requests-connection-leak.js new file mode 100644 index 00000000000000..aa5db56f77862b --- /dev/null +++ b/test/parallel/test-http-pipeline-requests-connection-leak.js @@ -0,0 +1,34 @@ +'use strict'; +require('../common'); +const Countdown = require('../common/countdown'); + +// This test ensures Node.js doesn't behave erratically when receiving pipelined +// requests +// https://github.com/nodejs/node/issues/3332 + +const http = require('http'); +const net = require('net'); + +const big = Buffer.alloc(16 * 1024, 'A'); + +const COUNT = 1e4; + +const countdown = new Countdown(COUNT, () => { + server.close(); + client.end(); +}); + +let client; +const server = http + .createServer(function(req, res) { + res.end(big, function() { + countdown.dec(); + }); + }) + .listen(0, function() { + const req = 'GET / HTTP/1.1\r\n\r\n'.repeat(COUNT); + client = net.connect(this.address().port, function() { + client.write(req); + }); + client.resume(); + }); diff --git a/test/parallel/test-http-pipeline-socket-parser-typeerror.js b/test/parallel/test-http-pipeline-socket-parser-typeerror.js new file mode 100644 index 00000000000000..0cb20e76172766 --- /dev/null +++ b/test/parallel/test-http-pipeline-socket-parser-typeerror.js @@ -0,0 +1,64 @@ +'use strict'; +require('../common'); + +// This test ensures that Node.js doesn't crash because of a TypeError by +// checking in `connectionListener` that the socket still has the parser. +// https://github.com/nodejs/node/issues/3508 + +const http = require('http'); +const net = require('net'); + +let once = false; +let first = null; +let second = null; + +const chunk = Buffer.alloc(1024, 'X'); + +let size = 0; + +let more; +let done; + +const server = http + .createServer(function(req, res) { + if (!once) server.close(); + once = true; + + if (first === null) { + first = res; + return; + } + if (second === null) { + second = res; + res.write(chunk); + } else { + res.end(chunk); + } + size += res.outputSize; + if (size <= req.socket.writableHighWaterMark) { + more(); + return; + } + done(); + }) + .on('upgrade', function(req, socket) { + second.end(chunk, function() { + socket.end(); + }); + first.end('hello'); + }) + .listen(0, function() { + const s = net.connect(this.address().port); + more = function() { + s.write('GET / HTTP/1.1\r\n\r\n'); + }; + done = function() { + s.write( + 'GET / HTTP/1.1\r\n\r\n' + + 'GET / HTTP/1.1\r\nConnection: upgrade\r\nUpgrade: ws\r\n\r\naaa' + ); + }; + more(); + more(); + s.resume(); + }); diff --git a/test/parallel/test-http-regr-gh-2821.js b/test/parallel/test-http-request-large-payload.js similarity index 70% rename from test/parallel/test-http-regr-gh-2821.js rename to test/parallel/test-http-request-large-payload.js index 9f1df0a0f56807..3be100b740417f 100644 --- a/test/parallel/test-http-regr-gh-2821.js +++ b/test/parallel/test-http-request-large-payload.js @@ -1,5 +1,10 @@ 'use strict'; require('../common'); + +// This test ensures Node.js doesn't throw an error when making requests with +// the payload 16kb or more in size. +// https://github.com/nodejs/node/issues/2821 + const http = require('http'); const server = http.createServer(function(req, res) { @@ -10,7 +15,6 @@ const server = http.createServer(function(req, res) { }); server.listen(0, function() { - const req = http.request({ method: 'POST', port: this.address().port