From f6cdde77844bf9e47ddb956e47520454ba7a55dd Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 18 Aug 2015 23:15:25 -0700 Subject: [PATCH 1/4] test: refactor test-https-simple.js This refactoring: * eliminates the need for the external `curl` command * speeds the test by running the two test requests simultaneously * checks the type of error in the test that expects a failure (previously, any error type would cause the test to pass) --- test/parallel/test-https-simple.js | 81 +++++++++++++++++++++++------- 1 file changed, 62 insertions(+), 19 deletions(-) diff --git a/test/parallel/test-https-simple.js b/test/parallel/test-https-simple.js index 2401ad2586bc8a..f93b873e92cb07 100644 --- a/test/parallel/test-https-simple.js +++ b/test/parallel/test-https-simple.js @@ -9,43 +9,86 @@ if (!common.hasCrypto) { var https = require('https'); var fs = require('fs'); -var exec = require('child_process').exec; var options = { key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'), cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem') }; -var reqCount = 0; +var reqReceivedCount = 0; +const tests = 2; +var successful = 0; + +var testSucceeded = function() { + successful = successful + 1; + if (successful === tests) { + server.close(); + } +}; + var body = 'hello world\n'; var server = https.createServer(options, function(req, res) { - reqCount++; - console.log('got request'); + reqReceivedCount++; res.writeHead(200, { 'content-type': 'text/plain' }); res.end(body); }); server.listen(common.PORT, function() { - var cmd = 'curl --insecure https://127.0.0.1:' + common.PORT + '/'; - console.error('executing %j', cmd); - exec(cmd, function(err, stdout, stderr) { - if (err) throw err; - common.error(common.inspect(stdout)); - assert.equal(body, stdout); - - // Do the same thing now without --insecure - // The connection should not be accepted. - var cmd = 'curl https://127.0.0.1:' + common.PORT + '/'; - console.error('executing %j', cmd); - exec(cmd, function(err, stdout, stderr) { - assert.ok(err); - server.close(); + // Do a request ignoring the invalid server certs + var noCertCheckOptions = { + hostname: '127.0.0.1', + port: common.PORT, + path: '/', + method: 'GET', + rejectUnauthorized: false + }; + noCertCheckOptions.Agent = new https.Agent(noCertCheckOptions); + + var req = https.request(noCertCheckOptions, function(res) { + var responseBody = ''; + res.on('data', function(d) { + responseBody = responseBody + d; + }); + + res.on('end', function() { + assert.equal(responseBody, body); + testSucceeded(); + }); + }); + req.end(); + + req.on('error', function(e) { + throw e; + }); + + // Do a request that throws error due to the invalid server certs + var checkCertOptions = { + hostname: '127.0.0.1', + port: common.PORT, + path: '/', + method: 'GET' + }; + + var checkCertReq = https.request(checkCertOptions, function(res) { + res.on('data', function() { + throw Error('data should not be received'); }); + + res.on('end', function() { + throw Error('connection should not be established'); + }); + }); + checkCertReq.end(); + + checkCertReq.on('error', function(e) { + assert.equal(e.code, 'UNABLE_TO_VERIFY_LEAF_SIGNATURE'); + testSucceeded(); }); }); process.on('exit', function() { - assert.equal(1, reqCount); + assert.equal(1, reqReceivedCount); + assert.equal(successful, tests); }); From 9a9b14c470ecb311780f604cd59e10d6a5766d67 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 19 Aug 2015 07:06:46 -0700 Subject: [PATCH 2/4] test: use const/let for consistency Since I'm rewriting most of the test anyway, might as well change the handful of `var` usages to `const` and `let`. --- test/parallel/test-https-simple.js | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/test/parallel/test-https-simple.js b/test/parallel/test-https-simple.js index f93b873e92cb07..75ea7885cba41d 100644 --- a/test/parallel/test-https-simple.js +++ b/test/parallel/test-https-simple.js @@ -1,34 +1,34 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); +const common = require('../common'); if (!common.hasCrypto) { console.log('1..0 # Skipped: missing crypto'); return; } -var https = require('https'); -var fs = require('fs'); +const assert = require('assert'); +const https = require('https'); +const fs = require('fs'); -var options = { +const options = { key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'), cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem') }; -var reqReceivedCount = 0; const tests = 2; -var successful = 0; +let reqReceivedCount = 0; +let successful = 0; -var testSucceeded = function() { +const testSucceeded = function() { successful = successful + 1; if (successful === tests) { server.close(); } }; -var body = 'hello world\n'; +const body = 'hello world\n'; -var server = https.createServer(options, function(req, res) { +const server = https.createServer(options, function(req, res) { reqReceivedCount++; res.writeHead(200, { 'content-type': 'text/plain' }); res.end(body); @@ -37,7 +37,7 @@ var server = https.createServer(options, function(req, res) { server.listen(common.PORT, function() { // Do a request ignoring the invalid server certs - var noCertCheckOptions = { + const noCertCheckOptions = { hostname: '127.0.0.1', port: common.PORT, path: '/', @@ -46,8 +46,8 @@ server.listen(common.PORT, function() { }; noCertCheckOptions.Agent = new https.Agent(noCertCheckOptions); - var req = https.request(noCertCheckOptions, function(res) { - var responseBody = ''; + const req = https.request(noCertCheckOptions, function(res) { + let responseBody = ''; res.on('data', function(d) { responseBody = responseBody + d; }); @@ -64,14 +64,14 @@ server.listen(common.PORT, function() { }); // Do a request that throws error due to the invalid server certs - var checkCertOptions = { + const checkCertOptions = { hostname: '127.0.0.1', port: common.PORT, path: '/', method: 'GET' }; - var checkCertReq = https.request(checkCertOptions, function(res) { + const checkCertReq = https.request(checkCertOptions, function(res) { res.on('data', function() { throw Error('data should not be received'); }); From 079257f5fa0a4fe6d106fc27061e5dec90530cb8 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 23 Aug 2015 19:57:44 -0700 Subject: [PATCH 3/4] test: style/wording changes --- test/parallel/test-https-simple.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-https-simple.js b/test/parallel/test-https-simple.js index 75ea7885cba41d..c3affc3dd8a23b 100644 --- a/test/parallel/test-https-simple.js +++ b/test/parallel/test-https-simple.js @@ -36,7 +36,7 @@ const server = https.createServer(options, function(req, res) { server.listen(common.PORT, function() { - // Do a request ignoring the invalid server certs + // Do a request ignoring the unauthorized server certs const noCertCheckOptions = { hostname: '127.0.0.1', port: common.PORT, @@ -73,11 +73,11 @@ server.listen(common.PORT, function() { const checkCertReq = https.request(checkCertOptions, function(res) { res.on('data', function() { - throw Error('data should not be received'); + throw new Error('data should not be received'); }); res.on('end', function() { - throw Error('connection should not be established'); + throw new Error('connection should not be established'); }); }); checkCertReq.end(); From a8cb028871127f66a2dce952f7c532f9e65b3077 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 23 Aug 2015 21:03:47 -0700 Subject: [PATCH 4/4] test: use common.mustCall() --- test/parallel/test-https-simple.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-https-simple.js b/test/parallel/test-https-simple.js index c3affc3dd8a23b..92f9bde2ef802a 100644 --- a/test/parallel/test-https-simple.js +++ b/test/parallel/test-https-simple.js @@ -16,7 +16,6 @@ const options = { }; const tests = 2; -let reqReceivedCount = 0; let successful = 0; const testSucceeded = function() { @@ -28,12 +27,12 @@ const testSucceeded = function() { const body = 'hello world\n'; -const server = https.createServer(options, function(req, res) { - reqReceivedCount++; +const serverCallback = common.mustCall(function(req, res) { res.writeHead(200, { 'content-type': 'text/plain' }); res.end(body); }); +const server = https.createServer(options, serverCallback); server.listen(common.PORT, function() { // Do a request ignoring the unauthorized server certs @@ -89,6 +88,5 @@ server.listen(common.PORT, function() { }); process.on('exit', function() { - assert.equal(1, reqReceivedCount); assert.equal(successful, tests); });