From 96159107a6cefbb99a52d2d7151ec4a305b830f9 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 00:49:13 +0200 Subject: [PATCH 01/84] doc: var => let / const in repl.md --- doc/api/repl.md | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index b78a13544add4a..ae836e7a135fbf 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -78,15 +78,16 @@ The default evaluator supports direct evaluation of JavaScript expressions: ```js > 1 + 1 2 -> var m = 2 +> const m = 2 undefined > m + 1 3 ``` Unless otherwise scoped within blocks (e.g. `{ ... }`) or functions, variables -declared either implicitly or using the `var` keyword are declared at the -`global` scope. +declared either implicitly or using the `const` or `let` keywords +are declared at the `global` scope (as well as with the `var` keyword +outside of functions). #### Global and Local Scope @@ -96,7 +97,7 @@ it to the `context` object associated with each `REPLServer`. For example: ```js const repl = require('repl'); -var msg = 'message'; +const msg = 'message'; repl.start('> ').context.m = msg; ``` @@ -115,7 +116,7 @@ To specify read-only globals, context properties must be defined using ```js const repl = require('repl'); -var msg = 'message'; +const msg = 'message'; const r = repl.start('> '); Object.defineProperty(r.context, 'm', { @@ -183,7 +184,7 @@ to the provided callback function: ```js function eval(cmd, context, filename, callback) { - var result; + let result; try { result = vm.runInThisContext(cmd); } catch (e) { @@ -275,7 +276,7 @@ function initializeContext(context) { context.m = 'test'; } -var r = repl.start({prompt: '>'}); +const r = repl.start({prompt: '>'}); initializeContext(r.context); r.on('reset', initializeContext); @@ -321,7 +322,7 @@ The following example shows two new commands added to the REPL instance: ```js const repl = require('repl'); -var replServer = repl.start({prompt: '> '}); +const replServer = repl.start({prompt: '> '}); replServer.defineCommand('sayhello', { help: 'Say hello', action: function(name) { @@ -421,7 +422,7 @@ without passing any arguments (or by passing the `-i` argument): ```js $ node -> a = [1, 2, 3]; +> const a = [1, 2, 3]; [ 1, 2, 3 ] > a.forEach((v) => { ... console.log(v); @@ -493,7 +494,7 @@ socket, and a TCP socket: ```js const net = require('net'); const repl = require('repl'); -var connections = 0; +let connections = 0; repl.start({ prompt: 'Node.js via stdin> ', From 3e18789635a8e96c11feaa96e2dd79160a29d1bb Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:07:03 +0200 Subject: [PATCH 02/84] doc: white space unification in repl.md Add an infix space in an argument list. Change `>` into `> ` in code bits and output examples. Explicitly clarify that default REPL prompt contains a trailing space. --- doc/api/repl.md | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index ae836e7a135fbf..0c0a8f61d92c85 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -218,10 +218,10 @@ following example, for instance, simply converts any input text to upper case: ```js const repl = require('repl'); -const r = repl.start({prompt: '>', eval: myEval, writer: myWriter}); +const r = repl.start({prompt: '> ', eval: myEval, writer: myWriter}); function myEval(cmd, context, filename, callback) { - callback(null,cmd); + callback(null, cmd); } function myWriter(output) { @@ -276,7 +276,7 @@ function initializeContext(context) { context.m = 'test'; } -const r = repl.start({prompt: '>'}); +const r = repl.start({prompt: '> '}); initializeContext(r.context); r.on('reset', initializeContext); @@ -287,15 +287,15 @@ reset to its initial value using the `.clear` command: ```js $ ./node example.js ->m +> m 'test' ->m = 1 +> m = 1 1 ->m +> m 1 ->.clear +> .clear Clearing context... ->m +> m 'test' > ``` @@ -373,7 +373,8 @@ added: v0.1.91 --> * `options` {Object} - * `prompt` {String} The input prompt to display. Defaults to `> `. + * `prompt` {String} The input prompt to display. Defaults to `> ` + (with a trailing space). * `input` {Readable} The Readable stream from which REPL input will be read. Defaults to `process.stdin`. * `output` {Writable} The Writable stream to which REPL output will be From 743f84db09bca9221fe5941d406185e25de97e80 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:15:04 +0200 Subject: [PATCH 03/84] doc: fix an output example in repl.md Make `_` reassignment example match more with the current output. Extend the example for more clarity. --- doc/api/repl.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 0c0a8f61d92c85..f78a409b3b25fa 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -141,6 +141,7 @@ global or scoped variable, the input `fs` will be evaluated on-demand as The default evaluator will, by default, assign the result of the most recently evaluated expression to the special variable `_` (underscore). +Explicitly setting `_` to a value will disable this behavior. ```js > [ 'a', 'b', 'c' ] @@ -148,11 +149,14 @@ evaluated expression to the special variable `_` (underscore). > _.length 3 > _ += 1 +Expression assignment to _ now disabled. +4 +> 1 + 1 +2 +> _ 4 ``` -Explicitly setting `_` to a value will disable this behavior. - ### Custom Evaluation Functions When a new `repl.REPLServer` is created, a custom evaluation function may be From a7501f25bb5ddc4d719b3e642315f71fb4e9ef1d Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:20:35 +0200 Subject: [PATCH 04/84] doc: fix a function name in repl.md `eval` => `myEval` to not shadow the global `eval` --- doc/api/repl.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index f78a409b3b25fa..dac6ff5f034576 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -187,7 +187,7 @@ multi-line input, the eval function can return an instance of `repl.Recoverable` to the provided callback function: ```js -function eval(cmd, context, filename, callback) { +function myEval(cmd, context, filename, callback) { let result; try { result = vm.runInThisContext(cmd); From 402c62544bbdffe3e30e8db3a75d841c443a8deb Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:22:35 +0200 Subject: [PATCH 05/84] doc: name anonymous functions in repl.md --- doc/api/repl.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index dac6ff5f034576..20fc74832ba94d 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -329,14 +329,14 @@ const repl = require('repl'); const replServer = repl.start({prompt: '> '}); replServer.defineCommand('sayhello', { help: 'Say hello', - action: function(name) { + action: function sayHello(name) { this.lineParser.reset(); this.bufferedCommand = ''; console.log(`Hello, ${name}!`); this.displayPrompt(); } }); -replServer.defineCommand('saybye', function() { +replServer.defineCommand('saybye', function sayBye() { console.log('Goodbye!'); this.close(); }); From e20c78513343830e0068b23229ff8f0f94d2f302 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:27:47 +0200 Subject: [PATCH 06/84] doc: document argument variant in repl.md `options` in the `repl.start([options])` can be a string. --- doc/api/repl.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 20fc74832ba94d..ba4defc6402d08 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -376,7 +376,7 @@ within the action function for commands registered using the added: v0.1.91 --> -* `options` {Object} +* `options` {Object | String} * `prompt` {String} The input prompt to display. Defaults to `> ` (with a trailing space). * `input` {Readable} The Readable stream from which REPL input will be read. @@ -417,6 +417,8 @@ added: v0.1.91 `SIGINT` is received, i.e. `Ctrl+C` is pressed. This cannot be used together with a custom `eval` function. Defaults to `false`. +If `options` is a string, then it specifies the input prompt. + The `repl.start()` method creates and starts a `repl.REPLServer` instance. ## The Node.js REPL From 435705935978c1a9fbd89c5a466a287f4c81fef1 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:31:05 +0200 Subject: [PATCH 07/84] doc: add the valid link for curl(1) in repl.md The current autoinserted link leads to 404 page. --- doc/api/repl.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index ba4defc6402d08..4bc532834d41d9 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -543,10 +543,11 @@ possible to connect to a long-running Node.js process without restarting it. For an example of running a "full-featured" (`terminal`) REPL over a `net.Server` and `net.Socket` instance, see: https://gist.github.com/2209310 -For an example of running a REPL instance over curl(1), +For an example of running a REPL instance over [curl(1)][], see: https://gist.github.com/2053342 [stream]: stream.html [`util.inspect()`]: util.html#util_util_inspect_object_options [`readline.Interface`]: readline.html#readline_class_interface [`readline.InterfaceCompleter`]: readline.html#readline_use_of_the_completer_function +[curl(1)]: https://curl.haxx.se/docs/manpage.html From 9c7054b2e189faea8a28c01ffd76834839a171fc Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 03:35:17 +0200 Subject: [PATCH 08/84] doc: replace anonymous functions in repl.md Replaced with an object shorthand and an arrow function. --- doc/api/repl.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 4bc532834d41d9..a56a731bbfe946 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -329,14 +329,14 @@ const repl = require('repl'); const replServer = repl.start({prompt: '> '}); replServer.defineCommand('sayhello', { help: 'Say hello', - action: function sayHello(name) { + action(name) { this.lineParser.reset(); this.bufferedCommand = ''; console.log(`Hello, ${name}!`); this.displayPrompt(); } }); -replServer.defineCommand('saybye', function sayBye() { +replServer.defineCommand('saybye', () => { console.log('Goodbye!'); this.close(); }); From cdeb85efc2cc6ab604830af300a5d0cfa48432fa Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 6 Dec 2016 21:11:29 -0800 Subject: [PATCH 09/84] test: move long-running test to sequential test-buffer-creation-regression is flaky on some SmartOS hosts in CI, timing out. Move to sequential so it does not compete with other tests for resources. Reduce three test cases to just the one needed to identify the regression. PR-URL: https://github.com/nodejs/node/pull/10161 Reviewed-By: Santiago Gimeno Reviewed-By: Colin Ihrig Reviewed-By: Italo A. Casas --- .../test-buffer-creation-regression.js | 41 ------------------- .../test-buffer-creation-regression.js | 36 ++++++++++++++++ 2 files changed, 36 insertions(+), 41 deletions(-) delete mode 100644 test/parallel/test-buffer-creation-regression.js create mode 100644 test/sequential/test-buffer-creation-regression.js diff --git a/test/parallel/test-buffer-creation-regression.js b/test/parallel/test-buffer-creation-regression.js deleted file mode 100644 index 650fbf48f12ae6..00000000000000 --- a/test/parallel/test-buffer-creation-regression.js +++ /dev/null @@ -1,41 +0,0 @@ -'use strict'; - -const common = require('../common'); -const assert = require('assert'); - -function test(arrayBuffer, offset, length) { - const uint8Array = new Uint8Array(arrayBuffer, offset, length); - for (let i = 0; i < length; i += 1) { - uint8Array[i] = 1; - } - - const buffer = Buffer.from(arrayBuffer, offset, length); - for (let i = 0; i < length; i += 1) { - assert.strictEqual(buffer[i], 1); - } -} - -const acceptableOOMErrors = [ - 'Array buffer allocation failed', - 'Invalid array buffer length' -]; - -const testCases = [ - [200, 50, 100], - [4294967296 /* 1 << 32 */, 2147483648 /* 1 << 31 */, 1000], - [8589934592 /* 1 << 33 */, 4294967296 /* 1 << 32 */, 1000] -]; - -for (let index = 0, arrayBuffer; index < testCases.length; index += 1) { - const [size, offset, length] = testCases[index]; - - try { - arrayBuffer = new ArrayBuffer(size); - } catch (e) { - if (e instanceof RangeError && acceptableOOMErrors.includes(e.message)) - return common.skip(`Unable to allocate ${size} bytes for ArrayBuffer`); - throw e; - } - - test(arrayBuffer, offset, length); -} diff --git a/test/sequential/test-buffer-creation-regression.js b/test/sequential/test-buffer-creation-regression.js new file mode 100644 index 00000000000000..b5c8450e036cc8 --- /dev/null +++ b/test/sequential/test-buffer-creation-regression.js @@ -0,0 +1,36 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); + +function test(arrayBuffer, offset, length) { + const uint8Array = new Uint8Array(arrayBuffer, offset, length); + for (let i = 0; i < length; i += 1) { + uint8Array[i] = 1; + } + + const buffer = Buffer.from(arrayBuffer, offset, length); + for (let i = 0; i < length; i += 1) { + assert.strictEqual(buffer[i], 1); + } +} + +const acceptableOOMErrors = [ + 'Array buffer allocation failed', + 'Invalid array buffer length' +]; + +const size = 8589934592; /* 1 << 33 */ +const offset = 4294967296; /* 1 << 32 */ +const length = 1000; +let arrayBuffer; + +try { + arrayBuffer = new ArrayBuffer(size); +} catch (e) { + if (e instanceof RangeError && acceptableOOMErrors.includes(e.message)) + return common.skip(`Unable to allocate ${size} bytes for ArrayBuffer`); + throw e; +} + +test(arrayBuffer, offset, length); From da077ee58adda1c9b075401e4e0745c9b97e609f Mon Sep 17 00:00:00 2001 From: Michael-Bryant Choa Date: Thu, 1 Dec 2016 10:39:59 -0600 Subject: [PATCH 10/84] test: refactor test-dgram-bind-default-address - changes var to const/let - changes assert.equal to assert.strictEqual PR-URL: https://github.com/nodejs/node/pull/9947 Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- test/parallel/test-dgram-bind-default-address.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) mode change 100644 => 100755 test/parallel/test-dgram-bind-default-address.js diff --git a/test/parallel/test-dgram-bind-default-address.js b/test/parallel/test-dgram-bind-default-address.js old mode 100644 new mode 100755 index b2bd72f6db6a7c..142a5134c013c3 --- a/test/parallel/test-dgram-bind-default-address.js +++ b/test/parallel/test-dgram-bind-default-address.js @@ -1,7 +1,7 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); -var dgram = require('dgram'); +const common = require('../common'); +const assert = require('assert'); +const dgram = require('dgram'); // skip test in FreeBSD jails since 0.0.0.0 will resolve to default interface if (common.inFreeBSDJail) { @@ -13,7 +13,7 @@ dgram.createSocket('udp4').bind(0, common.mustCall(function() { assert.strictEqual(typeof this.address().port, 'number'); assert.ok(isFinite(this.address().port)); assert.ok(this.address().port > 0); - assert.equal(this.address().address, '0.0.0.0'); + assert.strictEqual(this.address().address, '0.0.0.0'); this.close(); })); @@ -26,9 +26,9 @@ dgram.createSocket('udp6').bind(0, common.mustCall(function() { assert.strictEqual(typeof this.address().port, 'number'); assert.ok(isFinite(this.address().port)); assert.ok(this.address().port > 0); - var address = this.address().address; + let address = this.address().address; if (address === '::ffff:0.0.0.0') address = '::'; - assert.equal(address, '::'); + assert.strictEqual(address, '::'); this.close(); })); From 22e2c47f425e9553ca2bd5b4e78f874d54ed9fd1 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Tue, 6 Dec 2016 19:28:47 +0100 Subject: [PATCH 11/84] build: fix node_g target Currently when running make node_g the following error is displayed: if [ ! -r node -o ! -L ]; then ln -fs out/Debug/node node_g; fi /bin/sh: line 0: [: argument expected It looks like there was a typo for the NODE_EXE where node became lowercase instead of uppercase. Ref: https://github.com/nodejs/node/pull/9827 PR-URL: https://github.com/nodejs/node/pull/10153 Reviewed-By: Jeremiah Senkpiel Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Italo A. Casas --- Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 5e2388d5fe4eec..d17930776f3d67 100644 --- a/Makefile +++ b/Makefile @@ -71,11 +71,11 @@ endif # See comments on the build-addons target for some more info $(NODE_EXE): config.gypi out/Makefile $(MAKE) -C out BUILDTYPE=Release V=$(V) - if [ ! -r $(NODE_EXE) -o ! -L $(NODE_EXE) ]; then ln -fs out/Release/$(NODE_EXE) $@; fi + if [ ! -r $@ -o ! -L $@ ]; then ln -fs out/Release/$(NODE_EXE) $@; fi $(NODE_G_EXE): config.gypi out/Makefile $(MAKE) -C out BUILDTYPE=Debug V=$(V) - if [ ! -r $(NODE_EXE) -o ! -L $(node_EXE) ]; then ln -fs out/Debug/$(NODE_EXE) $@; fi + if [ ! -r $@ -o ! -L $@ ]; then ln -fs out/Debug/$(NODE_EXE) $@; fi out/Makefile: common.gypi deps/uv/uv.gyp deps/http_parser/http_parser.gyp \ deps/zlib/zlib.gyp deps/v8/gypfiles/toolchain.gypi \ From a2441476cc3a8f15b59fd52930039ea0ac0623c1 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 5 Dec 2016 17:13:21 +0100 Subject: [PATCH 12/84] test: check result of uv_loop_init and uv_write Silence coverity warnings about the return value not being checked. PR-URL: https://github.com/nodejs/node/pull/10126 Reviewed-By: Colin Ihrig Reviewed-By: Eugene Ostroukhov Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- test/cctest/test_inspector_socket.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/cctest/test_inspector_socket.cc b/test/cctest/test_inspector_socket.cc index c7a282b75b4b8d..ada3df3d438ce8 100644 --- a/test/cctest/test_inspector_socket.cc +++ b/test/cctest/test_inspector_socket.cc @@ -121,8 +121,9 @@ static void do_write(const char* data, int len) { uv_buf_t buf[1]; buf[0].base = const_cast(data); buf[0].len = len; - uv_write(&req, reinterpret_cast(&client_socket), buf, 1, - write_done); + GTEST_ASSERT_EQ(0, + uv_write(&req, reinterpret_cast(&client_socket), + buf, 1, write_done)); SPIN_WHILE(req.data); } @@ -360,7 +361,7 @@ class InspectorSocketTest : public ::testing::Test { connected = false; inspector_ready = false; last_event = kInspectorHandshakeHttpGet; - uv_loop_init(&loop); + GTEST_ASSERT_EQ(0, uv_loop_init(&loop)); server = uv_tcp_t(); client_socket = uv_tcp_t(); server.data = &inspector; From 1b25214001d7f7e640b673bb20419c795939023b Mon Sep 17 00:00:00 2001 From: CodeVana Date: Thu, 1 Dec 2016 08:09:01 -0800 Subject: [PATCH 13/84] test: use const and strictEqual in test-os-homedir-no-envvar PR-URL: https://github.com/nodejs/node/pull/9899 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- test/parallel/test-os-homedir-no-envvar.js | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/parallel/test-os-homedir-no-envvar.js b/test/parallel/test-os-homedir-no-envvar.js index cb4be4e6efcd4e..94d8ab5a08fa14 100644 --- a/test/parallel/test-os-homedir-no-envvar.js +++ b/test/parallel/test-os-homedir-no-envvar.js @@ -1,28 +1,28 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); -var cp = require('child_process'); -var os = require('os'); -var path = require('path'); +const common = require('../common'); +const assert = require('assert'); +const cp = require('child_process'); +const os = require('os'); +const path = require('path'); if (process.argv[2] === 'child') { if (common.isWindows) - assert.equal(process.env.USERPROFILE, undefined); + assert.strictEqual(process.env.USERPROFILE, undefined); else - assert.equal(process.env.HOME, undefined); + assert.strictEqual(process.env.HOME, undefined); - var home = os.homedir(); + const home = os.homedir(); - assert.ok(typeof home === 'string'); - assert.ok(home.indexOf(path.sep) !== -1); + assert.strictEqual(typeof home, 'string'); + assert(home.includes(path.sep)); } else { if (common.isWindows) delete process.env.USERPROFILE; else delete process.env.HOME; - var child = cp.spawnSync(process.execPath, [__filename, 'child'], { + const child = cp.spawnSync(process.execPath, [__filename, 'child'], { env: process.env }); From 61d6293033782a2abfbb376c38d5d7c600be3e8d Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Sat, 5 Nov 2016 17:37:24 -0700 Subject: [PATCH 14/84] url: improve URLSearchParams spec compliance - Make URLSearchParams constructor spec-compliant - Strip leading `?` in URL#search's setter - Spec-compliant iterable interface - More precise handling of update steps as mandated by the spec - Add class strings to URLSearchParams objects and their prototype - Make sure `this instanceof URLSearchParams` in methods Also included are relevant tests from W3C's Web Platform Tests (https://github.com/w3c/web-platform-tests/tree/master/url). Fixes: https://github.com/nodejs/node/issues/9302 PR-URL: https://github.com/nodejs/node/pull/9484 Reviewed-By: James M Snell --- lib/internal/url.js | 283 +++++++++++++++--- .../test-whatwg-url-searchparams-append.js | 52 ++++ ...est-whatwg-url-searchparams-constructor.js | 134 +++++++++ .../test-whatwg-url-searchparams-delete.js | 44 +++ .../test-whatwg-url-searchparams-foreach.js | 43 +++ .../test-whatwg-url-searchparams-get.js | 35 +++ .../test-whatwg-url-searchparams-getall.js | 43 +++ .../test-whatwg-url-searchparams-has.js | 39 +++ .../test-whatwg-url-searchparams-set.js | 38 +++ ...est-whatwg-url-searchparams-stringifier.js | 116 +++++++ test/parallel/test-whatwg-url-searchparams.js | 13 + 11 files changed, 797 insertions(+), 43 deletions(-) create mode 100644 test/parallel/test-whatwg-url-searchparams-append.js create mode 100644 test/parallel/test-whatwg-url-searchparams-constructor.js create mode 100644 test/parallel/test-whatwg-url-searchparams-delete.js create mode 100644 test/parallel/test-whatwg-url-searchparams-foreach.js create mode 100644 test/parallel/test-whatwg-url-searchparams-get.js create mode 100644 test/parallel/test-whatwg-url-searchparams-getall.js create mode 100644 test/parallel/test-whatwg-url-searchparams-has.js create mode 100644 test/parallel/test-whatwg-url-searchparams-set.js create mode 100644 test/parallel/test-whatwg-url-searchparams-stringifier.js diff --git a/lib/internal/url.js b/lib/internal/url.js index e19b546a1b237e..1129060b09f419 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -20,6 +20,11 @@ const kHost = Symbol('host'); const kPort = Symbol('port'); const kDomain = Symbol('domain'); +// https://tc39.github.io/ecma262/#sec-%iteratorprototype%-object +const IteratorPrototype = Object.getPrototypeOf( + Object.getPrototypeOf([][Symbol.iterator]()) +); + function StorageObject() {} StorageObject.prototype = Object.create(null); @@ -101,7 +106,8 @@ class URL { this[context].query = query; this[context].fragment = fragment; this[context].host = host; - this[searchParams] = new URLSearchParams(this); + this[searchParams] = new URLSearchParams(query); + this[searchParams][context] = this; }); } @@ -318,8 +324,31 @@ class URL { } set search(search) { - update(this, search); - this[searchParams][searchParams] = querystring.parse(this.search); + search = String(search); + if (search[0] === '?') search = search.slice(1); + if (!search) { + this[context].query = null; + this[context].flags &= ~binding.URL_FLAGS_HAS_QUERY; + this[searchParams][searchParams] = {}; + return; + } + this[context].query = ''; + binding.parse(search, + binding.kQuery, + null, + this[context], + (flags, protocol, username, password, + host, port, path, query, fragment) => { + if (flags & binding.URL_FLAGS_FAILED) + return; + if (query) { + this[context].query = query; + this[context].flags |= binding.URL_FLAGS_HAS_QUERY; + } else { + this[context].flags &= ~binding.URL_FLAGS_HAS_QUERY; + } + }); + this[searchParams][searchParams] = querystring.parse(search); } get hash() { @@ -493,76 +522,129 @@ function encodeAuth(str) { return out; } -function update(url, search) { - search = String(search); - if (!search) { - url[context].query = null; - url[context].flags &= ~binding.URL_FLAGS_HAS_QUERY; +function update(url, params) { + if (!url) return; + + url[context].query = params.toString(); +} + +function getSearchParamPairs(target) { + const obj = target[searchParams]; + const keys = Object.keys(obj); + const values = []; + for (var i = 0; i < keys.length; i++) { + const name = keys[i]; + const value = obj[name]; + if (Array.isArray(value)) { + for (const item of value) + values.push([name, item]); + } else { + values.push([name, value]); + } } - if (search[0] === '?') search = search.slice(1); - url[context].query = ''; - binding.parse(search, - binding.kQuery, - null, - url[context], - (flags, protocol, username, password, - host, port, path, query, fragment) => { - if (flags & binding.URL_FLAGS_FAILED) - return; - if (query) { - url[context].query = query; - url[context].flags |= binding.URL_FLAGS_HAS_QUERY; - } else { - url[context].flags &= ~binding.URL_FLAGS_HAS_QUERY; - } - }); + return values; } class URLSearchParams { - constructor(url) { - this[context] = url; - this[searchParams] = querystring.parse(url[context].search || ''); + constructor(init = '') { + if (init instanceof URLSearchParams) { + const childParams = init[searchParams]; + this[searchParams] = Object.assign(Object.create(null), childParams); + } else { + init = String(init); + if (init[0] === '?') init = init.slice(1); + this[searchParams] = querystring.parse(init); + } + + // "associated url object" + this[context] = null; + + // Class string for an instance of URLSearchParams. This is different from + // the class string of the prototype object (set below). + Object.defineProperty(this, Symbol.toStringTag, { + value: 'URLSearchParams', + writable: false, + enumerable: false, + configurable: true + }); } append(name, value) { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); + } + if (arguments.length < 2) { + throw new TypeError( + 'Both `name` and `value` arguments need to be specified'); + } + const obj = this[searchParams]; name = String(name); value = String(value); var existing = obj[name]; - if (!existing) { + if (existing === undefined) { obj[name] = value; } else if (Array.isArray(existing)) { existing.push(value); } else { obj[name] = [existing, value]; } - update(this[context], querystring.stringify(obj)); + update(this[context], this); } delete(name) { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); + } + if (arguments.length < 1) { + throw new TypeError('The `name` argument needs to be specified'); + } + const obj = this[searchParams]; name = String(name); delete obj[name]; - update(this[context], querystring.stringify(obj)); + update(this[context], this); } set(name, value) { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); + } + if (arguments.length < 2) { + throw new TypeError( + 'Both `name` and `value` arguments need to be specified'); + } + const obj = this[searchParams]; name = String(name); value = String(value); obj[name] = value; - update(this[context], querystring.stringify(obj)); + update(this[context], this); } get(name) { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); + } + if (arguments.length < 1) { + throw new TypeError('The `name` argument needs to be specified'); + } + const obj = this[searchParams]; name = String(name); var value = obj[name]; - return Array.isArray(value) ? value[0] : value; + return value === undefined ? null : Array.isArray(value) ? value[0] : value; } getAll(name) { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); + } + if (arguments.length < 1) { + throw new TypeError('The `name` argument needs to be specified'); + } + const obj = this[searchParams]; name = String(name); var value = obj[name]; @@ -570,28 +652,143 @@ class URLSearchParams { } has(name) { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); + } + if (arguments.length < 1) { + throw new TypeError('The `name` argument needs to be specified'); + } + const obj = this[searchParams]; name = String(name); return name in obj; } - *[Symbol.iterator]() { - const obj = this[searchParams]; - for (const name in obj) { - const value = obj[name]; - if (Array.isArray(value)) { - for (const item of value) - yield [name, item]; - } else { - yield [name, value]; - } + // https://heycam.github.io/webidl/#es-iterators + // Define entries here rather than [Symbol.iterator] as the function name + // must be set to `entries`. + entries() { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); } + + return createSearchParamsIterator(this, 'key+value'); } + forEach(callback, thisArg = undefined) { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); + } + if (arguments.length < 1) { + throw new TypeError('The `callback` argument needs to be specified'); + } + + let pairs = getSearchParamPairs(this); + + var i = 0; + while (i < pairs.length) { + const [key, value] = pairs[i]; + callback.call(thisArg, value, key, this); + pairs = getSearchParamPairs(this); + i++; + } + } + + // https://heycam.github.io/webidl/#es-iterable + keys() { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); + } + + return createSearchParamsIterator(this, 'key'); + } + + values() { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); + } + + return createSearchParamsIterator(this, 'value'); + } + + // https://url.spec.whatwg.org/#urlsearchparams-stringification-behavior toString() { + if (!this || !(this instanceof URLSearchParams)) { + throw new TypeError('Value of `this` is not a URLSearchParams'); + } + return querystring.stringify(this[searchParams]); } } +// https://heycam.github.io/webidl/#es-iterable-entries +URLSearchParams.prototype[Symbol.iterator] = URLSearchParams.prototype.entries; +Object.defineProperty(URLSearchParams.prototype, Symbol.toStringTag, { + value: 'URLSearchParamsPrototype', + writable: false, + enumerable: false, + configurable: true +}); + +// https://heycam.github.io/webidl/#dfn-default-iterator-object +function createSearchParamsIterator(target, kind) { + const iterator = Object.create(URLSearchParamsIteratorPrototype); + iterator[context] = { + target, + kind, + index: 0 + }; + return iterator; +} + +// https://heycam.github.io/webidl/#dfn-iterator-prototype-object +const URLSearchParamsIteratorPrototype = Object.setPrototypeOf({ + next() { + if (!this || + Object.getPrototypeOf(this) !== URLSearchParamsIteratorPrototype) { + throw new TypeError('Value of `this` is not a URLSearchParamsIterator'); + } + + const { + target, + kind, + index + } = this[context]; + const values = getSearchParamPairs(target); + const len = values.length; + if (index >= len) { + return { + value: undefined, + done: true + }; + } + + const pair = values[index]; + this[context].index = index + 1; + + let result; + if (kind === 'key') { + result = pair[0]; + } else if (kind === 'value') { + result = pair[1]; + } else { + result = pair; + } + + return { + value: result, + done: false + }; + } +}, IteratorPrototype); + +// Unlike interface and its prototype object, both default iterator object and +// iterator prototype object of an interface have the same class string. +Object.defineProperty(URLSearchParamsIteratorPrototype, Symbol.toStringTag, { + value: 'URLSearchParamsIterator', + writable: false, + enumerable: false, + configurable: true +}); URL.originFor = function(url) { if (!(url instanceof URL)) diff --git a/test/parallel/test-whatwg-url-searchparams-append.js b/test/parallel/test-whatwg-url-searchparams-append.js new file mode 100644 index 00000000000000..1f61cb0d11bb6b --- /dev/null +++ b/test/parallel/test-whatwg-url-searchparams-append.js @@ -0,0 +1,52 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const URL = require('url').URL; + +const m = new URL('http://example.org'); +let params = m.searchParams; + +// Until we export URLSearchParams +const URLSearchParams = params.constructor; + +// Append same name +params = new URLSearchParams(); +params.append('a', 'b'); +assert.strictEqual(params + '', 'a=b'); +params.append('a', 'b'); +assert.strictEqual(params + '', 'a=b&a=b'); +params.append('a', 'c'); +assert.strictEqual(params + '', 'a=b&a=b&a=c'); + +// Append empty strings +params = new URLSearchParams(); +params.append('', ''); +assert.strictEqual(params + '', '='); +params.append('', ''); +assert.strictEqual(params + '', '=&='); + +// Append null +params = new URLSearchParams(); +params.append(null, null); +assert.strictEqual(params + '', 'null=null'); +params.append(null, null); +assert.strictEqual(params + '', 'null=null&null=null'); + +// Append multiple +params = new URLSearchParams(); +params.append('first', 1); +params.append('second', 2); +params.append('third', ''); +params.append('first', 10); +assert.strictEqual(true, params.has('first'), + 'Search params object has name "first"'); +assert.strictEqual(params.get('first'), '1', + 'Search params object has name "first" with value "1"'); +assert.strictEqual(params.get('second'), '2', + 'Search params object has name "second" with value "2"'); +assert.strictEqual(params.get('third'), '', + 'Search params object has name "third" with value ""'); +params.append('first', 10); +assert.strictEqual(params.get('first'), '1', + 'Search params object has name "first" with value "1"'); diff --git a/test/parallel/test-whatwg-url-searchparams-constructor.js b/test/parallel/test-whatwg-url-searchparams-constructor.js new file mode 100644 index 00000000000000..98349021586f1a --- /dev/null +++ b/test/parallel/test-whatwg-url-searchparams-constructor.js @@ -0,0 +1,134 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const URL = require('url').URL; + +const m = new URL('http://example.org'); +let params = m.searchParams; + +// Until we export URLSearchParams +const URLSearchParams = params.constructor; + +// Basic URLSearchParams construction +params = new URLSearchParams(); +assert.strictEqual(params + '', ''); +params = new URLSearchParams(''); +assert.strictEqual(params + '', ''); +params = new URLSearchParams('a=b'); +assert.strictEqual(params + '', 'a=b'); +params = new URLSearchParams(params); +assert.strictEqual(params + '', 'a=b'); + +// URLSearchParams constructor, empty. +assert.throws(() => URLSearchParams(), TypeError, + 'Calling \'URLSearchParams\' without \'new\' should throw.'); +// assert.throws(() => new URLSearchParams(DOMException.prototype), TypeError); +assert.throws(() => { + new URLSearchParams({ + toString() { throw new TypeError('Illegal invocation'); } + }); +}, TypeError); +params = new URLSearchParams(''); +assert.notEqual(params, null, 'constructor returned non-null value.'); +// eslint-disable-next-line no-proto +assert.strictEqual(params.__proto__, URLSearchParams.prototype, + 'expected URLSearchParams.prototype as prototype.'); +params = new URLSearchParams({}); +// assert.strictEqual(params + '', '%5Bobject+Object%5D='); +assert.strictEqual(params + '', '%5Bobject%20Object%5D='); + +// URLSearchParams constructor, string. +params = new URLSearchParams('a=b'); +assert.notEqual(params, null, 'constructor returned non-null value.'); +assert.strictEqual(true, params.has('a'), + 'Search params object has name "a"'); +assert.strictEqual(false, params.has('b'), + 'Search params object has not got name "b"'); +params = new URLSearchParams('a=b&c'); +assert.notEqual(params, null, 'constructor returned non-null value.'); +assert.strictEqual(true, params.has('a'), + 'Search params object has name "a"'); +assert.strictEqual(true, params.has('c'), + 'Search params object has name "c"'); +params = new URLSearchParams('&a&&& &&&&&a+b=& c&m%c3%b8%c3%b8'); +assert.notEqual(params, null, 'constructor returned non-null value.'); +assert.strictEqual(true, params.has('a'), 'Search params object has name "a"'); +assert.strictEqual(true, params.has('a b'), + 'Search params object has name "a b"'); +assert.strictEqual(true, params.has(' '), + 'Search params object has name " "'); +assert.strictEqual(false, params.has('c'), + 'Search params object did not have the name "c"'); +assert.strictEqual(true, params.has(' c'), + 'Search params object has name " c"'); +assert.strictEqual(true, params.has('møø'), + 'Search params object has name "møø"'); + +// URLSearchParams constructor, object. +const seed = new URLSearchParams('a=b&c=d'); +params = new URLSearchParams(seed); +assert.notEqual(params, null, 'constructor returned non-null value.'); +assert.strictEqual(params.get('a'), 'b'); +assert.strictEqual(params.get('c'), 'd'); +assert.strictEqual(false, params.has('d')); +// The name-value pairs are copied when created; later updates +// should not be observable. +seed.append('e', 'f'); +assert.strictEqual(false, params.has('e')); +params.append('g', 'h'); +assert.strictEqual(false, seed.has('g')); + +// Parse + +params = new URLSearchParams('a=b+c'); +assert.strictEqual(params.get('a'), 'b c'); +params = new URLSearchParams('a+b=c'); +assert.strictEqual(params.get('a b'), 'c'); + +// Parse space +params = new URLSearchParams('a=b c'); +assert.strictEqual(params.get('a'), 'b c'); +params = new URLSearchParams('a b=c'); +assert.strictEqual(params.get('a b'), 'c'); + +// Parse %20 +params = new URLSearchParams('a=b%20c'); +assert.strictEqual(params.get('a'), 'b c'); +params = new URLSearchParams('a%20b=c'); +assert.strictEqual(params.get('a b'), 'c'); + +// Parse \0 +params = new URLSearchParams('a=b\0c'); +assert.strictEqual(params.get('a'), 'b\0c'); +params = new URLSearchParams('a\0b=c'); +assert.strictEqual(params.get('a\0b'), 'c'); + +// Parse %00 +params = new URLSearchParams('a=b%00c'); +assert.strictEqual(params.get('a'), 'b\0c'); +params = new URLSearchParams('a%00b=c'); +assert.strictEqual(params.get('a\0b'), 'c'); + +// Parse \u2384 (Unicode Character 'COMPOSITION SYMBOL' (U+2384)) +params = new URLSearchParams('a=b\u2384'); +assert.strictEqual(params.get('a'), 'b\u2384'); +params = new URLSearchParams('a\u2384b=c'); +assert.strictEqual(params.get('a\u2384b'), 'c'); + +// Parse %e2%8e%84 (Unicode Character 'COMPOSITION SYMBOL' (U+2384)) +params = new URLSearchParams('a=b%e2%8e%84'); +assert.strictEqual(params.get('a'), 'b\u2384'); +params = new URLSearchParams('a%e2%8e%84b=c'); +assert.strictEqual(params.get('a\u2384b'), 'c'); + +// Parse \uD83D\uDCA9 (Unicode Character 'PILE OF POO' (U+1F4A9)) +params = new URLSearchParams('a=b\uD83D\uDCA9c'); +assert.strictEqual(params.get('a'), 'b\uD83D\uDCA9c'); +params = new URLSearchParams('a\uD83D\uDCA9b=c'); +assert.strictEqual(params.get('a\uD83D\uDCA9b'), 'c'); + +// Parse %f0%9f%92%a9 (Unicode Character 'PILE OF POO' (U+1F4A9)) +params = new URLSearchParams('a=b%f0%9f%92%a9c'); +assert.strictEqual(params.get('a'), 'b\uD83D\uDCA9c'); +params = new URLSearchParams('a%f0%9f%92%a9b=c'); +assert.strictEqual(params.get('a\uD83D\uDCA9b'), 'c'); diff --git a/test/parallel/test-whatwg-url-searchparams-delete.js b/test/parallel/test-whatwg-url-searchparams-delete.js new file mode 100644 index 00000000000000..bdccf49baf1cc4 --- /dev/null +++ b/test/parallel/test-whatwg-url-searchparams-delete.js @@ -0,0 +1,44 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const URL = require('url').URL; + +const m = new URL('http://example.org'); +let params = m.searchParams; + +// Until we export URLSearchParams +const URLSearchParams = params.constructor; + +// Delete basics +params = new URLSearchParams('a=b&c=d'); +params.delete('a'); +assert.strictEqual(params + '', 'c=d'); +params = new URLSearchParams('a=a&b=b&a=a&c=c'); +params.delete('a'); +assert.strictEqual(params + '', 'b=b&c=c'); +params = new URLSearchParams('a=a&=&b=b&c=c'); +params.delete(''); +assert.strictEqual(params + '', 'a=a&b=b&c=c'); +params = new URLSearchParams('a=a&null=null&b=b'); +params.delete(null); +assert.strictEqual(params + '', 'a=a&b=b'); +params = new URLSearchParams('a=a&undefined=undefined&b=b'); +params.delete(undefined); +assert.strictEqual(params + '', 'a=a&b=b'); + +// Deleting appended multiple +params = new URLSearchParams(); +params.append('first', 1); +assert.strictEqual(true, params.has('first'), + 'Search params object has name "first"'); +assert.strictEqual(params.get('first'), '1', + 'Search params object has name "first" with value "1"'); +params.delete('first'); +assert.strictEqual(false, params.has('first'), + 'Search params object has no "first" name'); +params.append('first', 1); +params.append('first', 10); +params.delete('first'); +assert.strictEqual(false, params.has('first'), + 'Search params object has no "first" name'); diff --git a/test/parallel/test-whatwg-url-searchparams-foreach.js b/test/parallel/test-whatwg-url-searchparams-foreach.js new file mode 100644 index 00000000000000..b6d684b06743b0 --- /dev/null +++ b/test/parallel/test-whatwg-url-searchparams-foreach.js @@ -0,0 +1,43 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const URL = require('url').URL; + +const m = new URL('http://example.org'); +let params = m.searchParams; + +// Until we export URLSearchParams +const URLSearchParams = params.constructor; + +let a, b, i; + +// ForEach Check +params = new URLSearchParams('a=1&b=2&c=3'); +const keys = []; +const values = []; +params.forEach(function(value, key) { + keys.push(key); + values.push(value); +}); +assert.deepStrictEqual(keys, ['a', 'b', 'c']); +assert.deepStrictEqual(values, ['1', '2', '3']); + +// For-of Check +a = new URL('http://a.b/c?a=1&b=2&c=3&d=4'); +b = a.searchParams; +const c = []; +for (i of b) { + a.search = 'x=1&y=2&z=3'; + c.push(i); +} +assert.deepStrictEqual(c[0], ['a', '1']); +assert.deepStrictEqual(c[1], ['y', '2']); +assert.deepStrictEqual(c[2], ['z', '3']); + +// empty +a = new URL('http://a.b/c'); +b = a.searchParams; +for (i of b) { + assert(false, 'should not be reached'); +} diff --git a/test/parallel/test-whatwg-url-searchparams-get.js b/test/parallel/test-whatwg-url-searchparams-get.js new file mode 100644 index 00000000000000..667738f817b707 --- /dev/null +++ b/test/parallel/test-whatwg-url-searchparams-get.js @@ -0,0 +1,35 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const URL = require('url').URL; + +const m = new URL('http://example.org'); +let params = m.searchParams; + +// Until we export URLSearchParams +const URLSearchParams = params.constructor; + +// Get basics +params = new URLSearchParams('a=b&c=d'); +assert.strictEqual(params.get('a'), 'b'); +assert.strictEqual(params.get('c'), 'd'); +assert.strictEqual(params.get('e'), null); +params = new URLSearchParams('a=b&c=d&a=e'); +assert.strictEqual(params.get('a'), 'b'); +params = new URLSearchParams('=b&c=d'); +assert.strictEqual(params.get(''), 'b'); +params = new URLSearchParams('a=&c=d&a=e'); +assert.strictEqual(params.get('a'), ''); + +// More get() basics +params = new URLSearchParams('first=second&third&&'); +assert.notEqual(params, null, 'constructor returned non-null value.'); +assert.strictEqual(true, params.has('first'), + 'Search params object has name "first"'); +assert.strictEqual(params.get('first'), 'second', + 'Search params object has name "first" with value "second"'); +assert.strictEqual(params.get('third'), '', + 'Search params object has name "third" with empty value.'); +assert.strictEqual(params.get('fourth'), null, + 'Search params object has no "fourth" name and value.'); diff --git a/test/parallel/test-whatwg-url-searchparams-getall.js b/test/parallel/test-whatwg-url-searchparams-getall.js new file mode 100644 index 00000000000000..8333982d688c3b --- /dev/null +++ b/test/parallel/test-whatwg-url-searchparams-getall.js @@ -0,0 +1,43 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const URL = require('url').URL; + +const m = new URL('http://example.org'); +let params = m.searchParams; + +// Until we export URLSearchParams +const URLSearchParams = params.constructor; + +let matches; + +// getAll() basics +params = new URLSearchParams('a=b&c=d'); +assert.deepStrictEqual(params.getAll('a'), ['b']); +assert.deepStrictEqual(params.getAll('c'), ['d']); +assert.deepStrictEqual(params.getAll('e'), []); +params = new URLSearchParams('a=b&c=d&a=e'); +assert.deepStrictEqual(params.getAll('a'), ['b', 'e']); +params = new URLSearchParams('=b&c=d'); +assert.deepStrictEqual(params.getAll(''), ['b']); +params = new URLSearchParams('a=&c=d&a=e'); +assert.deepStrictEqual(params.getAll('a'), ['', 'e']); + +// getAll() multiples +params = new URLSearchParams('a=1&a=2&a=3&a'); +assert.strictEqual(true, params.has('a'), + 'Search params object has name "a"'); +matches = params.getAll('a'); +assert(matches && matches.length == 4, + 'Search params object has values for name "a"'); +assert.deepStrictEqual(matches, ['1', '2', '3', ''], + 'Search params object has expected name "a" values'); +params.set('a', 'one'); +assert.strictEqual(params.get('a'), 'one', + 'Search params object has name "a" with value "one"'); +matches = params.getAll('a'); +assert(matches && matches.length == 1, + 'Search params object has values for name "a"'); +assert.deepStrictEqual(matches, ['one'], + 'Search params object has expected name "a" values'); diff --git a/test/parallel/test-whatwg-url-searchparams-has.js b/test/parallel/test-whatwg-url-searchparams-has.js new file mode 100644 index 00000000000000..c884227e0b0f1d --- /dev/null +++ b/test/parallel/test-whatwg-url-searchparams-has.js @@ -0,0 +1,39 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const URL = require('url').URL; + +const m = new URL('http://example.org'); +let params = m.searchParams; + +// Until we export URLSearchParams +const URLSearchParams = params.constructor; + +// Has basics +params = new URLSearchParams('a=b&c=d'); +assert.strictEqual(true, params.has('a')); +assert.strictEqual(true, params.has('c')); +assert.strictEqual(false, params.has('e')); +params = new URLSearchParams('a=b&c=d&a=e'); +assert.strictEqual(true, params.has('a')); +params = new URLSearchParams('=b&c=d'); +assert.strictEqual(true, params.has('')); +params = new URLSearchParams('null=a'); +assert.strictEqual(true, params.has(null)); + +// has() following delete() +params = new URLSearchParams('a=b&c=d&&'); +params.append('first', 1); +params.append('first', 2); +assert.strictEqual(true, params.has('a'), + 'Search params object has name "a"'); +assert.strictEqual(true, params.has('c'), + 'Search params object has name "c"'); +assert.strictEqual(true, params.has('first'), + 'Search params object has name "first"'); +assert.strictEqual(false, params.has('d'), + 'Search params object has no name "d"'); +params.delete('first'); +assert.strictEqual(false, params.has('first'), + 'Search params object has no name "first"'); diff --git a/test/parallel/test-whatwg-url-searchparams-set.js b/test/parallel/test-whatwg-url-searchparams-set.js new file mode 100644 index 00000000000000..2d9ae8aaa8d021 --- /dev/null +++ b/test/parallel/test-whatwg-url-searchparams-set.js @@ -0,0 +1,38 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const URL = require('url').URL; + +const m = new URL('http://example.org'); +let params = m.searchParams; + +// Until we export URLSearchParams +const URLSearchParams = params.constructor; + +// Set basics +params = new URLSearchParams('a=b&c=d'); +params.set('a', 'B'); +assert.strictEqual(params + '', 'a=B&c=d'); +params = new URLSearchParams('a=b&c=d&a=e'); +params.set('a', 'B'); +assert.strictEqual(params + '', 'a=B&c=d'); +params.set('e', 'f'); +assert.strictEqual(params + '', 'a=B&c=d&e=f'); + +// URLSearchParams.set +params = new URLSearchParams('a=1&a=2&a=3'); +assert.strictEqual(true, params.has('a'), + 'Search params object has name "a"'); +assert.strictEqual(params.get('a'), '1', + 'Search params object has name "a" with value "1"'); +params.set('first', 4); +assert.strictEqual(true, params.has('a'), + 'Search params object has name "a"'); +assert.strictEqual(params.get('a'), '1', + 'Search params object has name "a" with value "1"'); +params.set('a', 4); +assert.strictEqual(true, params.has('a'), + 'Search params object has name "a"'); +assert.strictEqual(params.get('a'), '4', + 'Search params object has name "a" with value "4"'); diff --git a/test/parallel/test-whatwg-url-searchparams-stringifier.js b/test/parallel/test-whatwg-url-searchparams-stringifier.js new file mode 100644 index 00000000000000..0a53df634454b6 --- /dev/null +++ b/test/parallel/test-whatwg-url-searchparams-stringifier.js @@ -0,0 +1,116 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const URL = require('url').URL; + +const m = new URL('http://example.org'); +let params = m.searchParams; + +// Until we export URLSearchParams +const URLSearchParams = params.constructor; + +// Serialize space +// querystring does not currently handle spaces intelligently +// params = new URLSearchParams(); +// params.append('a', 'b c'); +// assert.strictEqual(params + '', 'a=b+c'); +// params.delete('a'); +// params.append('a b', 'c'); +// assert.strictEqual(params + '', 'a+b=c'); + +// Serialize empty value +params = new URLSearchParams(); +params.append('a', ''); +assert.strictEqual(params + '', 'a='); +params.append('a', ''); +assert.strictEqual(params + '', 'a=&a='); +params.append('', 'b'); +assert.strictEqual(params + '', 'a=&a=&=b'); +params.append('', ''); +assert.strictEqual(params + '', 'a=&a=&=b&='); +params.append('', ''); +assert.strictEqual(params + '', 'a=&a=&=b&=&='); + +// Serialize empty name +params = new URLSearchParams(); +params.append('', 'b'); +assert.strictEqual(params + '', '=b'); +params.append('', 'b'); +assert.strictEqual(params + '', '=b&=b'); + +// Serialize empty name and value +params = new URLSearchParams(); +params.append('', ''); +assert.strictEqual(params + '', '='); +params.append('', ''); +assert.strictEqual(params + '', '=&='); + +// Serialize + +params = new URLSearchParams(); +params.append('a', 'b+c'); +assert.strictEqual(params + '', 'a=b%2Bc'); +params.delete('a'); +params.append('a+b', 'c'); +assert.strictEqual(params + '', 'a%2Bb=c'); + +// Serialize = +params = new URLSearchParams(); +params.append('=', 'a'); +assert.strictEqual(params + '', '%3D=a'); +params.append('b', '='); +assert.strictEqual(params + '', '%3D=a&b=%3D'); + +// Serialize & +params = new URLSearchParams(); +params.append('&', 'a'); +assert.strictEqual(params + '', '%26=a'); +params.append('b', '&'); +assert.strictEqual(params + '', '%26=a&b=%26'); + +// Serialize *-._ +params = new URLSearchParams(); +params.append('a', '*-._'); +assert.strictEqual(params + '', 'a=*-._'); +params.delete('a'); +params.append('*-._', 'c'); +assert.strictEqual(params + '', '*-._=c'); + +// Serialize % +params = new URLSearchParams(); +params.append('a', 'b%c'); +assert.strictEqual(params + '', 'a=b%25c'); +params.delete('a'); +params.append('a%b', 'c'); +assert.strictEqual(params + '', 'a%25b=c'); + +// Serialize \\0 +params = new URLSearchParams(); +params.append('a', 'b\0c'); +assert.strictEqual(params + '', 'a=b%00c'); +params.delete('a'); +params.append('a\0b', 'c'); +assert.strictEqual(params + '', 'a%00b=c'); + +// Serialize \uD83D\uDCA9 +// Unicode Character 'PILE OF POO' (U+1F4A9) +params = new URLSearchParams(); +params.append('a', 'b\uD83D\uDCA9c'); +assert.strictEqual(params + '', 'a=b%F0%9F%92%A9c'); +params.delete('a'); +params.append('a\uD83D\uDCA9b', 'c'); +assert.strictEqual(params + '', 'a%F0%9F%92%A9b=c'); + +// URLSearchParams.toString + +// querystring parses `&&` as {'': ''} +// params = new URLSearchParams('a=b&c=d&&e&&'); +// assert.strictEqual(params.toString(), 'a=b&c=d&e='); + +// querystring does not currently handle spaces intelligently +// params = new URLSearchParams('a = b &a=b&c=d%20'); +// assert.strictEqual(params.toString(), 'a+=+b+&a=b&c=d+'); + +// The lone '=' _does_ survive the roundtrip. +params = new URLSearchParams('a=&a=b'); +assert.strictEqual(params.toString(), 'a=&a=b'); diff --git a/test/parallel/test-whatwg-url-searchparams.js b/test/parallel/test-whatwg-url-searchparams.js index 99e2e6a748c264..54743b97a31da9 100644 --- a/test/parallel/test-whatwg-url-searchparams.js +++ b/test/parallel/test-whatwg-url-searchparams.js @@ -29,8 +29,21 @@ assert.strictEqual(sp.toString(), serialized); assert.strictEqual(m.search, `?${serialized}`); +assert.strictEqual(sp[Symbol.iterator], sp.entries); + var key, val, n = 0; for ([key, val] of sp) { assert.strictEqual(key, 'a'); assert.strictEqual(val, String(values[n++])); } +n = 0; +for (key of sp.keys()) { + assert.strictEqual(key, 'a'); +} +n = 0; +for (val of sp.values()) { + assert.strictEqual(val, String(values[n++])); +} + +m.search = '?a=a&b=b'; +assert.strictEqual(sp.toString(), 'a=a&b=b'); From ac78812f7fac70f21e63b5fa8b0b94ea5fdfe9d3 Mon Sep 17 00:00:00 2001 From: Ethan Arrowood Date: Thu, 1 Dec 2016 11:22:14 -0600 Subject: [PATCH 15/84] test: refactor test-domain-exit-dispose-again setTimeout at 49:5 requires two arguments. On lines 72 and 73 changed assert.equal() to assert.strictEqual(). PR-URL: https://github.com/nodejs/node/pull/10003 Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel Reviewed-By: Rich Trott --- test/parallel/test-domain-exit-dispose-again.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-domain-exit-dispose-again.js b/test/parallel/test-domain-exit-dispose-again.js index 0928addd9ace55..b1911bb40e0c93 100644 --- a/test/parallel/test-domain-exit-dispose-again.js +++ b/test/parallel/test-domain-exit-dispose-again.js @@ -51,7 +51,7 @@ setTimeout(function firstTimer() { 'a domain that should be disposed.'); disposalFailed = true; process.exit(1); - }); + }, 1); // Make V8 throw an unreferenced error. As a result, the domain's error // handler is called, which disposes the domain "d" and should prevent the @@ -69,8 +69,8 @@ setTimeout(function secondTimer() { }, TIMEOUT_DURATION); process.on('exit', function() { - assert.equal(a, 10); - assert.equal(disposalFailed, false); + assert.strictEqual(a, 10); + assert.strictEqual(disposalFailed, false); assert(secondTimerRan); console.log('ok'); }); From b0c10a24a648af09605c8cf859ea02cc5629afc9 Mon Sep 17 00:00:00 2001 From: Daniel Sims Date: Thu, 1 Dec 2016 10:02:15 -0600 Subject: [PATCH 16/84] test: refactor test-domain-from-timer In this change, the setTimeout needed a second argument, so I set that value to 1. In addition, I changed the assertion to be a strictEquals instead of equals. I changed the var declarations to const in this test. PR-URL: https://github.com/nodejs/node/pull/9889 Reviewed-By: Colin Ihrig Reviewed-By: Luigi Pinca --- test/parallel/test-domain-from-timer.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-domain-from-timer.js b/test/parallel/test-domain-from-timer.js index f0115018ec1879..2ffae758ef6490 100644 --- a/test/parallel/test-domain-from-timer.js +++ b/test/parallel/test-domain-from-timer.js @@ -2,17 +2,17 @@ // Simple tests of most basic domain functionality. require('../common'); -var assert = require('assert'); +const assert = require('assert'); // timeouts call the callback directly from cc, so need to make sure the // domain will be used regardless setTimeout(function() { - var domain = require('domain'); - var d = domain.create(); + const domain = require('domain'); + const d = domain.create(); d.run(function() { process.nextTick(function() { console.trace('in nexttick', process.domain === d); - assert.equal(process.domain, d); + assert.strictEqual(process.domain, d); }); }); -}); +}, 1); From 586b787f1eea5d59be202ed5c70962281eab4e0d Mon Sep 17 00:00:00 2001 From: "Italo A. Casas" Date: Wed, 7 Dec 2016 12:52:47 -0500 Subject: [PATCH 17/84] doc: removing extra space in README MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/10168 Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott Reviewed-By: Evan Lucas Reviewed-By: Michaël Zasso --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 68ffbb90598d0f..065daefbc22f5e 100644 --- a/README.md +++ b/README.md @@ -238,7 +238,7 @@ more information about the governance of the Node.js project, see * [isaacs](https://github.com/isaacs) - **Isaac Z. Schlueter** <i@izs.me> * [italoacasas](https://github.com/italoacasas) -**Italo A. Casas** <me@italoacasas.com> +**Italo A. Casas** <me@italoacasas.com> * [iWuzHere](https://github.com/iWuzHere) - **Imran Iqbal** <imran@imraniqbal.org> * [JacksonTian](https://github.com/JacksonTian) - From df4018e4c5beec58d08a19f3f63a5e75f93f2711 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 20:39:10 +0200 Subject: [PATCH 18/84] doc: reword variable declaration note in repl.md --- doc/api/repl.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index a56a731bbfe946..8d540a61846a75 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -84,10 +84,9 @@ undefined 3 ``` -Unless otherwise scoped within blocks (e.g. `{ ... }`) or functions, variables -declared either implicitly or using the `const` or `let` keywords -are declared at the `global` scope (as well as with the `var` keyword -outside of functions). +Unless otherwise scoped within blocks or functions, variables declared +either implicitly, or using the const, let, or var keywords +are declared at the global scope. #### Global and Local Scope From 29b2e88e690ae7f23412e4c0d6f58a3663c8e390 Mon Sep 17 00:00:00 2001 From: Jenna Vuong Date: Tue, 15 Nov 2016 16:10:43 -0800 Subject: [PATCH 19/84] test: improve test-fs-read-stream.js MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit var -> const assert.equal() -> assert.strictEqual() PR-URL: https://github.com/nodejs/node/pull/9629 Reviewed-By: Rich Trott Reviewed-By: Colin Ihrig Reviewed-By: Michael Dawson Reviewed-By: James M Snell Reviewed-By: Michaël Zasso Reviewed-By: Luigi Pinca Reviewed-By: Ben Noordhuis Reviewed-By: Gibson Fahnestock Reviewed-By: Franziska Hinkelmann --- test/parallel/test-fs-read-stream.js | 38 ++++++++++++++-------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/test/parallel/test-fs-read-stream.js b/test/parallel/test-fs-read-stream.js index 7b133022511910..c8da0275c53c9b 100644 --- a/test/parallel/test-fs-read-stream.js +++ b/test/parallel/test-fs-read-stream.js @@ -1,11 +1,11 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); +const common = require('../common'); +const assert = require('assert'); -var path = require('path'); -var fs = require('fs'); -var fn = path.join(common.fixturesDir, 'elipses.txt'); -var rangeFile = path.join(common.fixturesDir, 'x.txt'); +const path = require('path'); +const fs = require('fs'); +const fn = path.join(common.fixturesDir, 'elipses.txt'); +const rangeFile = path.join(common.fixturesDir, 'x.txt'); var callbacks = { open: 0, end: 0, close: 0 }; @@ -20,7 +20,7 @@ assert.strictEqual(file.bytesRead, 0); file.on('open', function(fd) { file.length = 0; callbacks.open++; - assert.equal('number', typeof fd); + assert.strictEqual('number', typeof fd); assert.strictEqual(file.bytesRead, 0); assert.ok(file.readable); @@ -67,12 +67,12 @@ file.on('close', function() { var file3 = fs.createReadStream(fn, {encoding: 'utf8'}); file3.length = 0; file3.on('data', function(data) { - assert.equal('string', typeof data); + assert.strictEqual('string', typeof data); file3.length += data.length; for (var i = 0; i < data.length; i++) { // http://www.fileformat.info/info/unicode/char/2026/index.htm - assert.equal('\u2026', data[i]); + assert.strictEqual('\u2026', data[i]); } }); @@ -81,11 +81,11 @@ file3.on('close', function() { }); process.on('exit', function() { - assert.equal(1, callbacks.open); - assert.equal(1, callbacks.end); - assert.equal(2, callbacks.close); - assert.equal(30000, file.length); - assert.equal(10000, file3.length); + assert.strictEqual(1, callbacks.open); + assert.strictEqual(1, callbacks.end); + assert.strictEqual(2, callbacks.close); + assert.strictEqual(30000, file.length); + assert.strictEqual(10000, file3.length); console.error('ok'); }); @@ -95,7 +95,7 @@ file4.on('data', function(data) { contentRead += data.toString('utf-8'); }); file4.on('end', function(data) { - assert.equal(contentRead, 'yz'); + assert.strictEqual(contentRead, 'yz'); }); var file5 = fs.createReadStream(rangeFile, {bufferSize: 1, start: 1}); @@ -104,7 +104,7 @@ file5.on('data', function(data) { file5.data += data.toString('utf-8'); }); file5.on('end', function() { - assert.equal(file5.data, 'yz\n'); + assert.strictEqual(file5.data, 'yz\n'); }); // https://github.com/joyent/node/issues/2320 @@ -114,7 +114,7 @@ file6.on('data', function(data) { file6.data += data.toString('utf-8'); }); file6.on('end', function() { - assert.equal(file6.data, 'yz\n'); + assert.strictEqual(file6.data, 'yz\n'); }); assert.throws(function() { @@ -129,7 +129,7 @@ stream.on('data', function(chunk) { }); stream.on('end', function() { - assert.equal('x', stream.data); + assert.strictEqual('x', stream.data); }); // pause and then resume immediately. @@ -155,7 +155,7 @@ function file7Next() { file7.data += data; }); file7.on('end', function(err) { - assert.equal(file7.data, 'xyz\n'); + assert.strictEqual(file7.data, 'xyz\n'); }); } From c6eae5afffdb1d75e75c9b3c57ec46d5be51dbc4 Mon Sep 17 00:00:00 2001 From: "Italo A. Casas" Date: Wed, 7 Dec 2016 13:21:00 -0500 Subject: [PATCH 20/84] doc: adding missing - in README PR-URL: https://github.com/nodejs/node/pull/10170 Reviewed-By: Rich Trott --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 065daefbc22f5e..bdb74b5446ac40 100644 --- a/README.md +++ b/README.md @@ -237,7 +237,7 @@ more information about the governance of the Node.js project, see **Ilkka Myller** <ilkka.myller@nodefield.com> * [isaacs](https://github.com/isaacs) - **Isaac Z. Schlueter** <i@izs.me> -* [italoacasas](https://github.com/italoacasas) +* [italoacasas](https://github.com/italoacasas) - **Italo A. Casas** <me@italoacasas.com> * [iWuzHere](https://github.com/iWuzHere) - **Imran Iqbal** <imran@imraniqbal.org> From 89603835b3b165f3b64373c06f635aaf04dd626c Mon Sep 17 00:00:00 2001 From: Rob Adelmann Date: Sun, 4 Dec 2016 11:21:37 -0800 Subject: [PATCH 21/84] test: refactor test-beforeexit-event - replaced var with const/let. - removed all console.log() statements. - removed deaths and revivals vars. - wrapped beforexit listener callbacks with common.mustCall(). - removed exit event listener. PR-URL: https://github.com/nodejs/node/pull/10121 Reviewed-By: Jeremiah Senkpiel Reviewed-By: Brian White Reviewed-By: James M Snell Reviewed-By: Michael Dawson --- test/parallel/test-beforeexit-event.js | 40 ++++++++------------------ 1 file changed, 12 insertions(+), 28 deletions(-) diff --git a/test/parallel/test-beforeexit-event.js b/test/parallel/test-beforeexit-event.js index 4decbcf9f921f3..ef94da76af3883 100644 --- a/test/parallel/test-beforeexit-event.js +++ b/test/parallel/test-beforeexit-event.js @@ -1,42 +1,26 @@ 'use strict'; -require('../common'); -var assert = require('assert'); -var net = require('net'); -var revivals = 0; -var deaths = 0; +const common = require('../common'); +const net = require('net'); -process.on('beforeExit', function() { deaths++; }); - -process.once('beforeExit', tryImmediate); +process.once('beforeExit', common.mustCall(tryImmediate)); function tryImmediate() { - console.log('set immediate'); - setImmediate(function() { - revivals++; - process.once('beforeExit', tryTimer); - }); + setImmediate(common.mustCall(() => { + process.once('beforeExit', common.mustCall(tryTimer)); + })); } function tryTimer() { - console.log('set a timeout'); - setTimeout(function() { - console.log('timeout cb, do another once beforeExit'); - revivals++; - process.once('beforeExit', tryListen); - }, 1); + setTimeout(common.mustCall(() => { + process.once('beforeExit', common.mustCall(tryListen)); + }), 1); } function tryListen() { - console.log('create a server'); net.createServer() .listen(0) - .on('listening', function() { - revivals++; + .on('listening', common.mustCall(function() { this.close(); - }); + process.on('beforeExit', common.mustCall(() => {})); + })); } - -process.on('exit', function() { - assert.strictEqual(4, deaths); - assert.strictEqual(3, revivals); -}); From 83d9cd80bfd99f01e713aa35a7f2935be09c42da Mon Sep 17 00:00:00 2001 From: Diego Paez Date: Sat, 3 Dec 2016 16:21:11 +0000 Subject: [PATCH 22/84] test: refactor test-https-agent-session-reuse Use const and let instead of var and assert.strictEqual() instead of assert.equal() PR-URL: https://github.com/nodejs/node/pull/10105 Reviewed-By: Colin Ihrig Reviewed-By: Santiago Gimeno Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- .../test-https-agent-session-reuse.js | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/test/parallel/test-https-agent-session-reuse.js b/test/parallel/test-https-agent-session-reuse.js index 01f161574b29db..73465aa5a98fb9 100644 --- a/test/parallel/test-https-agent-session-reuse.js +++ b/test/parallel/test-https-agent-session-reuse.js @@ -1,39 +1,39 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); +const common = require('../common'); +const assert = require('assert'); if (!common.hasCrypto) { common.skip('missing crypto'); return; } -var https = require('https'); -var crypto = require('crypto'); +const https = require('https'); +const crypto = require('crypto'); -var fs = require('fs'); +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 ca = fs.readFileSync(common.fixturesDir + '/keys/ca1-cert.pem'); +const ca = fs.readFileSync(common.fixturesDir + '/keys/ca1-cert.pem'); -var clientSessions = {}; -var serverRequests = 0; +const clientSessions = {}; +let serverRequests = 0; -var agent = new https.Agent({ +const agent = new https.Agent({ maxCachedSessions: 1 }); -var server = https.createServer(options, function(req, res) { +const server = https.createServer(options, function(req, res) { if (req.url === '/drop-key') server.setTicketKeys(crypto.randomBytes(48)); serverRequests++; res.end('ok'); }).listen(0, function() { - var queue = [ + const queue = [ { name: 'first', @@ -97,7 +97,7 @@ var server = https.createServer(options, function(req, res) { ]; function request() { - var options = queue.shift(); + const options = queue.shift(); options.agent = agent; https.request(options, function(res) { clientSessions[options.name] = res.socket.getSession(); @@ -114,9 +114,9 @@ var server = https.createServer(options, function(req, res) { }); process.on('exit', function() { - assert.equal(serverRequests, 6); - assert.equal(clientSessions['first'].toString('hex'), - clientSessions['first-reuse'].toString('hex')); + assert.strictEqual(serverRequests, 6); + assert.strictEqual(clientSessions['first'].toString('hex'), + clientSessions['first-reuse'].toString('hex')); assert.notEqual(clientSessions['first'].toString('hex'), clientSessions['cipher-change'].toString('hex')); assert.notEqual(clientSessions['first'].toString('hex'), @@ -125,6 +125,6 @@ process.on('exit', function() { clientSessions['before-drop'].toString('hex')); assert.notEqual(clientSessions['before-drop'].toString('hex'), clientSessions['after-drop'].toString('hex')); - assert.equal(clientSessions['after-drop'].toString('hex'), - clientSessions['after-drop-reuse'].toString('hex')); + assert.strictEqual(clientSessions['after-drop'].toString('hex'), + clientSessions['after-drop-reuse'].toString('hex')); }); From fd6999ef6cb4a2f865f0d7b94cad0e00bdb24820 Mon Sep 17 00:00:00 2001 From: Paul Graham Date: Thu, 1 Dec 2016 10:22:06 -0600 Subject: [PATCH 23/84] test: improves test-tls-client-verify Swaps var -> const/let assert.equal becomes assert.strictEqual common.mustCall on single-use functions PR-URL: https://github.com/nodejs/node/pull/10051 Reviewed-By: Rich Trott Reviewed-By: Michael Dawson Reviewed-By: Anna Henningsen --- test/parallel/test-tls-client-verify.js | 54 ++++++++++++------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/test/parallel/test-tls-client-verify.js b/test/parallel/test-tls-client-verify.js index 7844253db9c15f..983d88296eb26f 100644 --- a/test/parallel/test-tls-client-verify.js +++ b/test/parallel/test-tls-client-verify.js @@ -1,17 +1,17 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); +const common = require('../common'); +const assert = require('assert'); if (!common.hasCrypto) { common.skip('missing crypto'); return; } -var tls = require('tls'); +const tls = require('tls'); -var fs = require('fs'); +const fs = require('fs'); -var hosterr = /Hostname\/IP doesn't match certificate's altnames/g; -var testCases = +const hosterr = /Hostname\/IP doesn't match certificate's altnames/g; +const testCases = [{ ca: ['ca1-cert'], key: 'agent2-key', cert: 'agent2-cert', @@ -52,16 +52,16 @@ function loadPEM(n) { return fs.readFileSync(filenamePEM(n)); } -var successfulTests = 0; +let successfulTests = 0; function testServers(index, servers, clientOptions, cb) { - var serverOptions = servers[index]; + const serverOptions = servers[index]; if (!serverOptions) { cb(); return; } - var ok = serverOptions.ok; + const ok = serverOptions.ok; if (serverOptions.key) { serverOptions.key = loadPEM(serverOptions.key); @@ -71,37 +71,37 @@ function testServers(index, servers, clientOptions, cb) { serverOptions.cert = loadPEM(serverOptions.cert); } - var server = tls.createServer(serverOptions, function(s) { + const server = tls.createServer(serverOptions, common.mustCall(function(s) { s.end('hello world\n'); - }); + })); - server.listen(0, function() { - var b = ''; + server.listen(0, common.mustCall(function() { + let b = ''; console.error('connecting...'); clientOptions.port = this.address().port; - var client = tls.connect(clientOptions, function() { - var authorized = client.authorized || - hosterr.test(client.authorizationError); + const client = tls.connect(clientOptions, common.mustCall(function() { + const authorized = client.authorized || + hosterr.test(client.authorizationError); console.error('expected: ' + ok + ' authed: ' + authorized); - assert.equal(ok, authorized); + assert.strictEqual(ok, authorized); server.close(); - }); + })); client.on('data', function(d) { b += d.toString(); }); - client.on('end', function() { - assert.equal('hello world\n', b); - }); + client.on('end', common.mustCall(function() { + assert.strictEqual('hello world\n', b); + })); - client.on('close', function() { + client.on('close', common.mustCall(function() { testServers(index + 1, servers, clientOptions, cb); - }); - }); + })); + })); } @@ -109,7 +109,7 @@ function runTest(testIndex) { var tcase = testCases[testIndex]; if (!tcase) return; - var clientOptions = { + const clientOptions = { port: undefined, ca: tcase.ca.map(loadPEM), key: loadPEM(tcase.key), @@ -118,10 +118,10 @@ function runTest(testIndex) { }; - testServers(0, tcase.servers, clientOptions, function() { + testServers(0, tcase.servers, clientOptions, common.mustCall(function() { successfulTests++; runTest(testIndex + 1); - }); + })); } From 57f993d0f57276c61794a7cb7615261353909422 Mon Sep 17 00:00:00 2001 From: Jared Young Date: Thu, 1 Dec 2016 10:15:36 -0800 Subject: [PATCH 24/84] test: renamed assert.Equal to assert.strictEqual --- test/parallel/test-crypto-authenticated.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-crypto-authenticated.js b/test/parallel/test-crypto-authenticated.js index 8eab91e549dfa7..4bed5aa665b64d 100644 --- a/test/parallel/test-crypto-authenticated.js +++ b/test/parallel/test-crypto-authenticated.js @@ -352,7 +352,7 @@ for (const i in TEST_CASES) { let msg = decrypt.update(test.ct, 'hex', outputEncoding); if (!test.tampered) { msg += decrypt.final(outputEncoding); - assert.equal(msg, test.plain); + assert.strictEqual(msg, test.plain); } else { // assert that final throws if input data could not be verified! assert.throws(function() { decrypt.final('ascii'); }, / auth/); From e6a0c39bf7b2b23bdd502828bfb5a549bec6f0c6 Mon Sep 17 00:00:00 2001 From: vazina robertson Date: Thu, 1 Dec 2016 11:22:42 -0600 Subject: [PATCH 25/84] test: changed assert.equal to assert.strictEqual PR-URL: https://github.com/nodejs/node/pull/10015 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen --- test/parallel/test-child-process-stdio-big-write-end.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-child-process-stdio-big-write-end.js b/test/parallel/test-child-process-stdio-big-write-end.js index 4b0f9d2cf3216f..4c9f4159d0d8bc 100644 --- a/test/parallel/test-child-process-stdio-big-write-end.js +++ b/test/parallel/test-child-process-stdio-big-write-end.js @@ -23,7 +23,7 @@ function parent() { n += c; }); child.stdout.on('end', function() { - assert.equal(+n, sent); + assert.strictEqual(+n, sent); console.log('ok'); }); From 558fa1cf1006aa149ca8a0f42aaff0036c9fb33c Mon Sep 17 00:00:00 2001 From: Aileen Date: Thu, 1 Dec 2016 10:36:13 -0600 Subject: [PATCH 26/84] test: change assert.equal to assert.strictEqual PR-URL: https://github.com/nodejs/node/pull/9946 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- test/parallel/test-cluster-send-handle-twice.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-cluster-send-handle-twice.js b/test/parallel/test-cluster-send-handle-twice.js index f1552d79265a0c..172a5563f306f5 100644 --- a/test/parallel/test-cluster-send-handle-twice.js +++ b/test/parallel/test-cluster-send-handle-twice.js @@ -14,7 +14,7 @@ if (cluster.isMaster) { for (var i = 0; i < workers.toStart; ++i) { var worker = cluster.fork(); worker.on('exit', function(code, signal) { - assert.equal(code, 0, 'Worker exited with an error code'); + assert.strictEqual(code, 0, 'Worker exited with an error code'); assert(!signal, 'Worker exited by a signal'); }); } From deb9cc0cde689e5edd5df45b7ca375f8debeb9be Mon Sep 17 00:00:00 2001 From: anoff Date: Thu, 1 Dec 2016 10:59:09 -0600 Subject: [PATCH 27/84] test: use `assert.strictEqual` * use `assert.strictEqual` PR-URL: https://github.com/nodejs/node/pull/9975 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- test/parallel/test-debug-port-from-cmdline.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-debug-port-from-cmdline.js b/test/parallel/test-debug-port-from-cmdline.js index e1125507c208ee..abec53cbd3c83a 100644 --- a/test/parallel/test-debug-port-from-cmdline.js +++ b/test/parallel/test-debug-port-from-cmdline.js @@ -42,7 +42,7 @@ function assertOutputLines() { 'Debugger listening on 127.0.0.1:' + debugPort, ]; - assert.equal(outputLines.length, expectedLines.length); + assert.strictEqual(outputLines.length, expectedLines.length); for (var i = 0; i < expectedLines.length; i++) assert(expectedLines[i].includes(outputLines[i])); } From 68488e9c75cf24632fd68063168992ca6830655c Mon Sep 17 00:00:00 2001 From: Bruce Lai Date: Thu, 1 Dec 2016 10:49:39 -0600 Subject: [PATCH 28/84] test: fix error in test-cluster-worker-death.js Replaced calls to assert.equal with assert.strictEqual in order to fix the following error: "Please use assert.strictEqual() instead of assert.strictEqual()" PR-URL: https://github.com/nodejs/node/pull/9981 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- test/parallel/test-cluster-worker-death.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-cluster-worker-death.js b/test/parallel/test-cluster-worker-death.js index 8bbf46de568eb0..ac32705106e03c 100644 --- a/test/parallel/test-cluster-worker-death.js +++ b/test/parallel/test-cluster-worker-death.js @@ -8,10 +8,10 @@ if (!cluster.isMaster) { } else { var worker = cluster.fork(); worker.on('exit', common.mustCall(function(exitCode, signalCode) { - assert.equal(exitCode, 42); - assert.equal(signalCode, null); + assert.strictEqual(exitCode, 42); + assert.strictEqual(signalCode, null); })); cluster.on('exit', common.mustCall(function(worker_) { - assert.equal(worker_, worker); + assert.strictEqual(worker_, worker); })); } From 1e4b9a1450d94bb7e45bcecc772e69638b6f10a3 Mon Sep 17 00:00:00 2001 From: eudaimos Date: Thu, 1 Dec 2016 10:25:58 -0600 Subject: [PATCH 29/84] test: refactor test-crypto-hmac * replace assert.equals with assert.strictEquals * use template strings where appropriate PR-URL: https://github.com/nodejs/node/pull/9958 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- test/parallel/test-crypto-hmac.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-crypto-hmac.js b/test/parallel/test-crypto-hmac.js index 7c550be4c2ebf3..5307ea4f6f9e05 100644 --- a/test/parallel/test-crypto-hmac.js +++ b/test/parallel/test-crypto-hmac.js @@ -13,7 +13,7 @@ var h1 = crypto.createHmac('sha1', 'Node') .update('some data') .update('to hmac') .digest('hex'); -assert.equal(h1, '19fd6e1ba73d9ed2224dd5094a71babe85d9a892', 'test HMAC'); +assert.strictEqual(h1, '19fd6e1ba73d9ed2224dd5094a71babe85d9a892', 'test HMAC'); // Test HMAC (Wikipedia Test Cases) var wikipedia = [ @@ -67,9 +67,9 @@ for (let i = 0, l = wikipedia.length; i < l; i++) { const result = crypto.createHmac(hash, wikipedia[i]['key']) .update(wikipedia[i]['data']) .digest('hex'); - assert.equal(wikipedia[i]['hmac'][hash], - result, - 'Test HMAC-' + hash + ': Test case ' + (i + 1) + ' wikipedia'); + assert.strictEqual(wikipedia[i]['hmac'][hash], + result, + `Test HMAC-${hash}: Test case ${i + 1} wikipedia`); } } @@ -233,10 +233,10 @@ for (let i = 0, l = rfc4231.length; i < l; i++) { result = result.substr(0, 32); // first 128 bits == 32 hex chars strRes = strRes.substr(0, 32); } - assert.equal(rfc4231[i]['hmac'][hash], - result, - 'Test HMAC-' + hash + ': Test case ' + (i + 1) + ' rfc 4231'); - assert.equal(strRes, result, 'Should get same result from stream'); + assert.strictEqual(rfc4231[i]['hmac'][hash], + result, + `Test HMAC-${hash}: Test case ${i + 1} rfc 4231`); + assert.strictEqual(strRes, result, 'Should get same result from stream'); } } @@ -356,7 +356,7 @@ if (!common.hasFipsCrypto) { crypto.createHmac('md5', rfc2202_md5[i]['key']) .update(rfc2202_md5[i]['data']) .digest('hex'), - 'Test HMAC-MD5 : Test case ' + (i + 1) + ' rfc 2202' + `Test HMAC-MD5 : Test case ${i + 1} rfc 2202` ); } } @@ -366,6 +366,6 @@ for (let i = 0, l = rfc2202_sha1.length; i < l; i++) { crypto.createHmac('sha1', rfc2202_sha1[i]['key']) .update(rfc2202_sha1[i]['data']) .digest('hex'), - 'Test HMAC-SHA1 : Test case ' + (i + 1) + ' rfc 2202' + `Test HMAC-SHA1 : Test case ${i + 1} rfc 2202` ); } From 2d0ce510e8e3d03ad3d45f5ba52dd061e1fd9497 Mon Sep 17 00:00:00 2001 From: anoff Date: Thu, 1 Dec 2016 11:54:46 -0600 Subject: [PATCH 30/84] doc: update `path.format` description and examples * removed pseudo-code * added info on which properties have priority * modified examples to show ignored properties PR-URL: https://github.com/nodejs/node/pull/10046 Reviewed-By: James M Snell Reviewed-By: Sam Roberts --- doc/api/path.md | 40 +++++++++++++--------------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/doc/api/path.md b/doc/api/path.md index fdd3063e69bf81..a9c72c2e2e78a6 100644 --- a/doc/api/path.md +++ b/doc/api/path.md @@ -182,26 +182,20 @@ added: v0.11.15 The `path.format()` method returns a path string from an object. This is the opposite of [`path.parse()`][]. -The following process is used when constructing the path string: - -* `output` is set to an empty string. -* If `pathObject.dir` is specified, `pathObject.dir` is appended to `output` - followed by the value of `path.sep`; -* Otherwise, if `pathObject.root` is specified, `pathObject.root` is appended - to `output`. -* If `pathObject.base` is specified, `pathObject.base` is appended to `output`; -* Otherwise: - * If `pathObject.name` is specified, `pathObject.name` is appended to `output` - * If `pathObject.ext` is specified, `pathObject.ext` is appended to `output`. -* Return `output` +When providing properties to the `pathObject` remember that there are +combinations where one property has priority over another: + +* `pathObject.root` is ignored if `pathObject.dir` is provided +* `pathObject.ext` and `pathObject.name` are ignored if `pathObject.base` exists For example, on POSIX: ```js -// If `dir` and `base` are provided, +// If `dir`, `root` and `base` are provided, // `${dir}${path.sep}${base}` -// will be returned. +// will be returned. `root` is ignored. path.format({ + root: '/ignored', dir: '/home/user/dir', base: 'file.txt' }); @@ -209,10 +203,11 @@ path.format({ // `root` will be used if `dir` is not specified. // If only `root` is provided or `dir` is equal to `root` then the -// platform separator will not be included. +// platform separator will not be included. `ext` will be ignored. path.format({ root: '/', - base: 'file.txt' + base: 'file.txt', + ext: 'ignored' }); // Returns: '/file.txt' @@ -223,23 +218,14 @@ path.format({ ext: '.txt' }); // Returns: '/file.txt' - -// `base` will be returned if `dir` or `root` are not provided. -path.format({ - base: 'file.txt' -}); -// Returns: 'file.txt' ``` On Windows: ```js path.format({ - root : "C:\\", - dir : "C:\\path\\dir", - base : "file.txt", - ext : ".txt", - name : "file" + dir : "C:\\path\\dir", + base : "file.txt" }); // Returns: 'C:\\path\\dir\\file.txt' ``` From 6967ed45acad1e421f50df426c2454031701267a Mon Sep 17 00:00:00 2001 From: Kyle Corsi Date: Thu, 1 Dec 2016 11:57:23 -0500 Subject: [PATCH 31/84] test: refactor test-net-keepalive.js - Replace require() vars with const. - Replace assert.equal() with assert.strictEqual(). - Add common.mustCall() to the setTimeout() callback. PR-URL: https://github.com/nodejs/node/pull/9995 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- test/parallel/test-net-keepalive.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/parallel/test-net-keepalive.js b/test/parallel/test-net-keepalive.js index d414393a9346cd..e466f0ff580d06 100644 --- a/test/parallel/test-net-keepalive.js +++ b/test/parallel/test-net-keepalive.js @@ -1,20 +1,20 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); -var net = require('net'); +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); var serverConnection; var clientConnection; var echoServer = net.createServer(function(connection) { serverConnection = connection; - setTimeout(function() { + setTimeout(common.mustCall(function() { // make sure both connections are still open - assert.equal(serverConnection.readyState, 'open'); - assert.equal(clientConnection.readyState, 'open'); + assert.strictEqual(serverConnection.readyState, 'open'); + assert.strictEqual(clientConnection.readyState, 'open'); serverConnection.end(); clientConnection.end(); echoServer.close(); - }, common.platformTimeout(100)); + }, 1), common.platformTimeout(100)); connection.setTimeout(0); assert.notEqual(connection.setKeepAlive, undefined); // send a keepalive packet after 50 ms From df3978421b86991fb45256f1a8694e0590019b13 Mon Sep 17 00:00:00 2001 From: Luca Maraschi Date: Sun, 4 Dec 2016 09:32:51 +0100 Subject: [PATCH 32/84] http: verify client method is a string Prior to this commit, it was possible to pass a truthy non-string value as the HTTP method to the HTTP client, resulting in an exception being thrown. This commit adds validation to the method. PR-URL: https://github.com/nodejs/node/pull/10111 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/_http_client.js | 6 ++- .../test-http-client-check-http-token.js | 40 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http-client-check-http-token.js diff --git a/lib/_http_client.js b/lib/_http_client.js index 6837c94df98eea..85e865f565a039 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -68,7 +68,11 @@ function ClientRequest(options, cb) { self.socketPath = options.socketPath; self.timeout = options.timeout; - var method = self.method = (options.method || 'GET').toUpperCase(); + var method = options.method; + if (method != null && typeof method !== 'string') { + throw new TypeError('Method must be a string'); + } + method = self.method = (method || 'GET').toUpperCase(); if (!common._checkIsHttpToken(method)) { throw new TypeError('Method must be a valid HTTP token'); } diff --git a/test/parallel/test-http-client-check-http-token.js b/test/parallel/test-http-client-check-http-token.js new file mode 100644 index 00000000000000..5a2b84a973262c --- /dev/null +++ b/test/parallel/test-http-client-check-http-token.js @@ -0,0 +1,40 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const expectedSuccesses = [undefined, null, 'GET', 'post']; +let requestCount = 0; + +const server = http.createServer((req, res) => { + requestCount++; + res.end(); + + if (expectedSuccesses.length === requestCount) { + server.close(); + } +}).listen(0, test); + +function test() { + function fail(input) { + assert.throws(() => { + http.request({ method: input, path: '/' }, common.fail); + }, /^TypeError: Method must be a string$/); + } + + fail(-1); + fail(1); + fail(0); + fail({}); + fail(true); + fail(false); + fail([]); + + function ok(method) { + http.request({ method: method, port: server.address().port }).end(); + } + + expectedSuccesses.forEach((method) => { + ok(method); + }); +} From a0a6ff2ea5afc93c5d6b592ebecc17ff5b4ffa6a Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Tue, 6 Dec 2016 11:24:26 -0500 Subject: [PATCH 33/84] doc: add some info on `tty#setRawMode()` Partially taken from https://linux.die.net/man/3/cfmakeraw A very simple test script: ``` if (process.argv[2] === 'raw') process.stdin.setRawMode(true) process.stdin.on('data', (chunk) => { console.log(chunk) console.log(chunk.toString()) }) ``` Refs: https://github.com/nodejs/node/pull/10037 PR-URL: https://github.com/nodejs/node/pull/10147 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen --- doc/api/tty.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/doc/api/tty.md b/doc/api/tty.md index f9be1fc4ffd3e5..8f250e45d655f9 100644 --- a/doc/api/tty.md +++ b/doc/api/tty.md @@ -50,6 +50,13 @@ raw device. Defaults to `false`. added: v0.7.7 --> +Allows configuration of `tty.ReadStream` so that it operates as a raw device. + +When in raw mode, input is always available character-by-character, not +including modifiers. Additionally, all special processing of characters by the +terminal is disabled, including echoing input characters. +Note that `CTRL`+`C` will no longer cause a `SIGINT` when in this mode. + * `mode` {boolean} If `true`, configures the `tty.ReadStream` to operate as a raw device. If `false`, configures the `tty.ReadStream` to operate in its default mode. The `readStream.isRaw` property will be set to the resulting From 6dd07545ff504c8349a24c936dbf0ecde3c66238 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 7 Dec 2016 13:08:27 -0800 Subject: [PATCH 34/84] test: fix flaky test-net-socket-timeout The setTimeout() call is unneeded. If the socket never times out, then the test will never finish. Because timers can be unreliable on machines under load, using setTimeout() here effectively creates a race condition. PR-URL: https://github.com/nodejs/node/pull/10172 Reviewed-By: Michael Dawson Reviewed-By: Colin Ihrig Reviewed-By: Santiago Gimeno --- test/parallel/test-net-socket-timeout.js | 29 ++++++++++++------------ 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/test/parallel/test-net-socket-timeout.js b/test/parallel/test-net-socket-timeout.js index 54f5ce301cc52e..61ea54debeaa61 100644 --- a/test/parallel/test-net-socket-timeout.js +++ b/test/parallel/test-net-socket-timeout.js @@ -1,14 +1,17 @@ 'use strict'; -var common = require('../common'); -var net = require('net'); -var assert = require('assert'); +const common = require('../common'); +const net = require('net'); +const assert = require('assert'); // Verify that invalid delays throw -var noop = function() {}; -var s = new net.Socket(); -var nonNumericDelays = ['100', true, false, undefined, null, '', {}, noop, []]; -var badRangeDelays = [-0.001, -1, -Infinity, Infinity, NaN]; -var validDelays = [0, 0.001, 1, 1e6]; +const noop = function() {}; +const s = new net.Socket(); +const nonNumericDelays = [ + '100', true, false, undefined, null, '', {}, noop, [] +]; +const badRangeDelays = [-0.001, -1, -Infinity, Infinity, NaN]; +const validDelays = [0, 0.001, 1, 1e6]; + for (let i = 0; i < nonNumericDelays.length; i++) { assert.throws(function() { @@ -28,15 +31,11 @@ for (let i = 0; i < validDelays.length; i++) { }); } -var server = net.Server(); +const server = net.Server(); server.listen(0, common.mustCall(function() { - var socket = net.createConnection(this.address().port); - socket.setTimeout(100, common.mustCall(function() { + const socket = net.createConnection(this.address().port); + socket.setTimeout(1, common.mustCall(function() { socket.destroy(); server.close(); - clearTimeout(timer); })); - var timer = setTimeout(function() { - process.exit(1); - }, common.platformTimeout(200)); })); From 9a5d47542d2e9e2874188251dc670d14929fe6f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karl=20B=C3=B6hlmark?= Date: Tue, 1 Nov 2016 16:59:31 +0100 Subject: [PATCH 35/84] http: remove stale timeout listeners In order to prevent a memory leak when using keep alive, ensure that the timeout listener for the request is removed when the response has ended. PR-URL: https://github.com/nodejs/node/pull/9440 Reviewed-By: Evan Lucas Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis Reviewed-By: Colin Ihrig --- lib/_http_client.js | 8 +++- ...st-http-client-timeout-option-listeners.js | 44 +++++++++++++++++++ 2 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-http-client-timeout-option-listeners.js diff --git a/lib/_http_client.js b/lib/_http_client.js index 85e865f565a039..9a97e9a2f28452 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -566,7 +566,13 @@ function tickOnSocket(req, socket) { socket.on('close', socketCloseListener); if (req.timeout) { - socket.once('timeout', () => req.emit('timeout')); + const emitRequestTimeout = () => req.emit('timeout'); + socket.once('timeout', emitRequestTimeout); + req.once('response', (res) => { + res.once('end', () => { + socket.removeListener('timeout', emitRequestTimeout); + }); + }); } req.emit('socket', socket); } diff --git a/test/parallel/test-http-client-timeout-option-listeners.js b/test/parallel/test-http-client-timeout-option-listeners.js new file mode 100644 index 00000000000000..3ac6cd4616b496 --- /dev/null +++ b/test/parallel/test-http-client-timeout-option-listeners.js @@ -0,0 +1,44 @@ +'use strict'; +const common = require('../common'); +const http = require('http'); +const assert = require('assert'); + +const agent = new http.Agent({ keepAlive: true }); + +const server = http.createServer((req, res) => { + res.end(''); +}); + +const options = { + agent, + method: 'GET', + port: undefined, + host: common.localhostIPv4, + path: '/', + timeout: common.platformTimeout(100) +}; + +server.listen(0, options.host, common.mustCall(() => { + options.port = server.address().port; + doRequest(common.mustCall((numListeners) => { + assert.strictEqual(numListeners, 1); + doRequest(common.mustCall((numListeners) => { + assert.strictEqual(numListeners, 1); + server.close(); + agent.destroy(); + })); + })); +})); + +function doRequest(cb) { + http.request(options, common.mustCall((response) => { + const sockets = agent.sockets[`${options.host}:${options.port}:`]; + assert.strictEqual(sockets.length, 1); + const socket = sockets[0]; + const numListeners = socket.listeners('timeout').length; + response.resume(); + response.once('end', common.mustCall(() => { + process.nextTick(cb, numListeners); + })); + })).end(); +} From f0da38a5cbdcae27e4545b0b5a5ac03471efc143 Mon Sep 17 00:00:00 2001 From: joyeecheung Date: Thu, 1 Dec 2016 10:44:55 -0600 Subject: [PATCH 36/84] test: increase test coverage of BufferList Add tests for edges cases of BufferList: - test operations when the length is 0 - test operations when the list only has one element PR-URL: https://github.com/nodejs/node/pull/10171 Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen --- test/parallel/test-stream-buffer-list.js | 27 ++++++++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 test/parallel/test-stream-buffer-list.js diff --git a/test/parallel/test-stream-buffer-list.js b/test/parallel/test-stream-buffer-list.js new file mode 100644 index 00000000000000..ddbff452de4be9 --- /dev/null +++ b/test/parallel/test-stream-buffer-list.js @@ -0,0 +1,27 @@ +// Flags: --expose_internals +'use strict'; +require('../common'); +const assert = require('assert'); +const BufferList = require('internal/streams/BufferList'); + +// Test empty buffer list. +const emptyList = new BufferList(); + +emptyList.shift(); +assert.deepStrictEqual(emptyList, new BufferList()); + +assert.strictEqual(emptyList.join(','), ''); + +assert.deepStrictEqual(emptyList.concat(0), Buffer.alloc(0)); + +// Test buffer list with one element. +const list = new BufferList(); +list.push('foo'); + +assert.strictEqual(list.concat(1), 'foo'); + +assert.strictEqual(list.join(','), 'foo'); + +const shifted = list.shift(); +assert.strictEqual(shifted, 'foo'); +assert.deepStrictEqual(list, new BufferList()); From 444b907e92f98df794da3ef97e7dda3572cc6ff8 Mon Sep 17 00:00:00 2001 From: davidmarkclements Date: Thu, 1 Dec 2016 14:12:17 -0600 Subject: [PATCH 37/84] test: refactor test-http-unix-socket Use `common.mustCall()` and `common.fail()` where appropriate. Change `assert.equal` to `assert.strictEqual` to ensure specificity. Change var declarations to const to take advantage of ES6 immutable bindings. PR-URL: https://github.com/nodejs/node/pull/10072 Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Santiago Gimeno --- test/parallel/test-http-unix-socket.js | 29 +++++++++++++------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/test/parallel/test-http-unix-socket.js b/test/parallel/test-http-unix-socket.js index 69c887b53bdbb1..d2b99bde95a234 100644 --- a/test/parallel/test-http-unix-socket.js +++ b/test/parallel/test-http-unix-socket.js @@ -1,9 +1,9 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); -var http = require('http'); +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); -var server = http.createServer(function(req, res) { +const server = http.createServer(function(req, res) { res.writeHead(200, { 'Content-Type': 'text/plain', 'Connection': 'close' @@ -23,8 +23,8 @@ server.listen(common.PIPE, common.mustCall(function() { }; var req = http.get(options, common.mustCall(function(res) { - assert.equal(res.statusCode, 200); - assert.equal(res.headers['content-type'], 'text/plain'); + assert.strictEqual(res.statusCode, 200); + assert.strictEqual(res.headers['content-type'], 'text/plain'); res.body = ''; res.setEncoding('utf8'); @@ -34,19 +34,18 @@ server.listen(common.PIPE, common.mustCall(function() { }); res.on('end', common.mustCall(function() { - assert.equal(res.body, 'hello world\n'); - server.close(function(error) { - assert.equal(error, undefined); - server.close(function(error) { - assert.equal(error && error.message, 'Not running'); - }); - }); + assert.strictEqual(res.body, 'hello world\n'); + server.close(common.mustCall(function(error) { + assert.strictEqual(error, undefined); + server.close(common.mustCall(function(error) { + assert.strictEqual(error && error.message, 'Not running'); + })); + })); })); })); req.on('error', function(e) { - console.log(e.stack); - process.exit(1); + common.fail(e.stack); }); req.end(); From 6aacef730096fc2b5850acec7ed280c396124e95 Mon Sep 17 00:00:00 2001 From: Matt Phillips Date: Thu, 1 Dec 2016 11:32:45 -0600 Subject: [PATCH 38/84] test: check for error on invalid signal Asserts that an error should be thrown when an invalid signal is passed to process.kill(). PR-URL: https://github.com/nodejs/node/pull/10026 Reviewed-By: Ben Noordhuis Reviewed-By: Jeremiah Senkpiel Reviewed-By: Colin Ihrig --- test/parallel/test-process-kill-pid.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/parallel/test-process-kill-pid.js b/test/parallel/test-process-kill-pid.js index 5c11be2903c4a7..ee0554c32083f6 100644 --- a/test/parallel/test-process-kill-pid.js +++ b/test/parallel/test-process-kill-pid.js @@ -24,6 +24,10 @@ assert.throws(function() { process.kill(+'not a number'); }, TypeError); assert.throws(function() { process.kill(1 / 0); }, TypeError); assert.throws(function() { process.kill(-1 / 0); }, TypeError); +// Test that kill throws an error for invalid signal + +assert.throws(function() { process.kill(1, 'test'); }, Error); + // Test kill argument processing in valid cases. // // Monkey patch _kill so that we don't actually send any signals, particularly From a8137dd06dc7351a7690f94638a644d09f8dcf92 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Sat, 3 Dec 2016 07:56:01 +0100 Subject: [PATCH 39/84] tools: add macosx-firwall script to avoid popups Currently, there are a number of popups that get displayed when running the tests asking to accept incoming network connections. Rules can be added manually to the socket firewall on Mac OS X but getting this right might not be obvious and quite a lot of time can be wasted trying to get the rules right. This script hopes to simplify things a little so that it can be re-run when needed. The script should be runnable from both the projects root directory and from the tools directory, for example: $ sudo ./tools/macosx-firewall.sh Fixes: https://github.com/nodejs/node/issues/8911 PR-URL: https://github.com/nodejs/node/pull/10114 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Gibson Fahnestock Reviewed-By: Colin Ihrig --- BUILDING.md | 9 ++++++++ tools/macosx-firewall.sh | 48 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100755 tools/macosx-firewall.sh diff --git a/BUILDING.md b/BUILDING.md index 5051da343353b3..b1060744f75d24 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -24,6 +24,15 @@ On OS X, you will also need: this under the menu `Xcode -> Preferences -> Downloads` * This step will install `gcc` and the related toolchain containing `make` +* You may want to setup [firewall rules](tools/macosx-firewall.sh) to avoid +popups asking to accept incoming network connections when running tests: + +```console +$ sudo ./tools/macosx-firewall.sh +``` +Running this script will add rules for the executable `node` in the out +directory and the symbolic `node` link in the projects root directory. + On FreeBSD and OpenBSD, you may also need: * libexecinfo diff --git a/tools/macosx-firewall.sh b/tools/macosx-firewall.sh new file mode 100755 index 00000000000000..c1de916e3c87ea --- /dev/null +++ b/tools/macosx-firewall.sh @@ -0,0 +1,48 @@ +#!/bin/bash +# Script that adds rules to Mac OS X Socket Firewall to avoid +# popups asking to accept incoming network connections when +# running tests. +SFW="/usr/libexec/ApplicationFirewall/socketfilterfw" +TOOLSDIR="`dirname \"$0\"`" +TOOLSDIR="`( cd \"$TOOLSDIR\" && pwd) `" +ROOTDIR="`( cd \"$TOOLSDIR/..\" && pwd) `" +OUTDIR="$TOOLSDIR/../out" +# Using cd and pwd here so that the path used for socketfilterfw does not +# contain a '..', which seems to cause the rules to be incorrectly added +# and they are not removed when this script is re-run. Instead the new +# rules are simply appended. By using pwd we can get the full path +# without '..' and things work as expected. +OUTDIR="`( cd \"$OUTDIR\" && pwd) `" +NODE_RELEASE="$OUTDIR/Release/node" +NODE_DEBUG="$OUTDIR/Debug/node" +NODE_LINK="$ROOTDIR/node" +CCTEST_RELEASE="$OUTDIR/Release/cctest" +CCTEST_DEBUG="$OUTDIR/Debug/cctest" + +if [ -f $SFW ]; +then + # Duplicating these commands on purpose as the symbolic link node might be + # linked to either out/Debug/node or out/Release/node depending on the + # BUILDTYPE. + $SFW --remove "$NODE_DEBUG" + $SFW --remove "$NODE_DEBUG" + $SFW --remove "$NODE_RELEASE" + $SFW --remove "$NODE_RELEASE" + $SFW --remove "$NODE_LINK" + $SFW --remove "$CCTEST_DEBUG" + $SFW --remove "$CCTEST_RELEASE" + + $SFW --add "$NODE_DEBUG" + $SFW --add "$NODE_RELEASE" + $SFW --add "$NODE_LINK" + $SFW --add "$CCTEST_DEBUG" + $SFW --add "$CCTEST_RELEASE" + + $SFW --unblock "$NODE_DEBUG" + $SFW --unblock "$NODE_RELEASE" + $SFW --unblock "$NODE_LINK" + $SFW --unblock "$CCTEST_DEBUG" + $SFW --unblock "$CCTEST_RELEASE" +else + echo "SocketFirewall not found in location: $SFW" +fi From def6dfb62cd40f4966783848c0fbee261f3933cc Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 6 Dec 2016 18:09:11 +0100 Subject: [PATCH 40/84] test: set stdin too for pseudo-tty tests Ref: https://github.com/nodejs/node/pull/10037 Ref: https://github.com/nodejs/node/pull/10146 PR-URL: https://github.com/nodejs/node/pull/10149 Reviewed-By: Jeremiah Senkpiel Reviewed-By: James M Snell Reviewed-By: Italo A. Casas Reviewed-By: Ben Noordhuis --- tools/test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tools/test.py b/tools/test.py index 4a272475ccfffc..c8edca2b991ad2 100755 --- a/tools/test.py +++ b/tools/test.py @@ -683,11 +683,12 @@ def Execute(args, context, timeout=None, env={}, faketty=False): if faketty: import pty (out_master, fd_out) = pty.openpty() - fd_err = fd_out + fd_in = fd_err = fd_out pty_out = out_master else: (fd_out, outname) = tempfile.mkstemp() (fd_err, errname) = tempfile.mkstemp() + fd_in = 0 pty_out = None # Extend environment @@ -699,6 +700,7 @@ def Execute(args, context, timeout=None, env={}, faketty=False): context, timeout, args = args, + stdin = fd_in, stdout = fd_out, stderr = fd_err, env = env_copy, From b8fc9a3c6af2aac2941144e93c24572bb706932d Mon Sep 17 00:00:00 2001 From: Jonathan Darling Date: Tue, 6 Dec 2016 10:20:08 -0600 Subject: [PATCH 41/84] test: add stdin-setrawmode.out file Adds an accompanying .out file for test/pseudo-tty/stdin-setrawmode.js. The test was originally merged without this file and an astute observer found that it was causing an error message. See discussion at https://github.com/nodejs/node/pull/10037. PR-URL: https://github.com/nodejs/node/pull/10149 Reviewed-By: Jeremiah Senkpiel Reviewed-By: James M Snell Reviewed-By: Italo A. Casas Reviewed-By: Ben Noordhuis --- test/pseudo-tty/stdin-setrawmode.out | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 test/pseudo-tty/stdin-setrawmode.out diff --git a/test/pseudo-tty/stdin-setrawmode.out b/test/pseudo-tty/stdin-setrawmode.out new file mode 100644 index 00000000000000..e69de29bb2d1d6 From bc335c0a8d68164fd978192409ab6c4986cfe374 Mon Sep 17 00:00:00 2001 From: joyeecheung Date: Wed, 7 Dec 2016 01:31:53 +0800 Subject: [PATCH 42/84] doc: buffer allocation throws for negative size PR-URL: https://github.com/nodejs/node/pull/10151 Reviewed-By: Jeremiah Senkpiel Reviewed-By: Anna Henningsen Reviewed-By: Sam Roberts Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- doc/api/buffer.md | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/doc/api/buffer.md b/doc/api/buffer.md index 016aa4ed8cb3bb..b9f8369335a0d6 100644 --- a/doc/api/buffer.md +++ b/doc/api/buffer.md @@ -392,9 +392,9 @@ deprecated: v6.0.0 * `size` {Integer} The desired length of the new `Buffer` -Allocates a new `Buffer` of `size` bytes. The `size` must be less than or equal -to the value of [`buffer.kMaxLength`]. Otherwise, a [`RangeError`] is thrown. -A zero-length `Buffer` will be created if `size <= 0`. +Allocates a new `Buffer` of `size` bytes. If the `size` is larger than +[`buffer.kMaxLength`] or smaller than 0, a [`RangeError`] will be thrown. +A zero-length `Buffer` will be created if `size` is 0. Unlike [`ArrayBuffers`][`ArrayBuffer`], the underlying memory for `Buffer` instances created in this way is *not initialized*. The contents of a newly created `Buffer` @@ -470,9 +470,9 @@ const buf = Buffer.alloc(5); console.log(buf); ``` -The `size` must be less than or equal to the value of [`buffer.kMaxLength`]. -Otherwise, a [`RangeError`] is thrown. A zero-length `Buffer` will be created if -`size <= 0`. +Allocates a new `Buffer` of `size` bytes. If the `size` is larger than +[`buffer.kMaxLength`] or smaller than 0, a [`RangeError`] will be thrown. +A zero-length `Buffer` will be created if `size` is 0. If `fill` is specified, the allocated `Buffer` will be initialized by calling [`buf.fill(fill)`][`buf.fill()`]. @@ -511,9 +511,9 @@ added: v5.10.0 * `size` {Integer} The desired length of the new `Buffer` -Allocates a new *non-zero-filled* `Buffer` of `size` bytes. The `size` must -be less than or equal to the value of [`buffer.kMaxLength`]. Otherwise, a -[`RangeError`] is thrown. A zero-length `Buffer` will be created if `size <= 0`. +Allocates a new `Buffer` of `size` bytes. If the `size` is larger than +[`buffer.kMaxLength`] or smaller than 0, a [`RangeError`] will be thrown. +A zero-length `Buffer` will be created if `size` is 0. The underlying memory for `Buffer` instances created in this way is *not initialized*. The contents of the newly created `Buffer` are unknown and @@ -557,10 +557,9 @@ added: v5.10.0 * `size` {Integer} The desired length of the new `Buffer` -Allocates a new *non-zero-filled* and non-pooled `Buffer` of `size` bytes. The -`size` must be less than or equal to the value of [`buffer.kMaxLength`]. -Otherwise, a [`RangeError`] is thrown. A zero-length `Buffer` will be created if -`size <= 0`. +Allocates a new `Buffer` of `size` bytes. If the `size` is larger than +[`buffer.kMaxLength`] or smaller than 0, a [`RangeError`] will be thrown. +A zero-length `Buffer` will be created if `size` is 0. The underlying memory for `Buffer` instances created in this way is *not initialized*. The contents of the newly created `Buffer` are unknown and @@ -2390,9 +2389,9 @@ deprecated: v6.0.0 * `size` {Integer} The desired length of the new `SlowBuffer` -Allocates a new `SlowBuffer` of `size` bytes. The `size` must be less than -or equal to the value of [`buffer.kMaxLength`]. Otherwise, a [`RangeError`] is -thrown. A zero-length `Buffer` will be created if `size <= 0`. +Allocates a new `Buffer` of `size` bytes. If the `size` is larger than +[`buffer.kMaxLength`] or smaller than 0, a [`RangeError`] will be thrown. +A zero-length `Buffer` will be created if `size` is 0. The underlying memory for `SlowBuffer` instances is *not initialized*. The contents of a newly created `SlowBuffer` are unknown and could contain From f9aadfbc5adb3b9ee9357636d846dff1556b9acc Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Fri, 18 Nov 2016 13:52:22 -0800 Subject: [PATCH 43/84] inspector: move options parsing As inspector functionality expands, more options will need to be added. Currently this requires changing adding function arguments, etc. This change packs the veriables into a single class that can be extended without changing APIs. PR-URL: https://github.com/nodejs/node/pull/9691 Reviewed-By: Ben Noordhuis --- node.gyp | 2 + src/debug-agent.cc | 16 ++-- src/debug-agent.h | 6 +- src/inspector_agent.cc | 25 +++--- src/inspector_agent.h | 5 +- src/node.cc | 165 ++++++++------------------------------ src/node_debug_options.cc | 144 +++++++++++++++++++++++++++++++++ src/node_debug_options.h | 51 ++++++++++++ 8 files changed, 260 insertions(+), 154 deletions(-) create mode 100644 src/node_debug_options.cc create mode 100644 src/node_debug_options.h diff --git a/node.gyp b/node.gyp index e5f02d73086a09..8f5510d44c7366 100644 --- a/node.gyp +++ b/node.gyp @@ -154,6 +154,7 @@ 'src/node_config.cc', 'src/node_constants.cc', 'src/node_contextify.cc', + 'src/node_debug_options.cc', 'src/node_file.cc', 'src/node_http_parser.cc', 'src/node_javascript.cc', @@ -194,6 +195,7 @@ 'src/node.h', 'src/node_buffer.h', 'src/node_constants.h', + 'src/node_debug_options.h', 'src/node_file.h', 'src/node_http_parser.h', 'src/node_internals.h', diff --git a/src/debug-agent.cc b/src/debug-agent.cc index 2766702525d088..2d8ed8afc980ec 100644 --- a/src/debug-agent.cc +++ b/src/debug-agent.cc @@ -50,7 +50,6 @@ using v8::Value; Agent::Agent(Environment* env) : state_(kNone), - port_(5858), wait_(false), parent_env_(env), child_env_(nullptr), @@ -69,7 +68,7 @@ Agent::~Agent() { } -bool Agent::Start(const char* host, int port, bool wait) { +bool Agent::Start(const DebugOptions& options) { int err; if (state_ == kRunning) @@ -85,9 +84,8 @@ bool Agent::Start(const char* host, int port, bool wait) { goto async_init_failed; uv_unref(reinterpret_cast(&child_signal_)); - host_ = host; - port_ = port; - wait_ = wait; + options_ = options; + wait_ = options_.wait_for_connect(); err = uv_thread_create(&thread_, reinterpret_cast(ThreadCb), @@ -210,9 +208,11 @@ void Agent::InitAdaptor(Environment* env) { api->Set(String::NewFromUtf8(isolate, "host", NewStringType::kNormal).ToLocalChecked(), - String::NewFromUtf8(isolate, host_.data(), NewStringType::kNormal, - host_.size()).ToLocalChecked()); - api->Set(String::NewFromUtf8(isolate, "port"), Integer::New(isolate, port_)); + String::NewFromUtf8(isolate, options_.host_name().data(), + NewStringType::kNormal, + options_.host_name().size()).ToLocalChecked()); + api->Set(String::NewFromUtf8(isolate, "port"), + Integer::New(isolate, options_.port())); env->process_object()->Set(String::NewFromUtf8(isolate, "_debugAPI"), api); api_.Reset(env->isolate(), api); diff --git a/src/debug-agent.h b/src/debug-agent.h index 783403cf0920d1..0faa25f12470a9 100644 --- a/src/debug-agent.h +++ b/src/debug-agent.h @@ -25,6 +25,7 @@ #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS #include "node_mutex.h" +#include "node_debug_options.h" #include "util.h" #include "util-inl.h" #include "uv.h" @@ -76,7 +77,7 @@ class Agent { typedef void (*DispatchHandler)(node::Environment* env); // Start the debugger agent thread - bool Start(const char* host, int port, bool wait); + bool Start(const DebugOptions& options); // Listen for debug events void Enable(); // Stop the debugger agent @@ -114,9 +115,8 @@ class Agent { }; State state_; + DebugOptions options_; - std::string host_; - int port_; bool wait_; uv_sem_t start_sem_; diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index fc478c49a09d61..aeb8339332d4c3 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -132,7 +132,8 @@ class AgentImpl { ~AgentImpl(); // Start the inspector agent thread - bool Start(v8::Platform* platform, const char* path, int port, bool wait); + bool Start(v8::Platform* platform, const char* path, + const DebugOptions& options); // Stop the inspector agent void Stop(); @@ -169,6 +170,7 @@ class AgentImpl { void NotifyMessageReceived(); State ToState(State state); + DebugOptions options_; uv_sem_t start_sem_; ConditionVariable incoming_message_cond_; Mutex state_lock_; @@ -176,8 +178,6 @@ class AgentImpl { uv_loop_t child_loop_; InspectorAgentDelegate* delegate_; - - int port_; bool wait_; bool shutting_down_; State state_; @@ -194,6 +194,8 @@ class AgentImpl { InspectorSocketServer* server_; std::string script_name_; + std::string script_path_; + const std::string id_; friend class ChannelImpl; friend class DispatchOnInspectorBackendTask; @@ -318,7 +320,6 @@ class V8NodeInspector : public v8_inspector::V8InspectorClient { }; AgentImpl::AgentImpl(Environment* env) : delegate_(nullptr), - port_(0), wait_(false), shutting_down_(false), state_(State::kNew), @@ -409,7 +410,10 @@ void InspectorWrapConsoleCall(const v8::FunctionCallbackInfo& args) { } bool AgentImpl::Start(v8::Platform* platform, const char* path, - int port, bool wait) { + const DebugOptions& options) { + options_ = options; + wait_ = options.wait_for_connect(); + auto env = parent_env_; inspector_ = new V8NodeInspector(this, env, platform); platform_ = platform; @@ -421,9 +425,6 @@ bool AgentImpl::Start(v8::Platform* platform, const char* path, int err = uv_loop_init(&child_loop_); CHECK_EQ(err, 0); - port_ = port; - wait_ = wait; - err = uv_thread_create(&thread_, AgentImpl::ThreadCbIO, this); CHECK_EQ(err, 0); uv_sem_wait(&start_sem_); @@ -433,7 +434,7 @@ bool AgentImpl::Start(v8::Platform* platform, const char* path, return false; } state_ = State::kAccepting; - if (wait) { + if (options_.wait_for_connect()) { DispatchMessages(); } return true; @@ -561,7 +562,7 @@ void AgentImpl::WorkerRunIO() { } InspectorAgentDelegate delegate(this, script_path, script_name_, wait_); delegate_ = &delegate; - InspectorSocketServer server(&delegate, port_); + InspectorSocketServer server(&delegate, options_.port()); if (!server.Start(&child_loop_)) { fprintf(stderr, "Unable to open devtools socket: %s\n", uv_strerror(err)); state_ = State::kError; // Safe, main thread is waiting on semaphore @@ -681,8 +682,8 @@ Agent::~Agent() { } bool Agent::Start(v8::Platform* platform, const char* path, - int port, bool wait) { - return impl->Start(platform, path, port, wait); + const DebugOptions& options) { + return impl->Start(platform, path, options); } void Agent::Stop() { diff --git a/src/inspector_agent.h b/src/inspector_agent.h index b31c77496b3d70..9cc2fa676d4d13 100644 --- a/src/inspector_agent.h +++ b/src/inspector_agent.h @@ -7,6 +7,8 @@ #error("This header can only be used when inspector is enabled") #endif +#include "node_debug_options.h" + // Forward declaration to break recursive dependency chain with src/env.h. namespace node { class Environment; @@ -31,7 +33,8 @@ class Agent { ~Agent(); // Start the inspector agent thread - bool Start(v8::Platform* platform, const char* path, int port, bool wait); + bool Start(v8::Platform* platform, const char* path, + const DebugOptions& options); // Stop the inspector agent void Stop(); diff --git a/src/node.cc b/src/node.cc index e87bddcb6ff2cc..3147a90d3e3844 100644 --- a/src/node.cc +++ b/src/node.cc @@ -7,6 +7,7 @@ #include "node_version.h" #include "node_internals.h" #include "node_revert.h" +#include "node_debug_options.h" #if defined HAVE_PERFCTR #include "node_counters.h" @@ -140,17 +141,6 @@ static bool track_heap_objects = false; static const char* eval_string = nullptr; static unsigned int preload_module_count = 0; static const char** preload_modules = nullptr; -#if HAVE_INSPECTOR -static bool use_inspector = false; -#else -static const bool use_inspector = false; -#endif -static bool use_debug_agent = false; -static bool debug_wait_connect = false; -static std::string* debug_host; // coverity[leaked_storage] -static const int default_debugger_port = 5858; -static const int default_inspector_port = 9229; -static int debug_port = -1; static const int v8_default_thread_pool_size = 4; static int v8_thread_pool_size = v8_default_thread_pool_size; static bool prof_process = false; @@ -197,6 +187,8 @@ static uv_async_t dispatch_debug_messages_async; static Mutex node_isolate_mutex; static v8::Isolate* node_isolate; +static node::DebugOptions debug_options; + static struct { #if NODE_USE_V8_PLATFORM void Initialize(int thread_pool_size) { @@ -213,14 +205,12 @@ static struct { platform_ = nullptr; } - bool StartInspector(Environment *env, const char* script_path, - int port, bool wait) { #if HAVE_INSPECTOR - return env->inspector_agent()->Start(platform_, script_path, port, wait); -#else - return true; -#endif // HAVE_INSPECTOR + bool StartInspector(Environment *env, const char* script_path, + const node::DebugOptions& options) { + return env->inspector_agent()->Start(platform_, script_path, options); } +#endif // HAVE_INSPECTOR v8::Platform* platform_; #else // !NODE_USE_V8_PLATFORM @@ -2520,7 +2510,7 @@ void FatalException(Isolate* isolate, if (exit_code) { #if HAVE_INSPECTOR - if (use_inspector) { + if (debug_options.inspector_enabled()) { env->inspector_agent()->FatalException(error, message); } #endif @@ -2924,17 +2914,14 @@ static Local GetFeatures(Environment* env) { static void DebugPortGetter(Local property, const PropertyCallbackInfo& info) { - int port = debug_port; - if (port < 0) - port = use_inspector ? default_inspector_port : default_debugger_port; - info.GetReturnValue().Set(port); + info.GetReturnValue().Set(debug_options.port()); } static void DebugPortSetter(Local property, Local value, const PropertyCallbackInfo& info) { - debug_port = value->Int32Value(); + debug_options.set_port(value->Int32Value()); } @@ -3277,7 +3264,7 @@ void SetupProcessObject(Environment* env, } // --debug-brk - if (debug_wait_connect) { + if (debug_options.wait_for_connect()) { READONLY_PROPERTY(process, "_debugWaitConnect", True(env->isolate())); } @@ -3469,90 +3456,6 @@ void LoadEnvironment(Environment* env) { f->Call(Null(env->isolate()), 1, &arg); } - -static void PrintHelp(); - -static bool ParseDebugOpt(const char* arg) { - const char* port = nullptr; - - if (!strcmp(arg, "--debug")) { - use_debug_agent = true; - } else if (!strncmp(arg, "--debug=", sizeof("--debug=") - 1)) { - use_debug_agent = true; - port = arg + sizeof("--debug=") - 1; - } else if (!strcmp(arg, "--debug-brk")) { - use_debug_agent = true; - debug_wait_connect = true; - } else if (!strncmp(arg, "--debug-brk=", sizeof("--debug-brk=") - 1)) { - use_debug_agent = true; - debug_wait_connect = true; - port = arg + sizeof("--debug-brk=") - 1; - } else if (!strncmp(arg, "--debug-port=", sizeof("--debug-port=") - 1)) { - // XXX(bnoordhuis) Misnomer, configures port and listen address. - port = arg + sizeof("--debug-port=") - 1; -#if HAVE_INSPECTOR - // Specifying both --inspect and --debug means debugging is on, using Chromium - // inspector. - } else if (!strcmp(arg, "--inspect")) { - use_debug_agent = true; - use_inspector = true; - } else if (!strncmp(arg, "--inspect=", sizeof("--inspect=") - 1)) { - use_debug_agent = true; - use_inspector = true; - port = arg + sizeof("--inspect=") - 1; -#else - } else if (!strncmp(arg, "--inspect", sizeof("--inspect") - 1)) { - fprintf(stderr, - "Inspector support is not available with this Node.js build\n"); - return false; -#endif - } else { - return false; - } - - if (port == nullptr) { - return true; - } - - // FIXME(bnoordhuis) Move IPv6 address parsing logic to lib/net.js. - // It seems reasonable to support [address]:port notation - // in net.Server#listen() and net.Socket#connect(). - const size_t port_len = strlen(port); - if (port[0] == '[' && port[port_len - 1] == ']') { - debug_host = new std::string(port + 1, port_len - 2); - return true; - } - - const char* const colon = strrchr(port, ':'); - if (colon == nullptr) { - // Either a port number or a host name. Assume that - // if it's not all decimal digits, it's a host name. - for (size_t n = 0; port[n] != '\0'; n += 1) { - if (port[n] < '0' || port[n] > '9') { - debug_host = new std::string(port); - return true; - } - } - } else { - const bool skip = (colon > port && port[0] == '[' && colon[-1] == ']'); - debug_host = new std::string(port + skip, colon - skip); - } - - char* endptr; - errno = 0; - const char* const digits = colon != nullptr ? colon + 1 : port; - const long result = strtol(digits, &endptr, 10); // NOLINT(runtime/int) - if (errno != 0 || *endptr != '\0' || result < 1024 || result > 65535) { - fprintf(stderr, "Debug port must be in range 1024 to 65535.\n"); - PrintHelp(); - exit(12); - } - - debug_port = static_cast(result); - - return true; -} - static void PrintHelp() { // XXX: If you add an option here, please also add it to doc/node.1 and // doc/api/cli.md @@ -3666,8 +3569,8 @@ static void ParseArgs(int* argc, const char* const arg = argv[index]; unsigned int args_consumed = 1; - if (ParseDebugOpt(arg)) { - // Done, consumed by ParseDebugOpt(). + if (debug_options.ParseOption(arg)) { + // Done, consumed by DebugOptions::ParseOption(). } else if (strcmp(arg, "--version") == 0 || strcmp(arg, "-v") == 0) { printf("%s\n", NODE_VERSION); exit(0); @@ -3809,22 +3712,21 @@ static void DispatchMessagesDebugAgentCallback(Environment* env) { } -static void StartDebug(Environment* env, const char* path, bool wait) { +static void StartDebug(Environment* env, const char* path, + DebugOptions debug_options) { CHECK(!debugger_running); - if (use_inspector) { - debugger_running = v8_platform.StartInspector(env, path, - debug_port >= 0 ? debug_port : default_inspector_port, wait); - } else { +#if HAVE_INSPECTOR + if (debug_options.inspector_enabled()) + debugger_running = v8_platform.StartInspector(env, path, debug_options); +#endif // HAVE_INSPECTOR + if (debug_options.debugger_enabled()) { env->debugger_agent()->set_dispatch_handler( DispatchMessagesDebugAgentCallback); - const char* host = debug_host ? debug_host->c_str() : "127.0.0.1"; - int port = debug_port >= 0 ? debug_port : default_debugger_port; - debugger_running = - env->debugger_agent()->Start(host, port, wait); + debugger_running = env->debugger_agent()->Start(debug_options); if (debugger_running == false) { - fprintf(stderr, "Starting debugger on %s:%d failed\n", host, port); + fprintf(stderr, "Starting debugger on %s:%d failed\n", + debug_options.host_name().c_str(), debug_options.port()); fflush(stderr); - return; } } } @@ -3834,7 +3736,7 @@ static void StartDebug(Environment* env, const char* path, bool wait) { static void EnableDebug(Environment* env) { CHECK(debugger_running); - if (use_inspector) { + if (!debug_options.debugger_enabled()) { return; } @@ -3875,8 +3777,8 @@ static void DispatchDebugMessagesAsyncCallback(uv_async_t* handle) { HandleScope scope(isolate); Environment* env = Environment::GetCurrent(isolate); Context::Scope context_scope(env->context()); - - StartDebug(env, nullptr, false); + debug_options.EnableDebugAgent(DebugAgentType::kDebugger); + StartDebug(env, nullptr, debug_options); EnableDebug(env); } @@ -4130,7 +4032,7 @@ static void DebugEnd(const FunctionCallbackInfo& args) { if (debugger_running) { Environment* env = Environment::GetCurrent(args); #if HAVE_INSPECTOR - if (use_inspector) { + if (debug_options.inspector_enabled()) { env->inspector_agent()->Stop(); } else { #endif @@ -4302,7 +4204,7 @@ void Init(int* argc, const char no_typed_array_heap[] = "--typed_array_max_size_in_heap=0"; V8::SetFlagsFromString(no_typed_array_heap, sizeof(no_typed_array_heap) - 1); - if (!use_debug_agent) { + if (!debug_options.debugger_enabled() && !debug_options.inspector_enabled()) { RegisterDebugSignalHandler(); } @@ -4419,11 +4321,14 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, Environment env(isolate_data, context); env.Start(argc, argv, exec_argc, exec_argv, v8_is_profiling); + bool debug_enabled = + debug_options.debugger_enabled() || debug_options.inspector_enabled(); + // Start debug agent when argv has --debug - if (use_debug_agent) { + if (debug_enabled) { const char* path = argc > 1 ? argv[1] : nullptr; - StartDebug(&env, path, debug_wait_connect); - if (use_inspector && !debugger_running) + StartDebug(&env, path, debug_options); + if (debug_options.debugger_enabled() && !debugger_running) return 12; // Signal internal error. } @@ -4435,7 +4340,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data, env.set_trace_sync_io(trace_sync_io); // Enable debugger - if (use_debug_agent) + if (debug_enabled) EnableDebug(&env); { diff --git a/src/node_debug_options.cc b/src/node_debug_options.cc new file mode 100644 index 00000000000000..5681e3d46edaca --- /dev/null +++ b/src/node_debug_options.cc @@ -0,0 +1,144 @@ +#include "node_debug_options.h" + +#include +#include +#include +#include "util.h" + +namespace node { + +namespace { +const int default_debugger_port = 5858; +const int default_inspector_port = 9229; + +inline std::string remove_brackets(const std::string& host) { + if (!host.empty() && host.front() == '[' && host.back() == ']') + return host.substr(1, host.size() - 2); + else + return host; +} + +int parse_and_validate_port(const std::string& port) { + char* endptr; + errno = 0; + const long result = strtol(port.c_str(), &endptr, 10); // NOLINT(runtime/int) + if (errno != 0 || *endptr != '\0'|| result < 1024 || result > 65535) { + fprintf(stderr, "Debug port must be in range 1024 to 65535.\n"); + exit(12); + } + return static_cast(result); +} + +std::pair split_host_port(const std::string& arg) { + // IPv6, no port + std::string host = remove_brackets(arg); + if (host.length() < arg.length()) + return {host, -1}; + + size_t colon = arg.rfind(':'); + if (colon == std::string::npos) { + // Either a port number or a host name. Assume that + // if it's not all decimal digits, it's a host name. + for (char c : arg) { + if (c < '0' || c > '9') { + return {arg, -1}; + } + } + return {"", parse_and_validate_port(arg)}; + } + return std::make_pair(remove_brackets(arg.substr(0, colon)), + parse_and_validate_port(arg.substr(colon + 1))); +} + +} // namespace + +DebugOptions::DebugOptions() : debugger_enabled_(false), +#if HAVE_INSPECTOR + inspector_enabled_(false), +#endif // HAVE_INSPECTOR + wait_connect_(false), http_enabled_(false), + host_name_("127.0.0.1"), port_(-1) { } + +void DebugOptions::EnableDebugAgent(DebugAgentType tool) { + switch (tool) { +#if HAVE_INSPECTOR + case DebugAgentType::kInspector: + inspector_enabled_ = true; + debugger_enabled_ = true; + break; +#endif // HAVE_INSPECTOR + case DebugAgentType::kDebugger: + debugger_enabled_ = true; + break; + case DebugAgentType::kNone: + break; + } +} + +bool DebugOptions::ParseOption(const std::string& option) { + bool enable_inspector = false; + bool has_argument = false; + std::string option_name; + std::string argument; + + auto pos = option.find("="); + if (pos == std::string::npos) { + option_name = option; + } else { + has_argument = true; + option_name = option.substr(0, pos); + argument = option.substr(pos + 1); + } + + if (option_name == "--debug") { + debugger_enabled_ = true; + } else if (option_name == "--debug-brk") { + debugger_enabled_ = true; + wait_connect_ = true; + } else if (option_name == "--inspect") { + debugger_enabled_ = true; + enable_inspector = true; + } else if (option_name != "--debug-port" || !has_argument) { + return false; + } + + if (enable_inspector) { +#if HAVE_INSPECTOR + inspector_enabled_ = true; +#else + fprintf(stderr, + "Inspector support is not available with this Node.js build\n"); + return false; +#endif + } + + if (!has_argument) { + return true; + } + + // FIXME(bnoordhuis) Move IPv6 address parsing logic to lib/net.js. + // It seems reasonable to support [address]:port notation + // in net.Server#listen() and net.Socket#connect(). + std::pair host_port = split_host_port(argument); + if (!host_port.first.empty()) { + host_name_ = host_port.first; + } + if (host_port.second >= 0) { + port_ = host_port.second; + } + return true; +} + +int DebugOptions::port() const { + int port = port_; + if (port < 0) { +#if HAVE_INSPECTOR + port = inspector_enabled_ ? default_inspector_port : default_debugger_port; +#else + port = default_debugger_port; +#endif // HAVE_INSPECTOR + } + return port; +} + +} // namespace node diff --git a/src/node_debug_options.h b/src/node_debug_options.h new file mode 100644 index 00000000000000..d03bdb15497bbf --- /dev/null +++ b/src/node_debug_options.h @@ -0,0 +1,51 @@ +#ifndef SRC_NODE_DEBUG_OPTIONS_H_ +#define SRC_NODE_DEBUG_OPTIONS_H_ + +#include + +// Forward declaration to break recursive dependency chain with src/env.h. +namespace node { + +enum class DebugAgentType { + kNone, + kDebugger, +#if HAVE_INSPECTOR + kInspector +#endif // HAVE_INSPECTOR +}; + +class DebugOptions { + public: + DebugOptions(); + bool ParseOption(const std::string& option); + bool debugger_enabled() const { + return debugger_enabled_ && !inspector_enabled(); + } + bool inspector_enabled() const { +#if HAVE_INSPECTOR + return inspector_enabled_; +#else + return false; +#endif // HAVE_INSPECTOR + } + void EnableDebugAgent(DebugAgentType type); + bool ToolsServerEnabled(); + bool wait_for_connect() const { return wait_connect_; } + std::string host_name() const { return host_name_; } + int port() const; + void set_port(int port) { port_ = port; } + + private: + bool debugger_enabled_; +#if HAVE_INSPECTOR + bool inspector_enabled_; +#endif // HAVE_INSPECTOR + bool wait_connect_; + bool http_enabled_; + std::string host_name_; + int port_; +}; + +} // namespace node + +#endif // SRC_NODE_DEBUG_OPTIONS_H_ From d8c7534fcd46283e52c8c9324d11390c6218b809 Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Mon, 21 Nov 2016 09:38:19 -0800 Subject: [PATCH 44/84] inspector: stop relying on magic strings Inspector uses magical strings to communicate some events between main thread and transport thread. This change replaces those strings with enums that are more mainatainable (and remove unnecessary encodings/decodings) PR-URL: https://github.com/nodejs/node/pull/10159 Reviewed-By: Ben Noordhuis --- src/inspector_agent.cc | 106 ++++++++++++++++++++++++----------------- 1 file changed, 61 insertions(+), 45 deletions(-) diff --git a/src/inspector_agent.cc b/src/inspector_agent.cc index aeb8339332d4c3..6b19fee13cfc1d 100644 --- a/src/inspector_agent.cc +++ b/src/inspector_agent.cc @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -30,9 +31,6 @@ namespace { using v8_inspector::StringBuffer; using v8_inspector::StringView; -const char TAG_CONNECT[] = "#connect"; -const char TAG_DISCONNECT[] = "#disconnect"; - static const uint8_t PROTOCOL_JSON[] = { #include "v8_inspector_protocol_json.h" // NOLINT(build/include_order) }; @@ -105,6 +103,14 @@ std::unique_ptr Utf8ToStringView(const std::string& message) { class V8NodeInspector; +enum class InspectorAction { + kStartSession, kEndSession, kSendMessage +}; + +enum class TransportAction { + kSendMessage, kStop +}; + class InspectorAgentDelegate: public node::inspector::SocketServerDelegate { public: InspectorAgentDelegate(AgentImpl* agent, const std::string& script_path, @@ -144,14 +150,16 @@ class AgentImpl { void FatalException(v8::Local error, v8::Local message); - void PostIncomingMessage(int session_id, const std::string& message); + void PostIncomingMessage(InspectorAction action, int session_id, + const std::string& message); void ResumeStartup() { uv_sem_post(&start_sem_); } private: + template using MessageQueue = - std::vector>>; + std::vector>>; enum class State { kNew, kAccepting, kConnected, kDone, kError }; static void ThreadCbIO(void* agent); @@ -162,10 +170,13 @@ class AgentImpl { void WorkerRunIO(); void SetConnected(bool connected); void DispatchMessages(); - void Write(int session_id, const StringView& message); - bool AppendMessage(MessageQueue* vector, int session_id, - std::unique_ptr buffer); - void SwapBehindLock(MessageQueue* vector1, MessageQueue* vector2); + void Write(TransportAction action, int session_id, const StringView& message); + template + bool AppendMessage(MessageQueue* vector, ActionType action, + int session_id, std::unique_ptr buffer); + template + void SwapBehindLock(MessageQueue* vector1, + MessageQueue* vector2); void WaitForFrontendMessage(); void NotifyMessageReceived(); State ToState(State state); @@ -187,8 +198,8 @@ class AgentImpl { uv_async_t io_thread_req_; V8NodeInspector* inspector_; v8::Platform* platform_; - MessageQueue incoming_message_queue_; - MessageQueue outgoing_message_queue_; + MessageQueue incoming_message_queue_; + MessageQueue outgoing_message_queue_; bool dispatching_messages_; int session_id_; InspectorSocketServer* server_; @@ -238,7 +249,7 @@ class ChannelImpl final : public v8_inspector::V8Inspector::Channel { void flushProtocolNotifications() override { } void sendMessageToFrontend(const StringView& message) { - agent_->Write(agent_->session_id_, message); + agent_->Write(TransportAction::kSendMessage, agent_->session_id_, message); } AgentImpl* const agent_; @@ -457,9 +468,7 @@ bool AgentImpl::IsStarted() { void AgentImpl::WaitForDisconnect() { if (state_ == State::kConnected) { shutting_down_ = true; - // Gives a signal to stop accepting new connections - // TODO(eugeneo): Introduce an API with explicit request names. - Write(0, StringView()); + Write(TransportAction::kStop, 0, StringView()); fprintf(stderr, "Waiting for the debugger to disconnect...\n"); fflush(stderr); inspector_->runMessageLoopOnPause(0); @@ -534,15 +543,17 @@ void AgentImpl::ThreadCbIO(void* agent) { // static void AgentImpl::WriteCbIO(uv_async_t* async) { AgentImpl* agent = static_cast(async->data); - MessageQueue outgoing_messages; + MessageQueue outgoing_messages; agent->SwapBehindLock(&agent->outgoing_message_queue_, &outgoing_messages); - for (const MessageQueue::value_type& outgoing : outgoing_messages) { - StringView view = outgoing.second->string(); - if (view.length() == 0) { + for (const auto& outgoing : outgoing_messages) { + switch (std::get<0>(outgoing)) { + case TransportAction::kStop: agent->server_->Stop(nullptr); - } else { - agent->server_->Send(outgoing.first, - StringViewToUtf8(outgoing.second->string())); + break; + case TransportAction::kSendMessage: + std::string message = StringViewToUtf8(std::get<2>(outgoing)->string()); + agent->server_->Send(std::get<1>(outgoing), message); + break; } } } @@ -586,22 +597,26 @@ void AgentImpl::WorkerRunIO() { server_ = nullptr; } -bool AgentImpl::AppendMessage(MessageQueue* queue, int session_id, +template +bool AgentImpl::AppendMessage(MessageQueue* queue, + ActionType action, int session_id, std::unique_ptr buffer) { Mutex::ScopedLock scoped_lock(state_lock_); bool trigger_pumping = queue->empty(); - queue->push_back(std::make_pair(session_id, std::move(buffer))); + queue->push_back(std::make_tuple(action, session_id, std::move(buffer))); return trigger_pumping; } -void AgentImpl::SwapBehindLock(MessageQueue* vector1, MessageQueue* vector2) { +template +void AgentImpl::SwapBehindLock(MessageQueue* vector1, + MessageQueue* vector2) { Mutex::ScopedLock scoped_lock(state_lock_); vector1->swap(*vector2); } -void AgentImpl::PostIncomingMessage(int session_id, +void AgentImpl::PostIncomingMessage(InspectorAction action, int session_id, const std::string& message) { - if (AppendMessage(&incoming_message_queue_, session_id, + if (AppendMessage(&incoming_message_queue_, action, session_id, Utf8ToStringView(message))) { v8::Isolate* isolate = parent_env_->isolate(); platform_->CallOnForegroundThread(isolate, @@ -631,25 +646,21 @@ void AgentImpl::DispatchMessages() { if (dispatching_messages_) return; dispatching_messages_ = true; - MessageQueue tasks; + MessageQueue tasks; do { tasks.clear(); SwapBehindLock(&incoming_message_queue_, &tasks); - for (const MessageQueue::value_type& pair : tasks) { - StringView message = pair.second->string(); - std::string tag; - if (message.length() == sizeof(TAG_CONNECT) - 1 || - message.length() == sizeof(TAG_DISCONNECT) - 1) { - tag = StringViewToUtf8(message); - } - - if (tag == TAG_CONNECT) { + for (const auto& task : tasks) { + StringView message = std::get<2>(task)->string(); + switch (std::get<0>(task)) { + case InspectorAction::kStartSession: CHECK_EQ(State::kAccepting, state_); - session_id_ = pair.first; + session_id_ = std::get<1>(task); state_ = State::kConnected; fprintf(stderr, "Debugger attached.\n"); inspector_->connectFrontend(); - } else if (tag == TAG_DISCONNECT) { + break; + case InspectorAction::kEndSession: CHECK_EQ(State::kConnected, state_); if (shutting_down_) { state_ = State::kDone; @@ -658,8 +669,10 @@ void AgentImpl::DispatchMessages() { } inspector_->quitMessageLoopOnPause(); inspector_->disconnectFrontend(); - } else { + break; + case InspectorAction::kSendMessage: inspector_->dispatchMessageFromFrontend(message); + break; } } } while (!tasks.empty()); @@ -667,8 +680,9 @@ void AgentImpl::DispatchMessages() { dispatching_messages_ = false; } -void AgentImpl::Write(int session_id, const StringView& inspector_message) { - AppendMessage(&outgoing_message_queue_, session_id, +void AgentImpl::Write(TransportAction action, int session_id, + const StringView& inspector_message) { + AppendMessage(&outgoing_message_queue_, action, session_id, StringBuffer::create(inspector_message)); int err = uv_async_send(&io_thread_req_); CHECK_EQ(0, err); @@ -725,7 +739,8 @@ bool InspectorAgentDelegate::StartSession(int session_id, if (connected_) return false; connected_ = true; - agent_->PostIncomingMessage(session_id, TAG_CONNECT); + session_id_++; + agent_->PostIncomingMessage(InspectorAction::kStartSession, session_id, ""); return true; } @@ -742,12 +757,13 @@ void InspectorAgentDelegate::MessageReceived(int session_id, agent_->ResumeStartup(); } } - agent_->PostIncomingMessage(session_id, message); + agent_->PostIncomingMessage(InspectorAction::kSendMessage, session_id, + message); } void InspectorAgentDelegate::EndSession(int session_id) { connected_ = false; - agent_->PostIncomingMessage(session_id, TAG_DISCONNECT); + agent_->PostIncomingMessage(InspectorAction::kEndSession, session_id, ""); } std::vector InspectorAgentDelegate::GetTargetIds() { From 3d24856bc6096c1ac5bca9ae9aa81464adcbc7be Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Fri, 9 Dec 2016 20:52:01 +0200 Subject: [PATCH 45/84] doc: revert documenting argument form in repl.md It will be postponed for more careful evaluating in a separate PR. --- doc/api/repl.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 8d540a61846a75..e07633e132a56a 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -375,7 +375,7 @@ within the action function for commands registered using the added: v0.1.91 --> -* `options` {Object | String} +* `options` {Object} * `prompt` {String} The input prompt to display. Defaults to `> ` (with a trailing space). * `input` {Readable} The Readable stream from which REPL input will be read. @@ -416,8 +416,6 @@ added: v0.1.91 `SIGINT` is received, i.e. `Ctrl+C` is pressed. This cannot be used together with a custom `eval` function. Defaults to `false`. -If `options` is a string, then it specifies the input prompt. - The `repl.start()` method creates and starts a `repl.REPLServer` instance. ## The Node.js REPL From cffbb326a2893348f137f8c53b141a61274742b1 Mon Sep 17 00:00:00 2001 From: Jonathan Darling Date: Thu, 1 Dec 2016 10:47:46 -0600 Subject: [PATCH 46/84] doc: add note to parallelize make Adds a note to the BUILDING doc to encourage parallelizing make. When I first built node I didn't know this trick and thought that the build was just stuck in an infinite loop after waiting for 10 minutes. Refs: https://github.com/nodejs/node/issues/8286 Refs: https://github.com/nodejs/node/pull/9881 PR-URL: https://github.com/nodejs/node/pull/9961 Reviewed-By: Jeremiah Senkpiel Reviewed-By: James M Snell Reviewed-By: Gibson Fahnestock Reviewed-By: Ben Noordhuis Reviewed-By: Stephen Belanger --- BUILDING.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/BUILDING.md b/BUILDING.md index b1060744f75d24..b1962d53c2caf4 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -40,9 +40,17 @@ To build Node.js: ```console $ ./configure -$ make +$ make -j4 ``` +Running `make` with the `-j4` flag will cause it to run 4 compilation jobs +concurrently which may significantly reduce build time. The number after `-j` +can be changed to best suit the number of processor cores on your machine. If +you run into problems running `make` with concurrency, try running it without +the `-j4` flag. See the +[GNU Make Documentation](https://www.gnu.org/software/make/manual/html_node/Parallel.html) +for more information. + Note that the above requires that `python` resolve to Python 2.6 or 2.7 and not a newer version. To run the tests: From 0cd1f54fab3f35c6f53a6106cd9b5299c2c67413 Mon Sep 17 00:00:00 2001 From: Jonathan Darling Date: Tue, 6 Dec 2016 13:08:56 -0600 Subject: [PATCH 47/84] doc: standardizing on make -j4 Standardizes docs to use -j4 instead of -j8 as it appears to be the most inclusive recommendation based on discussion in https://github.com/nodejs/node/pull/9961. PR-URL: https://github.com/nodejs/node/pull/9961 Reviewed-By: Jeremiah Senkpiel Reviewed-By: James M Snell Reviewed-By: Gibson Fahnestock Reviewed-By: Ben Noordhuis Reviewed-By: Stephen Belanger --- .github/PULL_REQUEST_TEMPLATE.md | 2 +- CONTRIBUTING.md | 4 ++-- doc/guides/building-node-with-ninja.md | 2 +- doc/onboarding-extras.md | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 6bf44097890794..7b004801207054 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -9,7 +9,7 @@ Contributors guide: https://github.com/nodejs/node/blob/master/CONTRIBUTING.md ##### Checklist -- [ ] `make -j8 test` (UNIX), or `vcbuild test nosign` (Windows) passes +- [ ] `make -j4 test` (UNIX), or `vcbuild test nosign` (Windows) passes - [ ] tests and/or benchmarks are included - [ ] documentation is changed or added - [ ] commit message follows commit guidelines diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6f44949a31e0ca..830f2615528170 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -156,7 +156,7 @@ to see how they should be structured can also help. To run the tests on Unix / OS X: ```text -$ ./configure && make -j8 test +$ ./configure && make -j4 test ``` Windows: @@ -189,7 +189,7 @@ You can run tests directly with node: $ ./node ./test/parallel/test-stream2-transform.js ``` -Remember to recompile with `make -j8` in between test runs if you change +Remember to recompile with `make -j4` in between test runs if you change core modules. ### Step 6: Push diff --git a/doc/guides/building-node-with-ninja.md b/doc/guides/building-node-with-ninja.md index 027c267e2b243d..29b32c3d7277de 100644 --- a/doc/guides/building-node-with-ninja.md +++ b/doc/guides/building-node-with-ninja.md @@ -16,7 +16,7 @@ ninja: Entering directory `out/Release` The bottom line will change while building, showing the progress as `[finished/total]` build steps. This is useful output that `make` does not produce and is one of the benefits of using Ninja. -Also, Ninja will likely compile much faster than even `make -j8` (or `-j`). +Also, Ninja will likely compile much faster than even `make -j4` (or `-j`). ## Considerations diff --git a/doc/onboarding-extras.md b/doc/onboarding-extras.md index 7b7f60fd00705b..9e1a4a7ed7b14b 100644 --- a/doc/onboarding-extras.md +++ b/doc/onboarding-extras.md @@ -71,7 +71,7 @@ Please use these when possible / appropriate * major vs. everything else: run last versions tests against this version, if they pass, **probably** minor or patch * A breaking change helper ([full source](https://gist.github.com/chrisdickinson/ba532fa0e4e243fb7b44)): ```sh - git checkout $(git show -s --pretty='%T' $(git show-ref -d $(git describe --abbrev=0) | tail -n1 | awk '{print $1}')) -- test; make -j8 test + git checkout $(git show -s --pretty='%T' $(git show-ref -d $(git describe --abbrev=0) | tail -n1 | awk '{print $1}')) -- test; make -j4 test ``` From 4d11c2ce5ce9406fddf6397b03967b2f755e3b5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Thu, 8 Dec 2016 10:27:05 +0100 Subject: [PATCH 48/84] lib,test: use consistent operator linebreak style We have a tacit rule that for multiline statements, the operator should be placed before the linebreak. This commit commit fixes the few violations of this rule in the code base. This allows us to enable the corresponding ESLint rule. PR-URL: https://github.com/nodejs/node/pull/10178 Reviewed-By: Ben Noordhuis Reviewed-By: Roman Reiss Reviewed-By: Colin Ihrig Reviewed-By: Jeremiah Senkpiel Reviewed-By: Michael Dawson Reviewed-By: Rich Trott Reviewed-By: Teddy Katz Reviewed-By: Sakthipriyan Vairamani --- lib/timers.js | 4 +- test/parallel/test-preload.js | 52 +++++++++++----------- test/parallel/test-repl-domain.js | 4 +- test/parallel/test-tls-client-mindhsize.js | 4 +- 4 files changed, 32 insertions(+), 32 deletions(-) diff --git a/lib/timers.js b/lib/timers.js index 56af3a92d6bfe2..a55e265f29ffe9 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -473,8 +473,8 @@ function unrefdHandle() { // Make sure we clean up if the callback is no longer a function // even if the timer is an interval. - if (!this.owner._repeat - || typeof this.owner._onTimeout !== 'function') { + if (!this.owner._repeat || + typeof this.owner._onTimeout !== 'function') { this.owner.close(); } } diff --git a/test/parallel/test-preload.js b/test/parallel/test-preload.js index f849bc3dda3243..4ee564f0520cc5 100644 --- a/test/parallel/test-preload.js +++ b/test/parallel/test-preload.js @@ -32,27 +32,27 @@ const fixtureD = fixture('define-global.js'); const fixtureThrows = fixture('throws_error4.js'); // test preloading a single module works -childProcess.exec(nodeBinary + ' ' - + preloadOption([fixtureA]) + ' ' - + fixtureB, +childProcess.exec(nodeBinary + ' ' + + preloadOption([fixtureA]) + ' ' + + fixtureB, function(err, stdout, stderr) { if (err) throw err; assert.strictEqual(stdout, 'A\nB\n'); }); // test preloading multiple modules works -childProcess.exec(nodeBinary + ' ' - + preloadOption([fixtureA, fixtureB]) + ' ' - + fixtureC, +childProcess.exec(nodeBinary + ' ' + + preloadOption([fixtureA, fixtureB]) + ' ' + + fixtureC, function(err, stdout, stderr) { if (err) throw err; assert.strictEqual(stdout, 'A\nB\nC\n'); }); // test that preloading a throwing module aborts -childProcess.exec(nodeBinary + ' ' - + preloadOption([fixtureA, fixtureThrows]) + ' ' - + fixtureB, +childProcess.exec(nodeBinary + ' ' + + preloadOption([fixtureA, fixtureThrows]) + ' ' + + fixtureB, function(err, stdout, stderr) { if (err) { assert.strictEqual(stdout, 'A\n'); @@ -62,9 +62,9 @@ childProcess.exec(nodeBinary + ' ' }); // test that preload can be used with --eval -childProcess.exec(nodeBinary + ' ' - + preloadOption([fixtureA]) - + '-e "console.log(\'hello\');"', +childProcess.exec(nodeBinary + ' ' + + preloadOption([fixtureA]) + + '-e "console.log(\'hello\');"', function(err, stdout, stderr) { if (err) throw err; assert.strictEqual(stdout, 'A\nhello\n'); @@ -108,19 +108,19 @@ replProc.on('close', function(code) { // test that preload placement at other points in the cmdline // also test that duplicated preload only gets loaded once -childProcess.exec(nodeBinary + ' ' - + preloadOption([fixtureA]) - + '-e "console.log(\'hello\');" ' - + preloadOption([fixtureA, fixtureB]), +childProcess.exec(nodeBinary + ' ' + + preloadOption([fixtureA]) + + '-e "console.log(\'hello\');" ' + + preloadOption([fixtureA, fixtureB]), function(err, stdout, stderr) { if (err) throw err; assert.strictEqual(stdout, 'A\nB\nhello\n'); }); // test that preload works with -i -const interactive = childProcess.exec(nodeBinary + ' ' - + preloadOption([fixtureD]) - + '-i', +const interactive = childProcess.exec(nodeBinary + ' ' + + preloadOption([fixtureD]) + + '-i', common.mustCall(function(err, stdout, stderr) { assert.ifError(err); assert.strictEqual(stdout, "> 'test'\n> "); @@ -129,9 +129,9 @@ const interactive = childProcess.exec(nodeBinary + ' ' interactive.stdin.write('a\n'); interactive.stdin.write('process.exit()\n'); -childProcess.exec(nodeBinary + ' ' - + '--require ' + fixture('cluster-preload.js') + ' ' - + fixture('cluster-preload-test.js'), +childProcess.exec(nodeBinary + ' ' + + '--require ' + fixture('cluster-preload.js') + ' ' + + fixture('cluster-preload-test.js'), function(err, stdout, stderr) { if (err) throw err; assert.ok(/worker terminated with code 43/.test(stdout)); @@ -139,10 +139,10 @@ childProcess.exec(nodeBinary + ' ' // https://github.com/nodejs/node/issues/1691 process.chdir(common.fixturesDir); -childProcess.exec(nodeBinary + ' ' - + '--expose_debug_as=v8debug ' - + '--require ' + fixture('cluster-preload.js') + ' ' - + 'cluster-preload-test.js', +childProcess.exec(nodeBinary + ' ' + + '--expose_debug_as=v8debug ' + + '--require ' + fixture('cluster-preload.js') + ' ' + + 'cluster-preload-test.js', function(err, stdout, stderr) { if (err) throw err; assert.ok(/worker terminated with code 43/.test(stdout)); diff --git a/test/parallel/test-repl-domain.js b/test/parallel/test-repl-domain.js index 9f66f306395d29..3cc88b75f72428 100644 --- a/test/parallel/test-repl-domain.js +++ b/test/parallel/test-repl-domain.js @@ -18,6 +18,6 @@ putIn.write = function(data) { }; putIn.run([ - 'require("domain").create().on("error", function() { console.log("OK") })' - + '.run(function() { throw new Error("threw") })' + 'require("domain").create().on("error", function() { console.log("OK") })' + + '.run(function() { throw new Error("threw") })' ]); diff --git a/test/parallel/test-tls-client-mindhsize.js b/test/parallel/test-tls-client-mindhsize.js index 9956c971ffbbf3..1d812a72093ba9 100644 --- a/test/parallel/test-tls-client-mindhsize.js +++ b/test/parallel/test-tls-client-mindhsize.js @@ -53,8 +53,8 @@ function test(size, err, next) { if (err) { client.on('error', function(e) { nerror++; - assert.strictEqual(e.message, 'DH parameter size 1024 is less' - + ' than 2048'); + assert.strictEqual(e.message, 'DH parameter size 1024 is less' + + ' than 2048'); server.close(); }); } From 7c2dbd13b595a266b63ab02fb7d820c08ecb6eb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Thu, 8 Dec 2016 10:30:45 +0100 Subject: [PATCH 49/84] tools: enforce consistent operator linebreak style Adds the `operator-linebreak` rule to our ESLint config. PR-URL: https://github.com/nodejs/node/pull/10178 Reviewed-By: Ben Noordhuis Reviewed-By: Roman Reiss Reviewed-By: Colin Ihrig Reviewed-By: Jeremiah Senkpiel Reviewed-By: Michael Dawson Reviewed-By: Rich Trott Reviewed-By: Teddy Katz Reviewed-By: Sakthipriyan Vairamani --- .eslintrc | 1 + 1 file changed, 1 insertion(+) diff --git a/.eslintrc b/.eslintrc index 8d3689443bd1de..618b3d51116053 100644 --- a/.eslintrc +++ b/.eslintrc @@ -93,6 +93,7 @@ rules: no-multiple-empty-lines: [2, {max: 2, maxEOF: 0, maxBOF: 0}] no-tabs: 2 no-trailing-spaces: 2 + operator-linebreak: [2, after, {overrides: {'?': ignore, ':': ignore}}] quotes: [2, single, avoid-escape] semi: 2 semi-spacing: 2 From aa77b767b6ba3d047ec946df5f65d1f835a9796a Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 9 Dec 2016 00:17:14 +0100 Subject: [PATCH 50/84] lib,src: support values > 4GB in heap statistics We were transporting the heap statistics as uint32 values to JS land but those wrap around for values > 4 GB. Use 64 bits floats instead, those should last us a while. Fixes: https://github.com/nodejs/node/issues/10185 PR-URL: https://github.com/nodejs/node/pull/10186 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Evan Lucas --- lib/v8.js | 4 ++-- src/env-inl.h | 8 ++++---- src/env.h | 12 ++++++------ src/node_v8.cc | 12 ++++++------ 4 files changed, 18 insertions(+), 18 deletions(-) diff --git a/lib/v8.js b/lib/v8.js index 90abc627a45af0..415aed593eb799 100644 --- a/lib/v8.js +++ b/lib/v8.js @@ -18,7 +18,7 @@ const v8binding = process.binding('v8'); // Properties for heap statistics buffer extraction. const heapStatisticsBuffer = - new Uint32Array(v8binding.heapStatisticsArrayBuffer); + new Float64Array(v8binding.heapStatisticsArrayBuffer); const kTotalHeapSizeIndex = v8binding.kTotalHeapSizeIndex; const kTotalHeapSizeExecutableIndex = v8binding.kTotalHeapSizeExecutableIndex; const kTotalPhysicalSizeIndex = v8binding.kTotalPhysicalSizeIndex; @@ -31,7 +31,7 @@ const kPeakMallocedMemoryIndex = v8binding.kPeakMallocedMemoryIndex; // Properties for heap space statistics buffer extraction. const heapSpaceStatisticsBuffer = - new Uint32Array(v8binding.heapSpaceStatisticsArrayBuffer); + new Float64Array(v8binding.heapSpaceStatisticsArrayBuffer); const kHeapSpaces = v8binding.kHeapSpaces; const kNumberOfHeapSpaces = kHeapSpaces.length; const kHeapSpaceStatisticsPropertiesCount = diff --git a/src/env-inl.h b/src/env-inl.h index 83db3d33b6d18f..4b43057de33f6a 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -316,22 +316,22 @@ inline std::vector* Environment::destroy_ids_list() { return &destroy_ids_list_; } -inline uint32_t* Environment::heap_statistics_buffer() const { +inline double* Environment::heap_statistics_buffer() const { CHECK_NE(heap_statistics_buffer_, nullptr); return heap_statistics_buffer_; } -inline void Environment::set_heap_statistics_buffer(uint32_t* pointer) { +inline void Environment::set_heap_statistics_buffer(double* pointer) { CHECK_EQ(heap_statistics_buffer_, nullptr); // Should be set only once. heap_statistics_buffer_ = pointer; } -inline uint32_t* Environment::heap_space_statistics_buffer() const { +inline double* Environment::heap_space_statistics_buffer() const { CHECK_NE(heap_space_statistics_buffer_, nullptr); return heap_space_statistics_buffer_; } -inline void Environment::set_heap_space_statistics_buffer(uint32_t* pointer) { +inline void Environment::set_heap_space_statistics_buffer(double* pointer) { CHECK_EQ(heap_space_statistics_buffer_, nullptr); // Should be set only once. heap_space_statistics_buffer_ = pointer; } diff --git a/src/env.h b/src/env.h index b99bb45f819e59..8c256ca9c7faa4 100644 --- a/src/env.h +++ b/src/env.h @@ -469,11 +469,11 @@ class Environment { // List of id's that have been destroyed and need the destroy() cb called. inline std::vector* destroy_ids_list(); - inline uint32_t* heap_statistics_buffer() const; - inline void set_heap_statistics_buffer(uint32_t* pointer); + inline double* heap_statistics_buffer() const; + inline void set_heap_statistics_buffer(double* pointer); - inline uint32_t* heap_space_statistics_buffer() const; - inline void set_heap_space_statistics_buffer(uint32_t* pointer); + inline double* heap_space_statistics_buffer() const; + inline void set_heap_space_statistics_buffer(double* pointer); inline char* http_parser_buffer() const; inline void set_http_parser_buffer(char* buffer); @@ -581,8 +581,8 @@ class Environment { &HandleCleanup::handle_cleanup_queue_> handle_cleanup_queue_; int handle_cleanup_waiting_; - uint32_t* heap_statistics_buffer_ = nullptr; - uint32_t* heap_space_statistics_buffer_ = nullptr; + double* heap_statistics_buffer_ = nullptr; + double* heap_space_statistics_buffer_ = nullptr; char* http_parser_buffer_; diff --git a/src/node_v8.cc b/src/node_v8.cc index a033c48a7c98be..7059922607089d 100644 --- a/src/node_v8.cc +++ b/src/node_v8.cc @@ -57,8 +57,8 @@ void UpdateHeapStatisticsArrayBuffer(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); HeapStatistics s; env->isolate()->GetHeapStatistics(&s); - uint32_t* const buffer = env->heap_statistics_buffer(); -#define V(index, name, _) buffer[index] = static_cast(s.name()); + double* const buffer = env->heap_statistics_buffer(); +#define V(index, name, _) buffer[index] = static_cast(s.name()); HEAP_STATISTICS_PROPERTIES(V) #undef V } @@ -68,13 +68,13 @@ void UpdateHeapSpaceStatisticsBuffer(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); HeapSpaceStatistics s; Isolate* const isolate = env->isolate(); - uint32_t* buffer = env->heap_space_statistics_buffer(); + double* buffer = env->heap_space_statistics_buffer(); for (size_t i = 0; i < number_of_heap_spaces; i++) { isolate->GetHeapSpaceStatistics(&s, i); size_t const property_offset = i * kHeapSpaceStatisticsPropertiesCount; #define V(index, name, _) buffer[property_offset + index] = \ - static_cast(s.name()); + static_cast(s.name()); HEAP_SPACE_STATISTICS_PROPERTIES(V) #undef V } @@ -103,7 +103,7 @@ void InitializeV8Bindings(Local target, "updateHeapStatisticsArrayBuffer", UpdateHeapStatisticsArrayBuffer); - env->set_heap_statistics_buffer(new uint32_t[kHeapStatisticsPropertiesCount]); + env->set_heap_statistics_buffer(new double[kHeapStatisticsPropertiesCount]); const size_t heap_statistics_buffer_byte_length = sizeof(*env->heap_statistics_buffer()) * kHeapStatisticsPropertiesCount; @@ -149,7 +149,7 @@ void InitializeV8Bindings(Local target, UpdateHeapSpaceStatisticsBuffer); env->set_heap_space_statistics_buffer( - new uint32_t[kHeapSpaceStatisticsPropertiesCount * number_of_heap_spaces]); + new double[kHeapSpaceStatisticsPropertiesCount * number_of_heap_spaces]); const size_t heap_space_statistics_buffer_byte_length = sizeof(*env->heap_space_statistics_buffer()) * From 8dbf1aff3a1026b6a1ff734a7d7d7e96ed7d6f48 Mon Sep 17 00:00:00 2001 From: "Italo A. Casas" Date: Wed, 30 Nov 2016 20:48:58 -0500 Subject: [PATCH 51/84] test: stream readableListening internal state PR-URL: https://github.com/nodejs/node/pull/9864 Refs: https://github.com/nodejs/node/issues/8683 Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig --- .../test-stream-readableListening-state.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 test/parallel/test-stream-readableListening-state.js diff --git a/test/parallel/test-stream-readableListening-state.js b/test/parallel/test-stream-readableListening-state.js new file mode 100644 index 00000000000000..5e3071faf370e5 --- /dev/null +++ b/test/parallel/test-stream-readableListening-state.js @@ -0,0 +1,34 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const stream = require('stream'); + +const r = new stream.Readable({ + read: () => {} +}); + +// readableListening state should start in `false`. +assert.strictEqual(r._readableState.readableListening, false); + +r.on('readable', common.mustCall(() => { + // Inside the readable event this state should be true. + assert.strictEqual(r._readableState.readableListening, true); +})); + +r.push(Buffer.from('Testing readableListening state')); + +const r2 = new stream.Readable({ + read: () => {} +}); + +// readableListening state should start in `false`. +assert.strictEqual(r2._readableState.readableListening, false); + +r2.on('data', common.mustCall((chunk) => { + // readableListening should be false because we don't have + // a `readable` listener + assert.strictEqual(r2._readableState.readableListening, false); +})); + +r2.push(Buffer.from('Testing readableListening state')); From 8621ccc25a3da62bfc8e756e34b4238c35fab8a4 Mon Sep 17 00:00:00 2001 From: Gregory Date: Thu, 1 Dec 2016 01:52:23 -0500 Subject: [PATCH 52/84] test: stream readableState readingMore state PR-URL: https://github.com/nodejs/node/pull/9868 Refs: https://github.com/nodejs/node/issues/8685 Reviewed-By: Matteo Collina Reviewed-By: Colin Ihrig Reviewed-By: Italo A. Casas --- ...est-stream-readable-reading-readingMore.js | 65 +++++++++++++++++++ 1 file changed, 65 insertions(+) create mode 100644 test/parallel/test-stream-readable-reading-readingMore.js diff --git a/test/parallel/test-stream-readable-reading-readingMore.js b/test/parallel/test-stream-readable-reading-readingMore.js new file mode 100644 index 00000000000000..bee3a1c82a8678 --- /dev/null +++ b/test/parallel/test-stream-readable-reading-readingMore.js @@ -0,0 +1,65 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const Readable = require('stream').Readable; + +const readable = new Readable({ + read(size) {} +}); + +const state = readable._readableState; + +// Starting off with false initially. +assert.strictEqual(state.reading, false); +assert.strictEqual(state.readingMore, false); + +readable.on('data', common.mustCall((data) => { + // while in a flowing state, should try to read more. + if (state.flowing) + assert.strictEqual(state.readingMore, true); + + // reading as long as we've not ended + assert.strictEqual(state.reading, !state.ended); +}, 2)); + +function onStreamEnd() { + // End of stream; state.reading is false + // And so should be readingMore. + assert.strictEqual(state.readingMore, false); + assert.strictEqual(state.reading, false); +} + +readable.on('readable', common.mustCall(() => { + // 'readable' always gets called before 'end' + // since 'end' hasn't been emitted, more data could be incoming + assert.strictEqual(state.readingMore, true); + + // if the stream has ended, we shouldn't be reading + assert.strictEqual(state.ended, !state.reading); + + if (readable.read() === null) // reached end of stream + process.nextTick(common.mustCall(onStreamEnd, 1)); +}, 2)); + +readable.on('end', common.mustCall(onStreamEnd)); + +readable.push('pushed'); + +// stop emitting 'data' events +readable.pause(); + +// read() should only be called while operating in paused mode +readable.read(6); + +// reading +assert.strictEqual(state.reading, true); +assert.strictEqual(state.readingMore, true); + +// resume emitting 'data' events +readable.resume(); + +// add chunk to front +readable.unshift('unshifted'); + +// end +readable.push(null); From f5c2c8c7f42ef7a65f5555779cf08b44cbabbea1 Mon Sep 17 00:00:00 2001 From: James Tenenbaum Date: Thu, 1 Dec 2016 10:07:28 -0700 Subject: [PATCH 53/84] test: improving crypto fips - using strictEqual instead equal - cast `response` to Number() PR-URL: https://github.com/nodejs/node/pull/10002 Reviewed-By: James M Snell Reviewed-By: Luigi Pinca Reviewed-By: Italo A. Casas --- test/parallel/test-crypto-fips.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-crypto-fips.js b/test/parallel/test-crypto-fips.js index 55b542ca9ead83..24b1af70b70329 100644 --- a/test/parallel/test-crypto-fips.js +++ b/test/parallel/test-crypto-fips.js @@ -53,7 +53,7 @@ function testHelper(stream, args, expectedOutput, cmd, env) { assert.notEqual(-1, response.indexOf(expectedOutput)); } else { // Normal path where we expect either FIPS enabled or disabled. - assert.equal(expectedOutput, response); + assert.strictEqual(expectedOutput, Number(response)); } childOk(child); } From 9f58e029087b73609fb19a0f226996a4cc019789 Mon Sep 17 00:00:00 2001 From: Johnny Reading Date: Thu, 1 Dec 2016 11:53:56 -0600 Subject: [PATCH 54/84] test: improve buffer transcode This test is for argument validation in transcode. PR-URL: https://github.com/nodejs/node/pull/10043 Reviewed-By: Colin Ihrig --- test/parallel/test-icu-transcode.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-icu-transcode.js b/test/parallel/test-icu-transcode.js index 2cdd4078a2cceb..c099e754ca55d6 100644 --- a/test/parallel/test-icu-transcode.js +++ b/test/parallel/test-icu-transcode.js @@ -38,13 +38,19 @@ for (const test in tests) { utf8_to_ucs2.toString('ucs2')); } +assert.throws( + () => buffer.transcode(null, 'utf8', 'ascii'), + /^TypeError: "source" argument must be a Buffer$/ +); + assert.throws( () => buffer.transcode(Buffer.from('a'), 'b', 'utf8'), - /Unable to transcode Buffer \[U_ILLEGAL_ARGUMENT_ERROR\]/ + /^Error: Unable to transcode Buffer \[U_ILLEGAL_ARGUMENT_ERROR\]/ ); + assert.throws( () => buffer.transcode(Buffer.from('a'), 'uf8', 'b'), - /Unable to transcode Buffer \[U_ILLEGAL_ARGUMENT_ERROR\]/ + /^Error: Unable to transcode Buffer \[U_ILLEGAL_ARGUMENT_ERROR\]$/ ); assert.deepStrictEqual( From a84017a689734b4519e5afc8571c69fe89aa5aeb Mon Sep 17 00:00:00 2001 From: joyeecheung Date: Thu, 1 Dec 2016 11:32:31 -0600 Subject: [PATCH 55/84] url, test: including base argument in originFor - Add tests to check if the `originFor` implementation for WHATWG url parsing is correnct. - Fix `originFor` by including a base as argument PR-URL: https://github.com/nodejs/node/pull/10021 Reviewed-By: James M Snell --- lib/internal/url.js | 4 ++-- test/parallel/test-whatwg-url-origin-for.js | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-whatwg-url-origin-for.js diff --git a/lib/internal/url.js b/lib/internal/url.js index 1129060b09f419..d8053dc2ad06cf 100644 --- a/lib/internal/url.js +++ b/lib/internal/url.js @@ -790,9 +790,9 @@ Object.defineProperty(URLSearchParamsIteratorPrototype, Symbol.toStringTag, { configurable: true }); -URL.originFor = function(url) { +URL.originFor = function(url, base) { if (!(url instanceof URL)) - url = new URL(url); + url = new URL(url, base); var origin; const protocol = url.protocol; switch (protocol) { diff --git a/test/parallel/test-whatwg-url-origin-for.js b/test/parallel/test-whatwg-url-origin-for.js new file mode 100644 index 00000000000000..a82f624e4e3392 --- /dev/null +++ b/test/parallel/test-whatwg-url-origin-for.js @@ -0,0 +1,19 @@ +'use strict'; + +const common = require('../common'); + +const URL = require('url').URL; +const path = require('path'); +const assert = require('assert'); +const tests = require(path.join(common.fixturesDir, 'url-tests.json')); + +for (const test of tests) { + if (typeof test === 'string') + continue; + + if (test.origin) { + const origin = URL.originFor(test.input, test.base); + // Pass true to origin.toString() to enable unicode serialization. + assert.strictEqual(origin.toString(true), test.origin); + } +} From 56072280996f2b39328952985723f0c76a11a189 Mon Sep 17 00:00:00 2001 From: Adrian Estrada Date: Thu, 8 Dec 2016 12:53:02 -0500 Subject: [PATCH 56/84] test: use ES6 in test-debugger-client.js implements ES6 const and let instead var in test-debugger-client.js PR-URL: https://github.com/nodejs/node/pull/10183 Reviewed-By: Colin Ihrig Reviewed-By: Italo A. Casas Reviewed-By: Sakthipriyan Vairamani --- test/debugger/test-debugger-client.js | 70 +++++++++++++-------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/test/debugger/test-debugger-client.js b/test/debugger/test-debugger-client.js index 04823113ec32c1..743a3c4352e2e1 100644 --- a/test/debugger/test-debugger-client.js +++ b/test/debugger/test-debugger-client.js @@ -1,12 +1,12 @@ 'use strict'; const common = require('../common'); -var assert = require('assert'); -var debug = require('_debugger'); +const assert = require('assert'); +const debug = require('_debugger'); process.env.NODE_DEBUGGER_TIMEOUT = 2000; -var debugPort = common.PORT; +const debugPort = common.PORT; debug.port = debugPort; -var spawn = require('child_process').spawn; +const spawn = require('child_process').spawn; setTimeout(function() { if (nodeProcess) nodeProcess.kill('SIGTERM'); @@ -14,8 +14,8 @@ setTimeout(function() { }, 10000).unref(); -var resCount = 0; -var p = new debug.Protocol(); +let resCount = 0; +const p = new debug.Protocol(); p.onResponse = function(res) { resCount++; }; @@ -29,12 +29,12 @@ assert.strictEqual(resCount, 1); // Make sure split messages go in. -var parts = []; +const parts = []; parts.push('Content-Length: 336\r\n'); assert.strictEqual(parts[0].length, 21); parts.push('\r\n'); assert.strictEqual(parts[1].length, 2); -var bodyLength = 0; +let bodyLength = 0; parts.push('{"seq":12,"type":"event","event":"break","body":' + '{"invocationText":"#'); @@ -55,7 +55,7 @@ bodyLength += parts[4].length; assert.strictEqual(bodyLength, 336); -for (var i = 0; i < parts.length; i++) { +for (let i = 0; i < parts.length; i++) { p.execute(parts[i]); } assert.strictEqual(resCount, 2); @@ -63,24 +63,24 @@ assert.strictEqual(resCount, 2); // Make sure that if we get backed up, we still manage to get all the // messages -var d = 'Content-Length: 466\r\n\r\n' + - '{"seq":10,"type":"event","event":"afterCompile","success":true,' + - '"body":{"script":{"handle":1,"type":"script","name":"dns.js",' + - '"id":34,"lineOffset":0,"columnOffset":0,"lineCount":241,' + - '"sourceStart":"(function(module, exports, require) {' + - 'var dns = process.binding(\'cares\')' + - ';\\nvar ne","sourceLength":6137,"scriptType":2,"compilationType":0,' + - '"context":{"ref":0},"text":"dns.js (lines: 241)"}},"refs":' + - '[{"handle":0' + - ',"type":"context","text":"#"}],"running":true}' + - '\r\n\r\nContent-Length: 119\r\n\r\n' + - '{"seq":11,"type":"event","event":"scriptCollected","success":true,' + - '"body":{"script":{"id":26}},"refs":[],"running":true}'; +const d = 'Content-Length: 466\r\n\r\n' + + '{"seq":10,"type":"event","event":"afterCompile","success":true,' + + '"body":{"script":{"handle":1,"type":"script","name":"dns.js",' + + '"id":34,"lineOffset":0,"columnOffset":0,"lineCount":241,' + + '"sourceStart":"(function(module, exports, require) {' + + 'var dns = process.binding(\'cares\')' + + ';\\nvar ne","sourceLength":6137,"scriptType":2,"compilationType"' + + ':0,"context":{"ref":0},"text":"dns.js (lines: 241)"}},"refs":' + + '[{"handle":0' + + ',"type":"context","text":"#"}],"running":true}' + + '\r\n\r\nContent-Length: 119\r\n\r\n' + + '{"seq":11,"type":"event","event":"scriptCollected","success":true' + + ',"body":{"script":{"id":26}},"refs":[],"running":true}'; p.execute(d); assert.strictEqual(resCount, 4); -var expectedConnections = 0; -var tests = []; +let expectedConnections = 0; +const tests = []; function addTest(cb) { expectedConnections++; tests.push(cb); @@ -102,9 +102,9 @@ addTest(function(client, done) { assert.ok(!err); console.error('got %d scripts', Object.keys(client.scripts).length); - var foundMainScript = false; - for (var k in client.scripts) { - var script = client.scripts[k]; + let foundMainScript = false; + for (const k in client.scripts) { + const script = client.scripts[k]; if (script && script.name === 'node.js') { foundMainScript = true; break; @@ -127,19 +127,19 @@ addTest(function(client, done) { }); -var connectCount = 0; -var script = 'setTimeout(function() { console.log("blah"); });' + - 'setInterval(function() {}, 1000000);'; +let connectCount = 0; +const script = 'setTimeout(function() { console.log("blah"); });' + + 'setInterval(function() {}, 1000000);'; -var nodeProcess; +let nodeProcess; function doTest(cb, done) { - var args = ['--debug=' + debugPort, '-e', script]; + const args = ['--debug=' + debugPort, '-e', script]; nodeProcess = spawn(process.execPath, args); nodeProcess.stdout.once('data', function(c) { console.log('>>> new node process: %d', nodeProcess.pid); - var failed = true; + let failed = true; try { process._debugProcess(nodeProcess.pid); failed = false; @@ -151,9 +151,9 @@ function doTest(cb, done) { console.log('>>> starting debugger session'); }); - var didTryConnect = false; + let didTryConnect = false; nodeProcess.stderr.setEncoding('utf8'); - var b = ''; + let b = ''; nodeProcess.stderr.on('data', function(data) { console.error('got stderr data %j', data); nodeProcess.stderr.resume(); From 14b0b4463e9dd2ba81e7c67b1c4a66f429ed7f0a Mon Sep 17 00:00:00 2001 From: pallxk Date: Tue, 6 Dec 2016 13:34:25 +0800 Subject: [PATCH 57/84] doc: fix typo in code example of 'path' module PR-URL: https://github.com/nodejs/node/pull/10136 Reviewed-By: Brian White Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Luigi Pinca --- doc/api/path.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/path.md b/doc/api/path.md index a9c72c2e2e78a6..df65616b6c4e94 100644 --- a/doc/api/path.md +++ b/doc/api/path.md @@ -24,7 +24,7 @@ On POSIX: ```js path.basename('C:\\temp\\myfile.html'); -// Returns: 'C:\temp\myfile.html' +// Returns: 'C:\\temp\\myfile.html' ``` On Windows: From 10929f6cb6f433ef82696ca24cefc539c3ddd165 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 6 Dec 2016 18:23:13 +0100 Subject: [PATCH 58/84] test: fail for missing output files Instead of ignoring missing `.out` files for message/pseudo-tty tests, raise an error to indicate that something is not quite right. Ref: https://github.com/nodejs/node/pull/10037 PR-URL: https://github.com/nodejs/node/pull/10150 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Sakthipriyan Vairamani --- test/message/testcfg.py | 3 +-- test/pseudo-tty/testcfg.py | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/test/message/testcfg.py b/test/message/testcfg.py index 7183ab333d8274..3c668459079f0a 100644 --- a/test/message/testcfg.py +++ b/test/message/testcfg.py @@ -127,8 +127,7 @@ def ListTests(self, current_path, path, arch, mode): file_path = file_prefix + ".js" output_path = file_prefix + ".out" if not exists(output_path): - print "Could not find %s" % output_path - continue + raise Exception("Could not find %s" % output_path) result.append(MessageTestCase(test, file_path, output_path, arch, mode, self.context, self)) return result diff --git a/test/pseudo-tty/testcfg.py b/test/pseudo-tty/testcfg.py index 96b30253498857..40396db247e279 100644 --- a/test/pseudo-tty/testcfg.py +++ b/test/pseudo-tty/testcfg.py @@ -142,8 +142,7 @@ def ListTests(self, current_path, path, arch, mode): file_path = file_prefix + ".js" output_path = file_prefix + ".out" if not exists(output_path): - print "Could not find %s" % output_path - continue + raise Exception("Could not find %s" % output_path) result.append(TTYTestCase(test, file_path, output_path, arch, mode, self.context, self)) return result From f418a227677a2801eaa1ceb084d64bd16e17fc45 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Sat, 3 Dec 2016 07:36:19 +0200 Subject: [PATCH 59/84] doc: modernize child_process example code 1. equal => strictEqual. 2. let => const for the variable that is not reassigned. 3. fix spaces. 4. stringify erroneous raw buffer outputs. 5. fix a typo. PR-URL: https://github.com/nodejs/node/pull/10102 Reviewed-By: Sam Roberts Reviewed-By: Sakthipriyan Vairamani --- doc/api/child_process.md | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/doc/api/child_process.md b/doc/api/child_process.md index 738b916eb6ac4b..74e9a930fb05b4 100644 --- a/doc/api/child_process.md +++ b/doc/api/child_process.md @@ -91,11 +91,11 @@ const spawn = require('child_process').spawn; const bat = spawn('cmd.exe', ['/c', 'my.bat']); bat.stdout.on('data', (data) => { - console.log(data); + console.log(data.toString()); }); bat.stderr.on('data', (data) => { - console.log(data); + console.log(data.toString()); }); bat.on('exit', (code) => { @@ -113,7 +113,7 @@ exec('my.bat', (err, stdout, stderr) => { }); // Script with spaces in the filename: -const bat = spawn('"my script.cmd"', ['a', 'b'], { shell:true }); +const bat = spawn('"my script.cmd"', ['a', 'b'], { shell: true }); // or: exec('"my script.cmd" a b', (err, stdout, stderr) => { // ... @@ -383,7 +383,7 @@ ps.on('close', (code) => { }); grep.stdout.on('data', (data) => { - console.log(`${data}`); + console.log(data.toString()); }); grep.stderr.on('data', (data) => { @@ -467,8 +467,8 @@ const out = fs.openSync('./out.log', 'a'); const err = fs.openSync('./out.log', 'a'); const child = spawn('prg', [], { - detached: true, - stdio: [ 'ignore', out, err ] + detached: true, + stdio: [ 'ignore', out, err ] }); child.unref(); @@ -860,7 +860,7 @@ as in this example: 'use strict'; const spawn = require('child_process').spawn; -let child = spawn('sh', ['-c', +const child = spawn('sh', ['-c', `node -e "setInterval(() => { console.log(process.pid, 'is alive') }, 500);"` @@ -1107,21 +1107,21 @@ const fs = require('fs'); const child_process = require('child_process'); const child = child_process.spawn('ls', { - stdio: [ - 0, // Use parents stdin for child - 'pipe', // Pipe child's stdout to parent - fs.openSync('err.out', 'w') // Direct child's stderr to a file - ] + stdio: [ + 0, // Use parent's stdin for child + 'pipe', // Pipe child's stdout to parent + fs.openSync('err.out', 'w') // Direct child's stderr to a file + ] }); -assert.equal(child.stdio[0], null); -assert.equal(child.stdio[0], child.stdin); +assert.strictEqual(child.stdio[0], null); +assert.strictEqual(child.stdio[0], child.stdin); assert(child.stdout); -assert.equal(child.stdio[1], child.stdout); +assert.strictEqual(child.stdio[1], child.stdout); -assert.equal(child.stdio[2], null); -assert.equal(child.stdio[2], child.stderr); +assert.strictEqual(child.stdio[2], null); +assert.strictEqual(child.stdio[2], child.stderr); ``` ### child.stdout From 9ee915be76f9e4cc8bc43020a721a03257f9b984 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 6 Dec 2016 02:19:57 +0100 Subject: [PATCH 60/84] buffer: handle UCS2 `.fill()` properly on BE There was a byte-order mismatch for `buffer#fill` on big-endian platforms. Weirdly, the tests seemed to expect that wrong behaviour. PR-URL: https://github.com/nodejs/node/pull/9837 Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis Reviewed-By: Trevor Norris --- src/node_buffer.cc | 3 +++ test/parallel/test-buffer-fill.js | 23 ++++++++++------------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 540de1827f9716..0e96fd6deed8d3 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -606,6 +606,9 @@ void Fill(const FunctionCallbackInfo& args) { } else if (enc == UCS2) { node::TwoByteValue str(env->isolate(), args[1]); + if (IsBigEndian()) + SwapBytes16(reinterpret_cast(&str[0]), str_length); + memcpy(ts_obj_data + start, *str, MIN(str_length, fill_length)); } else { diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js index 4272d686940cc7..68b8bbd69c4566 100644 --- a/test/parallel/test-buffer-fill.js +++ b/test/parallel/test-buffer-fill.js @@ -2,13 +2,11 @@ require('../common'); const assert = require('assert'); -const os = require('os'); const SIZE = 28; const buf1 = Buffer.allocUnsafe(SIZE); const buf2 = Buffer.allocUnsafe(SIZE); - // Default encoding testBufs('abc'); testBufs('\u0222aa'); @@ -112,8 +110,7 @@ testBufs('\u0222aa', 8, 1, 'ucs2'); testBufs('a\u0234b\u0235c\u0236', 4, -1, 'ucs2'); testBufs('a\u0234b\u0235c\u0236', 4, 1, 'ucs2'); testBufs('a\u0234b\u0235c\u0236', 12, 1, 'ucs2'); -assert.strictEqual(Buffer.allocUnsafe(1).fill('\u0222', 'ucs2')[0], - os.endianness() === 'LE' ? 0x22 : 0x02); +assert.strictEqual(Buffer.allocUnsafe(1).fill('\u0222', 'ucs2')[0], 0x22); // HEX @@ -259,15 +256,6 @@ function writeToFill(string, offset, end, encoding) { } } while (offset < buf2.length); - // Correction for UCS2 operations. - if (os.endianness() === 'BE' && encoding === 'ucs2') { - for (var i = 0; i < buf2.length; i += 2) { - var tmp = buf2[i]; - buf2[i] = buf2[i + 1]; - buf2[i + 1] = tmp; - } - } - return buf2; } @@ -406,3 +394,12 @@ assert.throws(() => { }); buf.fill(''); }, /^RangeError: out of range index$/); + + +assert.deepStrictEqual( + Buffer.allocUnsafeSlow(16).fill('ab', 'utf16le'), + Buffer.from('61006200610062006100620061006200', 'hex')); + +assert.deepStrictEqual( + Buffer.allocUnsafeSlow(15).fill('ab', 'utf16le'), + Buffer.from('610062006100620061006200610062', 'hex')); From d4f00fe1098b0d7b8444565e741d8b457fd9c091 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 29 Nov 2016 05:03:24 -0600 Subject: [PATCH 61/84] buffer: fix single-character string filling Fix the fast path for `buffer.fill()` with a single-character string. The fast path only works for strings that are equivalent to a single-byte buffer, but that condition was not checked properly for the `utf8` or `utf16le` encodings and is always true for the `latin1` encoding. This change fixes these problems. Fixes: https://github.com/nodejs/node/issues/9836 PR-URL: https://github.com/nodejs/node/pull/9837 Reviewed-By: James M Snell Reviewed-By: Ben Noordhuis Reviewed-By: Trevor Norris --- lib/buffer.js | 26 +++++++++++++++----------- test/parallel/test-buffer-fill.js | 28 +++++++++++++++++++++++++++- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 3453f5fe8cd6ae..557ac867e2e0fc 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -672,23 +672,27 @@ Buffer.prototype.fill = function fill(val, start, end, encoding) { encoding = end; end = this.length; } - if (val.length === 1) { - var code = val.charCodeAt(0); - if (code < 256) - val = code; - } - if (val.length === 0) { - // Previously, if val === '', the Buffer would not fill, - // which is rather surprising. - val = 0; - } + if (encoding !== undefined && typeof encoding !== 'string') { throw new TypeError('encoding must be a string'); } - if (typeof encoding === 'string' && !Buffer.isEncoding(encoding)) { + var normalizedEncoding = internalUtil.normalizeEncoding(encoding); + if (normalizedEncoding === undefined) { throw new TypeError('Unknown encoding: ' + encoding); } + if (val.length === 0) { + // Previously, if val === '', the Buffer would not fill, + // which is rather surprising. + val = 0; + } else if (val.length === 1) { + var code = val.charCodeAt(0); + if ((normalizedEncoding === 'utf8' && code < 128) || + normalizedEncoding === 'latin1') { + // Fast path: If `val` fits into a single byte, use that numeric value. + val = code; + } + } } else if (typeof val === 'number') { val = val & 255; } diff --git a/test/parallel/test-buffer-fill.js b/test/parallel/test-buffer-fill.js index 68b8bbd69c4566..eecb14abb06001 100644 --- a/test/parallel/test-buffer-fill.js +++ b/test/parallel/test-buffer-fill.js @@ -395,7 +395,6 @@ assert.throws(() => { buf.fill(''); }, /^RangeError: out of range index$/); - assert.deepStrictEqual( Buffer.allocUnsafeSlow(16).fill('ab', 'utf16le'), Buffer.from('61006200610062006100620061006200', 'hex')); @@ -403,3 +402,30 @@ assert.deepStrictEqual( assert.deepStrictEqual( Buffer.allocUnsafeSlow(15).fill('ab', 'utf16le'), Buffer.from('610062006100620061006200610062', 'hex')); + +assert.deepStrictEqual( + Buffer.allocUnsafeSlow(16).fill('ab', 'utf16le'), + Buffer.from('61006200610062006100620061006200', 'hex')); +assert.deepStrictEqual( + Buffer.allocUnsafeSlow(16).fill('a', 'utf16le'), + Buffer.from('61006100610061006100610061006100', 'hex')); + +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('a', 'utf16le').toString('utf16le'), + 'a'.repeat(8)); +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('a', 'latin1').toString('latin1'), + 'a'.repeat(16)); +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('a', 'utf8').toString('utf8'), + 'a'.repeat(16)); + +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('Љ', 'utf16le').toString('utf16le'), + 'Љ'.repeat(8)); +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('Љ', 'latin1').toString('latin1'), + '\t'.repeat(16)); +assert.strictEqual( + Buffer.allocUnsafeSlow(16).fill('Љ', 'utf8').toString('utf8'), + 'Љ'.repeat(8)); From c3839f7ed47a4f09fe67fb5a1905a209249c0b9c Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Fri, 18 Nov 2016 11:37:32 -0800 Subject: [PATCH 62/84] tls: fix/annotate connect arg comments PR-URL: https://github.com/nodejs/node/pull/9800 Reviewed-By: Roman Reiss Reviewed-By: Michael Dawson --- lib/_tls_wrap.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index d9dafceb950dc7..b57a40a3be946d 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -975,6 +975,11 @@ function normalizeConnectArgs(listArgs) { var options = args[0]; var cb = args[1]; + // If args[0] was options, then normalize dealt with it. + // If args[0] is port, or args[0], args[1] is host,port, we need to + // find the options and merge them in, normalize's options has only + // the host/port/path args that it knows about, not the tls options. + // This means that options.host overrides a host arg. if (listArgs[1] !== null && typeof listArgs[1] === 'object') { options = util._extend(options, listArgs[1]); } else if (listArgs[2] !== null && typeof listArgs[2] === 'object') { @@ -984,7 +989,7 @@ function normalizeConnectArgs(listArgs) { return (cb) ? [options, cb] : [options]; } -exports.connect = function(/* [port, host], options, cb */) { +exports.connect = function(/* [port,] [host,] [options,] [cb] */) { const argsLen = arguments.length; var args = new Array(argsLen); for (var i = 0; i < argsLen; i++) From d4050b38d6bc42982ddc91571933d284c587e698 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Mon, 21 Nov 2016 13:50:02 -0800 Subject: [PATCH 63/84] tls: document and test option-less createServer Either the options or the listener argument to tls.createServer() was optional, but not both. This makes no sense, so align the argument checking and documentation with net.createServer(), which accepts the same option sequence, and which tls.createServer() is modelled on. PR-URL: https://github.com/nodejs/node/pull/9800 Reviewed-By: Roman Reiss Reviewed-By: Michael Dawson --- doc/api/tls.md | 2 +- lib/_tls_wrap.js | 17 +++++++++-------- test/parallel/test-tls-no-cert-required.js | 19 +++++++++++++++++-- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/doc/api/tls.md b/doc/api/tls.md index 488337a0764e4c..eeeb07069dcbbe 100644 --- a/doc/api/tls.md +++ b/doc/api/tls.md @@ -948,7 +948,7 @@ publicly trusted list of CAs as given in . -## tls.createServer(options[, secureConnectionListener]) +## tls.createServer([options][, secureConnectionListener]) diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index b57a40a3be946d..434384cec81595 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -745,18 +745,19 @@ TLSSocket.prototype.getProtocol = function() { // "PATH_LENGTH_EXCEEDED", "INVALID_PURPOSE" "CERT_UNTRUSTED", // "CERT_REJECTED" // -function Server(/* [options], listener */) { - var options, listener; +function Server(options, listener) { + if (!(this instanceof Server)) + return new Server(options, listener); - if (arguments[0] !== null && typeof arguments[0] === 'object') { - options = arguments[0]; - listener = arguments[1]; - } else if (typeof arguments[0] === 'function') { + if (typeof options === 'function') { + listener = options; options = {}; - listener = arguments[0]; + } else if (options == null || typeof options === 'object') { + options = options || {}; + } else { + throw new TypeError('options must be an object'); } - if (!(this instanceof Server)) return new Server(options, listener); this._contexts = []; diff --git a/test/parallel/test-tls-no-cert-required.js b/test/parallel/test-tls-no-cert-required.js index de723e73e8a335..8d907d9f8a4e06 100644 --- a/test/parallel/test-tls-no-cert-required.js +++ b/test/parallel/test-tls-no-cert-required.js @@ -1,4 +1,5 @@ 'use strict'; +var assert = require('assert'); var common = require('../common'); if (!common.hasCrypto) { @@ -10,6 +11,20 @@ var tls = require('tls'); // Omitting the cert or pfx option to tls.createServer() should not throw. // AECDH-NULL-SHA is a no-authentication/no-encryption cipher and hence // doesn't need a certificate. -tls.createServer({ ciphers: 'AECDH-NULL-SHA' }).listen(0, function() { +tls.createServer({ ciphers: 'AECDH-NULL-SHA' }) + .listen(0, common.mustCall(close)); + +tls.createServer(assert.fail) + .listen(0, common.mustCall(close)); + +tls.createServer({}) + .listen(0, common.mustCall(close)); + +assert.throws(() => tls.createServer('this is not valid'), TypeError); + +tls.createServer() + .listen(0, common.mustCall(close)); + +function close() { this.close(); -}); +} From 475b8db8354b50b971653912de82fc336001c6d3 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Tue, 22 Nov 2016 13:00:59 -0800 Subject: [PATCH 64/84] test: tls key/cert ordering not necessary PR-URL: https://github.com/nodejs/node/pull/9800 Reviewed-By: Roman Reiss Reviewed-By: Michael Dawson --- test/parallel/test-tls-multi-key.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-tls-multi-key.js b/test/parallel/test-tls-multi-key.js index e73ee2e25e6931..e29f5ee522829e 100644 --- a/test/parallel/test-tls-multi-key.js +++ b/test/parallel/test-tls-multi-key.js @@ -11,8 +11,8 @@ var fs = require('fs'); var options = { key: [ + fs.readFileSync(common.fixturesDir + '/keys/ec-key.pem'), fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'), - fs.readFileSync(common.fixturesDir + '/keys/ec-key.pem') ], cert: [ fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem'), From a28e9491453d7574344815a632ac6385c2054394 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Tue, 22 Nov 2016 13:23:06 -0800 Subject: [PATCH 65/84] tls: do not refer to secureOptions as flags Its confusing to have multiple names for the same thing, use secureOptions consistently. PR-URL: https://github.com/nodejs/node/pull/9800 Reviewed-By: Roman Reiss Reviewed-By: Michael Dawson --- lib/_tls_common.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/_tls_common.js b/lib/_tls_common.js index 6e98c1ee4d66d2..9cb70453860484 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -12,9 +12,9 @@ var crypto = null; const binding = process.binding('crypto'); const NativeSecureContext = binding.SecureContext; -function SecureContext(secureProtocol, flags, context) { +function SecureContext(secureProtocol, secureOptions, context) { if (!(this instanceof SecureContext)) { - return new SecureContext(secureProtocol, flags, context); + return new SecureContext(secureProtocol, secureOptions, context); } if (context) { @@ -29,7 +29,7 @@ function SecureContext(secureProtocol, flags, context) { } } - if (flags) this.context.setOptions(flags); + if (secureOptions) this.context.setOptions(secureOptions); } exports.SecureContext = SecureContext; From caa7fa982affddccb0058b3a77233d78bfa50a51 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Wed, 23 Nov 2016 14:14:34 -0800 Subject: [PATCH 66/84] doc: rework tls for accuracy and clarity Document all TLSSocket options: - All the secure context options are valid options to a secureContext - isServer modifies the default value of requestCert Describe all tls.connect() variants: - tls.connect(path) was undocumented - tls.connect(port) was underdocumented, and its relationship to tls.connect(options) was obscure Socket passed to tls.connect is user managed: - Replace https://github.com/nodejs/node/pull/8996 Add documentation to: - describe and add tests for the pfx and key variants, and describe how and when passphrase is used. - describe tls cert and ca options - describe buffer forms of tls crl option - describe tls cipher option and defaults - fix link to Crypto Constants - describe that honorCipherOrder sets SSL_OP_CIPHER_SERVER_PREFERENCE. - describe tls ecdhCurve/dhparam options - describe tls secureProtocol option - describe tls secureOptions - describe tls sessionIdContext De-deduplicate secure context docs: The secure context options were documented 4 times, making it difficult to understand where the options come from, where they are supported, and under what conditions they are used. The multiple copies were inconsistent and contradictory in their descriptions of the options, and also inconsistent in whether the options would be documented at all. Cut through this gordian knot by linking all APIs that use the secureContext options to the single source of truth about the options. PR-URL: https://github.com/nodejs/node/pull/9800 Reviewed-By: Roman Reiss Reviewed-By: Michael Dawson --- doc/api/crypto.md | 33 +-- doc/api/tls.md | 361 +++++++++++---------------- test/fixtures/raw-key.pem | 15 ++ test/parallel/test-tls-passphrase.js | 190 ++++++++++++-- 4 files changed, 346 insertions(+), 253 deletions(-) create mode 100644 test/fixtures/raw-key.pem diff --git a/doc/api/crypto.md b/doc/api/crypto.md index ef9db7ffc65420..7f77d3f940768c 100644 --- a/doc/api/crypto.md +++ b/doc/api/crypto.md @@ -1082,26 +1082,15 @@ deprecated: v0.11.13 > Stability: 0 - Deprecated: Use [`tls.createSecureContext()`][] instead. -The `crypto.createCredentials()` method is a deprecated alias for creating -and returning a `tls.SecureContext` object. The `crypto.createCredentials()` -method should not be used. +- `details` {Object} Identical to [`tls.createSecureContext()`][]. -The optional `details` argument is a hash object with keys: +The `crypto.createCredentials()` method is a deprecated function for creating +and returning a `tls.SecureContext`. It should not be used. Replace it with +[`tls.createSecureContext()`][] which has the exact same arguments and return +value. -* `pfx` : {String|Buffer} - PFX or PKCS12 encoded private - key, certificate and CA certificates -* `key` : {String} - PEM encoded private key -* `passphrase` : {String} - passphrase for the private key or PFX -* `cert` : {String} - PEM encoded certificate -* `ca` : {String|Array} - Either a string or array of strings of PEM encoded CA - certificates to trust. -* `crl` : {String|Array} - Either a string or array of strings of PEM encoded CRLs - (Certificate Revocation List) -* `ciphers`: {String} using the [OpenSSL cipher list format][] describing the - cipher algorithms to use or exclude. - -If no 'ca' details are given, Node.js will use Mozilla's default -[publicly trusted list of CAs][]. +Returns a `tls.SecureContext`, as-if [`tls.createSecureContext()`][] had been +called. ### crypto.createDecipher(algorithm, password) -* `options` {Object} - * `host` {string} Host the client should connect to. - * `port` {number} Port the client should connect to. - * `socket` {net.Socket} Establish secure connection on a given socket rather - than creating a new socket. If this option is specified, `host` and `port` - are ignored. - * `path` {string} Creates unix socket connection to path. If this option is - specified, `host` and `port` are ignored. - * `pfx` {string|Buffer} A string or `Buffer` containing the private key, - certificate, and CA certs of the client in PFX or PKCS12 format. - * `key` {string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of - strings, or array of `Buffer`s containing the private key of the client in - PEM format. - * `passphrase` {string} A string containing the passphrase for the private key - or pfx. - * `cert` {string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of - strings, or array of `Buffer`s containing the certificate key of the client - in PEM format. - * `ca` {string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of strings, - or array of `Buffer`s of trusted certificates in PEM format. If this is - omitted several well known "root" CAs (like VeriSign) will be used. These - are used to authorize connections. - * `ciphers` {string} A string describing the ciphers to use or exclude, - separated by `:`. Uses the same default cipher suite as - [`tls.createServer()`][]. - * `rejectUnauthorized` {boolean} If `true`, the server certificate is verified - against the list of supplied CAs. An `'error'` event is emitted if - verification fails; `err.code` contains the OpenSSL error code. Defaults to - `true`. - * `NPNProtocols` {string[]|Buffer[]} An array of strings or `Buffer`s - containing supported NPN protocols. `Buffer`s should have the format - `[len][name][len][name]...` e.g. `0x05hello0x05world`, where the first - byte is the length of the next protocol name. Passing an array is usually - much simpler, e.g. `['hello', 'world']`. - * `ALPNProtocols`: {string[]|Buffer[]} An array of strings or `Buffer`s - containing the supported ALPN protocols. `Buffer`s should have the format - `[len][name][len][name]...` e.g. `0x05hello0x05world`, where the first byte - is the length of the next protocol name. Passing an array is usually much - simpler: `['hello', 'world']`.) - * `servername`: {string} Server name for the SNI (Server Name Indication) TLS - extension. - * `checkServerIdentity(servername, cert)` {Function} A callback function - to be used when checking the server's hostname against the certificate. - This should throw an error if verification fails. The method should return - `undefined` if the `servername` and `cert` are verified. - * `secureProtocol` {string} The SSL method to use, e.g. `SSLv3_method` to - force SSL version 3. The possible values depend on the version of OpenSSL - installed in the environment and are defined in the constant - [SSL_METHODS][]. - * `secureContext` {object} An optional TLS context object as returned by from - `tls.createSecureContext( ... )`. It can be used for caching client - certificates, keys, and CA certificates. - * `session` {Buffer} A `Buffer` instance, containing TLS session. - * `minDHSize` {number} Minimum size of the DH parameter in bits to accept a - TLS connection. When a server offers a DH parameter with a size less - than `minDHSize`, the TLS connection is destroyed and an error is thrown. - Defaults to `1024`. -* `callback` {Function} +* `port` {number} Default value for `options.port`. +* `host` {string} Optional default value for `options.host`. +* `options` {Object} See [`tls.connect()`][]. +* `callback` {Function} See [`tls.connect()`][]. -Creates a new client connection to the given `options.port` and `options.host` -If `options.host` is omitted, it defaults to `localhost`. +Same as [`tls.connect()`][] except that `port` and `host` can be provided +as arguments instead of options. -The `callback` function, if specified, will be added as a listener for the -[`'secureConnect'`][] event. +*Note*: A port or host option, if specified, will take precedence over any port +or host argument. -`tls.connect()` returns a [`tls.TLSSocket`][] object. +## tls.connect(path[, options][, callback]) + -## tls.connect(port[, host][, options][, callback]) +* `path` {string} Default value for `options.path`. +* `options` {Object} See [`tls.connect()`][]. +* `callback` {Function} See [`tls.connect()`][]. + +Same as [`tls.connect()`][] except that `path` can be provided +as an argument instead of an option. + +*Note*: A path option, if specified, will take precedence over the path +argument. + +## tls.connect(options[, callback]) -* `port` {number} -* `host` {string} * `options` {Object} - * `host` {string} Host the client should connect to. + * `host` {string} Host the client should connect to, defaults to 'localhost'. * `port` {number} Port the client should connect to. - * `socket` {net.Socket} Establish secure connection on a given socket rather - than creating a new socket. If this option is specified, `host` and `port` - are ignored. * `path` {string} Creates unix socket connection to path. If this option is specified, `host` and `port` are ignored. - * `pfx` {string|Buffer} A string or `Buffer` containing the private key, - certificate, and CA certs of the client in PFX or PKCS12 format. - * `key` {string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of - strings, or array of `Buffer`s containing the private key of the client in - PEM format. - * `passphrase` {string} A string containing the passphrase for the private key - or pfx. - * `cert` {string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of - strings, or array of `Buffer`s containing the certificate key of the client - in PEM format. - * `ca` {string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of strings, - or array of `Buffer`s of trusted certificates in PEM format. If this is - omitted several well known "root" CAs (like VeriSign) will be used. These - are used to authorize connections. - * `ciphers` {string} A string describing the ciphers to use or exclude, - separated by `:`. Uses the same default cipher suite as - [`tls.createServer()`][]. + * `socket` {net.Socket} Establish secure connection on a given socket rather + than creating a new socket. If this option is specified, `path`, `host` and + `port` are ignored. Usually, a socket is already connected when passed to + `tls.connect()`, but it can be connected later. Note that + connection/disconnection/destruction of `socket` is the user's + responsibility, calling `tls.connect()` will not cause `net.connect()` to be + called. * `rejectUnauthorized` {boolean} If `true`, the server certificate is verified against the list of supplied CAs. An `'error'` event is emitted if verification fails; `err.code` contains the OpenSSL error code. Defaults to @@ -833,24 +806,21 @@ added: v0.11.3 to be used when checking the server's hostname against the certificate. This should throw an error if verification fails. The method should return `undefined` if the `servername` and `cert` are verified. - * `secureProtocol` {string} The SSL method to use, e.g. `SSLv3_method` to - force SSL version 3. The possible values depend on the version of OpenSSL - installed in the environment and are defined in the constant - [SSL_METHODS][]. - * `secureContext` {object} An optional TLS context object as returned by from - `tls.createSecureContext( ... )`. It can be used for caching client - certificates, keys, and CA certificates. * `session` {Buffer} A `Buffer` instance, containing TLS session. * `minDHSize` {number} Minimum size of the DH parameter in bits to accept a TLS connection. When a server offers a DH parameter with a size less than `minDHSize`, the TLS connection is destroyed and an error is thrown. Defaults to `1024`. + * `secureContext`: Optional TLS context object created with + [`tls.createSecureContext()`][]. If a `secureContext` is _not_ provided, one + will be created by passing the entire `options` object to + `tls.createSecureContext()`. *Note*: In effect, all + [`tls.createSecureContext()`][] options can be provided, but they will be + _completely ignored_ unless the `secureContext` option is missing. + * ...: Optional [`tls.createSecureContext()`][] options can be provided, see + the `secureContext` option for more information. * `callback` {Function} -Creates a new client connection to the given `port` and `host` or -`options.port` and `options.host`. (If `host` is omitted, it defaults to -`localhost`.) - The `callback` function, if specified, will be added as a listener for the [`'secureConnect'`][] event. @@ -918,31 +888,72 @@ added: v0.11.13 --> * `options` {Object} - * `pfx` {string|Buffer} A string or `Buffer` holding the PFX or PKCS12 encoded - private key, certificate, and CA certificates. - * `key` {string|string[]|Buffer|Object[]} The private key of the server in - PEM format. To support multiple keys using different algorithms, an array - can be provided either as an array of key strings or as an array of objects - in the format `{pem: key, passphrase: passphrase}`. This option is - *required* for ciphers that make use of private keys. - * `passphrase` {string} A string containing the passphrase for the private key - or pfx. - * `cert` {string} A string containing the PEM encoded certificate - * `ca`{string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of strings, - or array of `Buffer`s of trusted certificates in PEM format. If omitted, - several well known "root" CAs (like VeriSign) will be used. These are used - to authorize connections. - * `crl` {string|string[]} Either a string or array of strings of PEM encoded - CRLs (Certificate Revocation List). - * `ciphers` {string} A string describing the ciphers to use or exclude. - Consult - - for details on the format. - * `honorCipherOrder` {boolean} If `true`, when a cipher is being selected, - the server's preferences will be used instead of the client preferences. + * `pfx` {string|Buffer} Optional PFX or PKCS12 encoded private key and + certificate chain. `pfx` is an alternative to providing `key` and `cert` + individually. PFX is usually encrypted, if it is, `passphrase` will be used + to decrypt it. + * `key` {string|string[]|Buffer|Buffer[]|Object[]} Optional private keys in + PEM format. Single keys will be decrypted with `passphrase` if necessary. + Multiple keys, probably using different algorithms, can be provided either + as an array of unencrypted key strings or buffers, or an array of objects in + the form `{pem: , passphrase: }`. The object form can + only occur in an array, and it _must_ include a passphrase, even if key is + not encrypted. + * `passphrase` {string} Optional shared passphrase used for a single private + key and/or a PFX. + * `cert` {string|string[]|Buffer|Buffer[]} Optional cert chains in PEM format. + One cert chain should be provided per private key. Each cert chain should + consist of the PEM formatted certificate for a provided private `key`, + followed by the PEM formatted intermediate certificates (if any), in order, + and not including the root CA (the root CA must be pre-known to the peer, + see `ca`). When providing multiple cert chains, they do not have to be in + the same order as their private keys in `key`. If the intermediate + certificates are not provided, the peer will not be able to validate the + certificate, and the handshake will fail. + * `ca`{string|string[]|Buffer|Buffer[]} Optional CA certificates to trust. + Default is the well-known CAs from Mozilla. When connecting to peers that + use certificates issued privately, or self-signed, the private root CA or + self-signed certificate must be provided to verify the peer. + * `crl` {string|string[]|Buffer|Buffer[]} Optional PEM formatted + CRLs (Certificate Revocation Lists). + * `ciphers` {string} Optional cipher suite specification, replacing the + default. For more information, see [modifying the default cipher suite][]. + * `honorCipherOrder` {boolean} Attempt to use the server's cipher suite + preferences instead of the client's. When `true`, causes + `SSL_OP_CIPHER_SERVER_PREFERENCE` to be set in `secureOptions`, see + [OpenSSL Options][] for more information. + *Note*: [`tls.createServer()`][] sets the default value to `true`, other + APIs that create secure contexts leave it unset. + * `ecdhCurve` {string} A string describing a named curve to use for ECDH key + agreement or `false` to disable ECDH. Defaults to `prime256v1` (NIST P-256). + Use [`crypto.getCurves()`][] to obtain a list of available curve names. On + recent releases, `openssl ecparam -list_curves` will also display the name + and description of each available elliptic curve. + * `dhparam` {string|Buffer} Diffie Hellman parameters, required for + [Perfect Forward Secrecy][]. Use `openssl dhparam` to create the parameters. + The key length must be greater than or equal to 1024 bits, otherwise an + error will be thrown. It is strongly recommended to use 2048 bits or larger + for stronger security. If omitted or invalid, the parameters are silently + discarded and DHE ciphers will not be available. + * `secureProtocol` {string} Optional SSL method to use, default is + `"SSLv23_method"`. The possible values are listed as [SSL_METHODS][], use + the function names as strings. For example, `"SSLv3_method"` to force SSL + version 3. + * `secureOptions` {number} Optionally affect the OpenSSL protocol behaviour, + which is not usually necessary. This should be used carefully if at all! + Value is a numeric bitmask of the `SSL_OP_*` options from + [OpenSSL Options][]. + * `sessionIdContext` {string} Optional opaque identifier used by servers to + ensure session state is not shared between applications. Unused by clients. + *Note*: [`tls.createServer()`][] uses a 128 bit truncated SHA1 hash value + generated from `process.argv`, other APIs that create secure contexts + have no default value. The `tls.createSecureContext()` method creates a credentials object. +A key is *required* for ciphers that make use of certificates. Either `key` or +`pfx` can be used to provide it. + If the 'ca' option is not given, then Node.js will use the default publicly trusted list of CAs as given in . @@ -954,45 +965,10 @@ added: v0.3.2 --> * `options` {Object} - * `pfx` {string|Buffer} A string or `Buffer` containing the private key, - certificate and CA certs of the server in PFX or PKCS12 format. (Mutually - exclusive with the `key`, `cert`, and `ca` options.) - * `key` {string|string[]|Buffer|Object[]} The private key of the server in - PEM format. To support multiple keys using different algorithms an array can - be provided either as a plain array of key strings or an array of objects - in the format `{pem: key, passphrase: passphrase}`. This option is - *required* for ciphers that make use of private keys. - * `passphrase` {string} A string containing the passphrase for the private - key or pfx. - * `cert` {string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of - strings, or array of `Buffer`s containing the certificate key of the server - in PEM format. (Required) - * `ca` {string|string[]|Buffer|Buffer[]} A string, `Buffer`, array of strings, - or array of `Buffer`s of trusted certificates in PEM format. If this is - omitted several well known "root" CAs (like VeriSign) will be used. These - are used to authorize connections. - * `crl` {string|string[]} Either a string or array of strings of PEM encoded - CRLs (Certificate Revocation List). - * `ciphers` {string} A string describing the ciphers to use or exclude, - separated by `:`. - * `ecdhCurve` {string} A string describing a named curve to use for ECDH key - agreement or `false` to disable ECDH. Defaults to `prime256v1` (NIST P-256). - Use [`crypto.getCurves()`][] to obtain a list of available curve names. On - recent releases, `openssl ecparam -list_curves` will also display the name - and description of each available elliptic curve. - * `dhparam` {string|Buffer} A string or `Buffer` containing Diffie Hellman - parameters, required for [Perfect Forward Secrecy][]. Use - `openssl dhparam` to create the parameters. The key length must be greater - than or equal to 1024 bits, otherwise an error will be thrown. It is - strongly recommended to use 2048 bits or larger for stronger security. If - omitted or invalid, the parameters are silently discarded and DHE ciphers - will not be available. * `handshakeTimeout` {number} Abort the connection if the SSL/TLS handshake does not finish in the specified number of milliseconds. Defaults to `120` seconds. A `'clientError'` is emitted on the `tls.Server` object whenever a handshake times out. - * `honorCipherOrder` {boolean} When choosing a cipher, use the server's - preferences instead of the client preferences. Defaults to `true`. * `requestCert` {boolean} If `true` the server will request a certificate from clients that connect and attempt to verify that certificate. Defaults to `false`. @@ -1019,58 +995,13 @@ added: v0.3.2 a 16-byte HMAC key, and a 16-byte AES key. This can be used to accept TLS session tickets on multiple instances of the TLS server. *Note* that this is automatically shared between `cluster` module workers. - * `sessionIdContext` {string} A string containing an opaque identifier for - session resumption. If `requestCert` is `true`, the default is a 128 bit - truncated SHA1 hash value generated from the command-line. Otherwise, a - default is not provided. - * `secureProtocol` {string} The SSL method to use, e.g. `SSLv3_method` to - force SSL version 3. The possible values depend on the version of OpenSSL - installed in the environment and are defined in the constant - [SSL_METHODS][]. + * ...: Any [`tls.createSecureContext()`][] options can be provided. For + servers, the identity options (`pfx` or `key`/`cert`) are usually required. * `secureConnectionListener` {Function} Creates a new [tls.Server][]. The `secureConnectionListener`, if provided, is automatically set as a listener for the [`'secureConnection'`][] event. -For the `ciphers` option, the default cipher suite is: - -```text -ECDHE-RSA-AES128-GCM-SHA256: -ECDHE-ECDSA-AES128-GCM-SHA256: -ECDHE-RSA-AES256-GCM-SHA384: -ECDHE-ECDSA-AES256-GCM-SHA384: -DHE-RSA-AES128-GCM-SHA256: -ECDHE-RSA-AES128-SHA256: -DHE-RSA-AES128-SHA256: -ECDHE-RSA-AES256-SHA384: -DHE-RSA-AES256-SHA384: -ECDHE-RSA-AES256-SHA256: -DHE-RSA-AES256-SHA256: -HIGH: -!aNULL: -!eNULL: -!EXPORT: -!DES: -!RC4: -!MD5: -!PSK: -!SRP: -!CAMELLIA -``` - -The default cipher suite prefers GCM ciphers for [Chrome's 'modern -cryptography' setting] and also prefers ECDHE and DHE ciphers for Perfect -Forward Secrecy, while offering *some* backward compatibility. - -128 bit AES is preferred over 192 and 256 bit AES in light of [specific -attacks affecting larger AES key sizes]. - -Old clients that rely on insecure and deprecated RC4 or DES-based ciphers -(like Internet Explorer 6) cannot complete the handshaking process with -the default configuration. If these clients _must_ be supported, the -[TLS recommendations] may offer a compatible cipher suite. For more details -on the format, see the [OpenSSL cipher list format documentation]. - The following illustrates a simple echo server: ```js @@ -1254,6 +1185,8 @@ where `secure_socket` has the same API as `pair.cleartext`. [OpenSSL cipher list format documentation]: https://www.openssl.org/docs/man1.0.2/apps/ciphers.html#CIPHER-LIST-FORMAT [Chrome's 'modern cryptography' setting]: https://www.chromium.org/Home/chromium-security/education/tls#TOC-Cipher-Suites +[OpenSSL Options]: crypto.html#crypto_openssl_options +[modifying the default cipher suite]: #tls_modifying_the_default_tls_cipher_suite [specific attacks affecting larger AES key sizes]: https://www.schneier.com/blog/archives/2009/07/another_new_aes.html [`crypto.getCurves()`]: crypto.html#crypto_crypto_getcurves [`tls.createServer()`]: #tls_tls_createserver_options_secureconnectionlistener diff --git a/test/fixtures/raw-key.pem b/test/fixtures/raw-key.pem new file mode 100644 index 00000000000000..3e27f9f8870ad0 --- /dev/null +++ b/test/fixtures/raw-key.pem @@ -0,0 +1,15 @@ +-----BEGIN RSA PRIVATE KEY----- +MIICXAIBAAKBgQChmQeFwsaomtQbw9Nm55Dn6KSR9bkY8PDroQUeTNa90BlIbhGs +KYm4l7bERaasFgOrkcQpk45fdDVYPjKxraZiGXXKjSIDYeDAIC/+CkwQKrejgCPm +Js4gV4g+npvwi1gVr2NAg7fkJOyEW2TDp4dsAD8qtG8Aml0C1hJXwFYmBwIDAQAB +AoGAVgZpAsQVjVwe3kj5GSbc9Rfbw/fTeXuKRWWKm/67soA9dVli/wt9zU62dPW/ +LIzrl0IZ8ygh+p6aZ0d1JTEUCPx7e0KocCmNg77i5AG0eK5i/KKjTWB4UGRDylfD +dnBXQc814bK+VB0mrcp46U/7tLGYkV2Kz/LiNpmxKwITS4ECQQDPoA6WIU87Eulq +OuVmJnFIQ2IR3SycVisO7TUq2MItq2U4BwsA3aQ4ehpP/uJdAfJEfwi2omRV5pGb +806pWkfPAkEAxz+igHS8tR11aLck71dD4BRBY7XZCUg6G4zmYYWsqj0yvM6c4Yn0 +HRcrZqFvV/xuMFphWEmMBhrqLvgy66yUSQJBALkei4LeRid0sDswMhMHGaAFvG4T +FtB5n8CaTPpb854GoKP42521ANP+QnGq36dvsdPStDEqz20rvA4hPLSQs08CQCV8 +eWxFikNg+XfsDQzilCiSZwMFcYHnjtckGSv75FJbFTKkhKuCMuVOOKIkeThKi8iZ +GHttyuRTKAASPjJM09ECQBrhlKJwYKuUDMp3qkLBgrXYqbFxZtkS2GeFMUfLcRlx +oMrTFEczz9lZ0huTuQYPeAAOY0Gd84mL0kQqTRTzNLs= +-----END RSA PRIVATE KEY----- diff --git a/test/parallel/test-tls-passphrase.js b/test/parallel/test-tls-passphrase.js index 8999f470187f78..319c3511dce7ee 100644 --- a/test/parallel/test-tls-passphrase.js +++ b/test/parallel/test-tls-passphrase.js @@ -1,21 +1,27 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); +const common = require('../common'); +const assert = require('assert'); if (!common.hasCrypto) { common.skip('missing crypto'); return; } -var tls = require('tls'); +const tls = require('tls'); -var fs = require('fs'); -var path = require('path'); +const fs = require('fs'); +const path = require('path'); -var key = fs.readFileSync(path.join(common.fixturesDir, 'pass-key.pem')); -var cert = fs.readFileSync(path.join(common.fixturesDir, 'pass-cert.pem')); +const passKey = fs.readFileSync(path.join(common.fixturesDir, 'pass-key.pem')); +const rawKey = fs.readFileSync(path.join(common.fixturesDir, 'raw-key.pem')); +const cert = fs.readFileSync(path.join(common.fixturesDir, 'pass-cert.pem')); -var server = tls.Server({ - key: key, +assert(Buffer.isBuffer(passKey)); +assert(Buffer.isBuffer(cert)); +assert.strictEqual(typeof passKey.toString(), 'string'); +assert.strictEqual(typeof cert.toString(), 'string'); + +const server = tls.Server({ + key: passKey, passphrase: 'passphrase', cert: cert, ca: [cert], @@ -26,24 +32,174 @@ var server = tls.Server({ }); server.listen(0, common.mustCall(function() { - var c = tls.connect({ + // Buffer + tls.connect({ port: this.address().port, - key: key, + key: passKey, passphrase: 'passphrase', cert: cert, rejectUnauthorized: false }, common.mustCall(function() {})); - c.on('end', function() { - server.close(); - }); -})); + + tls.connect({ + port: this.address().port, + key: rawKey, + cert: cert, + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: rawKey, + passphrase: 'passphrase', // Ignored. + cert: cert, + rejectUnauthorized: false + }, common.mustCall(function() {})); + + // Buffer[] + /* XXX(sam) Should work, but its unimplemented ATM. + tls.connect({ + port: this.address().port, + key: [passKey], + passphrase: 'passphrase', + cert: [cert], + rejectUnauthorized: false + }, common.mustCall(function() {})); + */ + + tls.connect({ + port: this.address().port, + key: [rawKey], + cert: [cert], + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: [rawKey], + passphrase: 'passphrase', // Ignored. + cert: [cert], + rejectUnauthorized: false + }, common.mustCall(function() {})); + + // string + tls.connect({ + port: this.address().port, + key: passKey.toString(), + passphrase: 'passphrase', + cert: cert.toString(), + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: rawKey.toString(), + cert: cert.toString(), + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: rawKey.toString(), + passphrase: 'passphrase', // Ignored. + cert: cert.toString(), + rejectUnauthorized: false + }, common.mustCall(function() {})); + + // String[] + /* XXX(sam) Should work, but its unimplemented ATM. + tls.connect({ + port: this.address().port, + key: [passKey.toString()], + passphrase: 'passphrase', + cert: [cert.toString()], + rejectUnauthorized: false + }, common.mustCall(function() {})); + */ + + tls.connect({ + port: this.address().port, + key: [rawKey.toString()], + cert: [cert.toString()], + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: [rawKey.toString()], + passphrase: 'passphrase', // Ignored. + cert: [cert.toString()], + rejectUnauthorized: false + }, common.mustCall(function() {})); + + // Object[] + tls.connect({ + port: this.address().port, + key: [{pem: passKey, passphrase: 'passphrase'}], + cert: cert, + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: [{pem: passKey.toString(), passphrase: 'passphrase'}], + cert: cert, + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: [{pem: rawKey, passphrase: 'passphrase'}], + cert: cert, + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: [{pem: rawKey.toString(), passphrase: 'passphrase'}], + cert: cert, + rejectUnauthorized: false + }, common.mustCall(function() {})); + + /* XXX(sam) Should work, but unimplemented ATM + tls.connect({ + port: this.address().port, + key: [{pem: rawKey}], + passphrase: 'passphrase', + cert: cert, + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: [{pem: rawKey.toString()}], + passphrase: 'passphrase', + cert: cert, + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: [{pem: rawKey}], + cert: cert, + rejectUnauthorized: false + }, common.mustCall(function() {})); + + tls.connect({ + port: this.address().port, + key: [{pem: rawKey.toString()}], + cert: cert, + rejectUnauthorized: false + }, common.mustCall(function() {})); + */ +})).unref(); assert.throws(function() { tls.connect({ port: server.address().port, - key: key, + key: passKey, passphrase: 'invalid', cert: cert, rejectUnauthorized: false }); -}); +}, /bad decrypt/); From db50307d5c555da1d81c5b5d736853ce8474c6d1 Mon Sep 17 00:00:00 2001 From: Sam Roberts Date: Fri, 9 Dec 2016 11:08:51 -0800 Subject: [PATCH 67/84] test: var to const in tls-no-cert-required PR-URL: https://github.com/nodejs/node/pull/9800 Reviewed-By: Roman Reiss Reviewed-By: Michael Dawson --- test/parallel/test-tls-no-cert-required.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-tls-no-cert-required.js b/test/parallel/test-tls-no-cert-required.js index 8d907d9f8a4e06..3c7cf4462fcd07 100644 --- a/test/parallel/test-tls-no-cert-required.js +++ b/test/parallel/test-tls-no-cert-required.js @@ -1,12 +1,12 @@ 'use strict'; -var assert = require('assert'); -var common = require('../common'); +const assert = require('assert'); +const common = require('../common'); if (!common.hasCrypto) { common.skip('missing crypto'); return; } -var tls = require('tls'); +const tls = require('tls'); // Omitting the cert or pfx option to tls.createServer() should not throw. // AECDH-NULL-SHA is a no-authentication/no-encryption cipher and hence From fa4f1587d34c8d18ab93b7c3e9fc37d7e05697a9 Mon Sep 17 00:00:00 2001 From: Amar Zavery Date: Thu, 1 Dec 2016 10:27:02 -0600 Subject: [PATCH 68/84] test: replace var with const in test-require-dot PR-URL: https://github.com/nodejs/node/pull/9916 Reviewed-By: Colin Ihrig Reviewed-By: Teddy Katz Reviewed-By: Gibson Fahnestock Reviewed-By: James M Snell Reviewed-By: Italo A. Casas --- test/parallel/test-require-dot.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-require-dot.js b/test/parallel/test-require-dot.js index 2fb161486a9241..26733c7fc438a1 100644 --- a/test/parallel/test-require-dot.js +++ b/test/parallel/test-require-dot.js @@ -1,17 +1,17 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); -var module = require('module'); +const common = require('../common'); +const assert = require('assert'); +const m = require('module'); -var a = require(common.fixturesDir + '/module-require/relative/dot.js'); -var b = require(common.fixturesDir + '/module-require/relative/dot-slash.js'); +const a = require(common.fixturesDir + '/module-require/relative/dot.js'); +const b = require(common.fixturesDir + '/module-require/relative/dot-slash.js'); -assert.equal(a.value, 42); -assert.equal(a, b, 'require(".") should resolve like require("./")'); +assert.strictEqual(a.value, 42); +assert.strictEqual(a, b, 'require(".") should resolve like require("./")'); process.env.NODE_PATH = common.fixturesDir + '/module-require/relative'; -module._initPaths(); +m._initPaths(); -var c = require('.'); +const c = require('.'); -assert.equal(c.value, 42, 'require(".") should honor NODE_PATH'); +assert.strictEqual(c.value, 42, 'require(".") should honor NODE_PATH'); From a8e87084cf7903f6ec12ef43acde921d8e32d5d6 Mon Sep 17 00:00:00 2001 From: Adrian Estrada Date: Wed, 7 Dec 2016 11:20:27 -0500 Subject: [PATCH 69/84] test: add ES6 and strictEqual to test-fs-truncate - use const and let for variables - replace assert.equal with assert.strictEqual PR-URL: https://github.com/nodejs/node/pull/10167 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Italo A. Casas --- test/parallel/test-fs-truncate.js | 42 +++++++++++++++---------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/test/parallel/test-fs-truncate.js b/test/parallel/test-fs-truncate.js index 442038eeb50a51..1ba0db7f012fe4 100644 --- a/test/parallel/test-fs-truncate.js +++ b/test/parallel/test-fs-truncate.js @@ -1,43 +1,43 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); -var path = require('path'); -var fs = require('fs'); -var tmp = common.tmpDir; -var filename = path.resolve(tmp, 'truncate-file.txt'); -var data = Buffer.alloc(1024 * 16, 'x'); +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const tmp = common.tmpDir; +const filename = path.resolve(tmp, 'truncate-file.txt'); +const data = Buffer.alloc(1024 * 16, 'x'); common.refreshTmpDir(); -var stat; +let stat; // truncateSync fs.writeFileSync(filename, data); stat = fs.statSync(filename); -assert.equal(stat.size, 1024 * 16); +assert.strictEqual(stat.size, 1024 * 16); fs.truncateSync(filename, 1024); stat = fs.statSync(filename); -assert.equal(stat.size, 1024); +assert.strictEqual(stat.size, 1024); fs.truncateSync(filename); stat = fs.statSync(filename); -assert.equal(stat.size, 0); +assert.strictEqual(stat.size, 0); // ftruncateSync fs.writeFileSync(filename, data); -var fd = fs.openSync(filename, 'r+'); +const fd = fs.openSync(filename, 'r+'); stat = fs.statSync(filename); -assert.equal(stat.size, 1024 * 16); +assert.strictEqual(stat.size, 1024 * 16); fs.ftruncateSync(fd, 1024); stat = fs.statSync(filename); -assert.equal(stat.size, 1024); +assert.strictEqual(stat.size, 1024); fs.ftruncateSync(fd); stat = fs.statSync(filename); -assert.equal(stat.size, 0); +assert.strictEqual(stat.size, 0); fs.closeSync(fd); @@ -54,19 +54,19 @@ function testTruncate(cb) { if (er) return cb(er); fs.stat(filename, function(er, stat) { if (er) return cb(er); - assert.equal(stat.size, 1024 * 16); + assert.strictEqual(stat.size, 1024 * 16); fs.truncate(filename, 1024, function(er) { if (er) return cb(er); fs.stat(filename, function(er, stat) { if (er) return cb(er); - assert.equal(stat.size, 1024); + assert.strictEqual(stat.size, 1024); fs.truncate(filename, function(er) { if (er) return cb(er); fs.stat(filename, function(er, stat) { if (er) return cb(er); - assert.equal(stat.size, 0); + assert.strictEqual(stat.size, 0); cb(); }); }); @@ -82,7 +82,7 @@ function testFtruncate(cb) { if (er) return cb(er); fs.stat(filename, function(er, stat) { if (er) return cb(er); - assert.equal(stat.size, 1024 * 16); + assert.strictEqual(stat.size, 1024 * 16); fs.open(filename, 'w', function(er, fd) { if (er) return cb(er); @@ -90,13 +90,13 @@ function testFtruncate(cb) { if (er) return cb(er); fs.stat(filename, function(er, stat) { if (er) return cb(er); - assert.equal(stat.size, 1024); + assert.strictEqual(stat.size, 1024); fs.ftruncate(fd, function(er) { if (er) return cb(er); fs.stat(filename, function(er, stat) { if (er) return cb(er); - assert.equal(stat.size, 0); + assert.strictEqual(stat.size, 0); fs.close(fd, cb); }); }); From 499fc7ab1b455280900ee5c6be34bea95c8e1771 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 8 Dec 2016 22:26:28 -0800 Subject: [PATCH 70/84] test: refactor test-handle-wrap-close-abort * use common.mustCall() to confirm number of uncaught exceptions * var -> const * specify duration of 1ms for setTimeout() and setInterval() PR-URL: https://github.com/nodejs/node/pull/10188 Reviewed-By: Santiago Gimeno Reviewed-By: Colin Ihrig Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-handle-wrap-close-abort.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/parallel/test-handle-wrap-close-abort.js b/test/parallel/test-handle-wrap-close-abort.js index 8572668f666864..5355e65df60821 100644 --- a/test/parallel/test-handle-wrap-close-abort.js +++ b/test/parallel/test-handle-wrap-close-abort.js @@ -1,16 +1,16 @@ 'use strict'; -require('../common'); +const common = require('../common'); -process.on('uncaughtException', function() { }); +process.on('uncaughtException', common.mustCall(function() {}, 2)); setTimeout(function() { process.nextTick(function() { - var c = setInterval(function() { + const c = setInterval(function() { clearInterval(c); throw new Error('setInterval'); - }); + }, 1); }); setTimeout(function() { throw new Error('setTimeout'); - }); + }, 1); }); From 50cb3a3711e1f9d522f7d0865c41546d6857b847 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Sun, 11 Dec 2016 01:22:04 +0200 Subject: [PATCH 71/84] doc: document argument variant in the repl.md `options` in the `repl.start([options])` can be a string. Ref: https://github.com/nodejs/node/pull/10160 PR-URL: https://github.com/nodejs/node/pull/10221 Reviewed-By: Benjamin Gruenbaum --- doc/api/repl.md | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index b78a13544add4a..9ff416e004f4b3 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -371,7 +371,7 @@ within the action function for commands registered using the added: v0.1.91 --> -* `options` {Object} +* `options` {Object | String} * `prompt` {String} The input prompt to display. Defaults to `> `. * `input` {Readable} The Readable stream from which REPL input will be read. Defaults to `process.stdin`. @@ -413,6 +413,15 @@ added: v0.1.91 The `repl.start()` method creates and starts a `repl.REPLServer` instance. +If `options` is a string, then it specifies the input prompt: + +```js +const repl = require('repl'); + +// a Unix style prompt +repl.start('$ '); +``` + ## The Node.js REPL Node.js itself uses the `repl` module to provide its own interactive interface From 7346e55750f9929397432acfa3add02632bb52b4 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 8 Dec 2016 22:46:39 -0800 Subject: [PATCH 72/84] test: refactor http pipelined socket test In test-http-incoming-pipelined-socket-destory: * setTimeout() with no duration -> setImmediate() * eliminate unneeded exit listener * use common.mustCall() * var -> const/let PR-URL: https://github.com/nodejs/node/pull/10189 Reviewed-By: Santiago Gimeno Reviewed-By: Colin Ihrig Reviewed-By: Sakthipriyan Vairamani --- ...-http-incoming-pipelined-socket-destroy.js | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/test/parallel/test-http-incoming-pipelined-socket-destroy.js b/test/parallel/test-http-incoming-pipelined-socket-destroy.js index 9789e9976cb7a7..1465f189dd2045 100644 --- a/test/parallel/test-http-incoming-pipelined-socket-destroy.js +++ b/test/parallel/test-http-incoming-pipelined-socket-destroy.js @@ -1,16 +1,17 @@ 'use strict'; -require('../common'); +const common = require('../common'); -var http = require('http'); -var net = require('net'); +const http = require('http'); +const net = require('net'); +const seeds = [ 3, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4 ]; // Set up some timing issues where sockets can be destroyed // via either the req or res. -var server = http.createServer(function(req, res) { +const server = http.createServer(common.mustCall(function(req, res) { switch (req.url) { case '/1': - return setTimeout(function() { + return setImmediate(function() { req.socket.destroy(); server.emit('requestDone'); }); @@ -24,7 +25,7 @@ var server = http.createServer(function(req, res) { // in one case, actually send a response in 2 chunks case '/3': res.write('hello '); - return setTimeout(function() { + return setImmediate(function() { res.end('world!'); server.emit('requestDone'); }); @@ -33,7 +34,7 @@ var server = http.createServer(function(req, res) { res.destroy(); server.emit('requestDone'); } -}); +}, seeds.length)); // Make a bunch of requests pipelined on the same socket @@ -47,10 +48,9 @@ function generator(seeds) { } -server.listen(0, function() { - var seeds = [ 3, 1, 2, 3, 4, 1, 2, 3, 4, 1, 2, 3, 4 ]; - var client = net.connect({ port: this.address().port }); - var done = 0; +server.listen(0, common.mustCall(function() { + const client = net.connect({ port: this.address().port }); + let done = 0; server.on('requestDone', function() { if (++done === seeds.length) { server.close(); @@ -60,9 +60,4 @@ server.listen(0, function() { // immediately write the pipelined requests. // Some of these will not have a socket to destroy! client.write(generator(seeds)); -}); - -process.on('exit', function(c) { - if (!c) - console.log('ok'); -}); +})); From 10891a17547a0b99169d7b1b1ee4d61ae108da9c Mon Sep 17 00:00:00 2001 From: "Prieto, Marcos" Date: Thu, 1 Dec 2016 11:24:16 -0600 Subject: [PATCH 73/84] test: refactor assert.equal, update syntax to ES6 PR-URL: https://github.com/nodejs/node/pull/10190 Reviewed-By: Colin Ihrig Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Gibson Fahnestock --- test/parallel/test-buffer-arraybuffer.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/parallel/test-buffer-arraybuffer.js b/test/parallel/test-buffer-arraybuffer.js index f40c43939f975e..4353af17ef1d3a 100644 --- a/test/parallel/test-buffer-arraybuffer.js +++ b/test/parallel/test-buffer-arraybuffer.js @@ -13,9 +13,9 @@ const buf = Buffer.from(ab); assert.ok(buf instanceof Buffer); -assert.equal(buf.parent, buf.buffer); -assert.equal(buf.buffer, ab); -assert.equal(buf.length, ab.byteLength); +assert.strictEqual(buf.parent, buf.buffer); +assert.strictEqual(buf.buffer, ab); +assert.strictEqual(buf.length, ab.byteLength); buf.fill(0xC); @@ -44,7 +44,7 @@ assert.throws(function() { }, TypeError); // write{Double,Float}{LE,BE} with noAssert should not crash, cf. #3766 -var b = Buffer.allocUnsafe(1); +const b = Buffer.allocUnsafe(1); b.writeFloatLE(11.11, 0, true); b.writeFloatBE(11.11, 0, true); b.writeDoubleLE(11.11, 0, true); From 8bad37a547d74e084d113bae6b8f3a2ea6d964a7 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 10 Dec 2016 21:12:58 -0800 Subject: [PATCH 74/84] test: refactor test-https-truncate * use common.mustCall() where appropriate * Buffer.allocUnsafe() -> Buffer.alloc() * do crypto check before loading any additional modules * specify 1ms duration for `setTimeout()` PR-URL: https://github.com/nodejs/node/pull/10225 Reviewed-By: Colin Ihrig Reviewed-By: Benjamin Gruenbaum Reviewed-By: Luigi Pinca --- test/parallel/test-https-truncate.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-https-truncate.js b/test/parallel/test-https-truncate.js index 4101a8c974e736..c96b385fc37fde 100644 --- a/test/parallel/test-https-truncate.js +++ b/test/parallel/test-https-truncate.js @@ -1,11 +1,12 @@ 'use strict'; const common = require('../common'); -const assert = require('assert'); if (!common.hasCrypto) { common.skip('missing crypto'); return; } + +const assert = require('assert'); const https = require('https'); const fs = require('fs'); @@ -14,7 +15,7 @@ const key = fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'); const cert = fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem'); // number of bytes discovered empirically to trigger the bug -const data = Buffer.allocUnsafe(1024 * 32 + 1); +const data = Buffer.alloc(1024 * 32 + 1); httpsTest(); @@ -36,12 +37,11 @@ function httpsTest() { } -function test(res) { - res.on('end', function() { +const test = common.mustCall(function(res) { + res.on('end', common.mustCall(function() { assert.strictEqual(res._readableState.length, 0); assert.strictEqual(bytes, data.length); - console.log('ok'); - }); + })); // Pause and then resume on each chunk, to ensure that there will be // a lone byte hanging out at the very end. @@ -49,6 +49,6 @@ function test(res) { res.on('data', function(chunk) { bytes += chunk.length; this.pause(); - setTimeout(this.resume.bind(this)); + setTimeout(this.resume.bind(this), 1); }); -} +}); From 5f18b405aa45bce957aff9f14a9c96c27a7b1ef0 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 00:49:13 +0200 Subject: [PATCH 75/84] doc: var => let / const in repl.md --- doc/api/repl.md | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 9ff416e004f4b3..acf3b41f808e65 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -78,15 +78,16 @@ The default evaluator supports direct evaluation of JavaScript expressions: ```js > 1 + 1 2 -> var m = 2 +> const m = 2 undefined > m + 1 3 ``` Unless otherwise scoped within blocks (e.g. `{ ... }`) or functions, variables -declared either implicitly or using the `var` keyword are declared at the -`global` scope. +declared either implicitly or using the `const` or `let` keywords +are declared at the `global` scope (as well as with the `var` keyword +outside of functions). #### Global and Local Scope @@ -96,7 +97,7 @@ it to the `context` object associated with each `REPLServer`. For example: ```js const repl = require('repl'); -var msg = 'message'; +const msg = 'message'; repl.start('> ').context.m = msg; ``` @@ -115,7 +116,7 @@ To specify read-only globals, context properties must be defined using ```js const repl = require('repl'); -var msg = 'message'; +const msg = 'message'; const r = repl.start('> '); Object.defineProperty(r.context, 'm', { @@ -183,7 +184,7 @@ to the provided callback function: ```js function eval(cmd, context, filename, callback) { - var result; + let result; try { result = vm.runInThisContext(cmd); } catch (e) { @@ -275,7 +276,7 @@ function initializeContext(context) { context.m = 'test'; } -var r = repl.start({prompt: '>'}); +const r = repl.start({prompt: '>'}); initializeContext(r.context); r.on('reset', initializeContext); @@ -321,7 +322,7 @@ The following example shows two new commands added to the REPL instance: ```js const repl = require('repl'); -var replServer = repl.start({prompt: '> '}); +const replServer = repl.start({prompt: '> '}); replServer.defineCommand('sayhello', { help: 'Say hello', action: function(name) { @@ -430,7 +431,7 @@ without passing any arguments (or by passing the `-i` argument): ```js $ node -> a = [1, 2, 3]; +> const a = [1, 2, 3]; [ 1, 2, 3 ] > a.forEach((v) => { ... console.log(v); @@ -502,7 +503,7 @@ socket, and a TCP socket: ```js const net = require('net'); const repl = require('repl'); -var connections = 0; +let connections = 0; repl.start({ prompt: 'Node.js via stdin> ', From bed9aaec5cc02c40c44b0020f22f6628c159d48d Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:07:03 +0200 Subject: [PATCH 76/84] doc: white space unification in repl.md Add an infix space in an argument list. Change `>` into `> ` in code bits and output examples. Explicitly clarify that default REPL prompt contains a trailing space. --- doc/api/repl.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index acf3b41f808e65..2a0f42fc7a0229 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -218,10 +218,10 @@ following example, for instance, simply converts any input text to upper case: ```js const repl = require('repl'); -const r = repl.start({prompt: '>', eval: myEval, writer: myWriter}); +const r = repl.start({prompt: '> ', eval: myEval, writer: myWriter}); function myEval(cmd, context, filename, callback) { - callback(null,cmd); + callback(null, cmd); } function myWriter(output) { @@ -276,7 +276,7 @@ function initializeContext(context) { context.m = 'test'; } -const r = repl.start({prompt: '>'}); +const r = repl.start({prompt: '> '}); initializeContext(r.context); r.on('reset', initializeContext); @@ -287,15 +287,15 @@ reset to its initial value using the `.clear` command: ```js $ ./node example.js ->m +> m 'test' ->m = 1 +> m = 1 1 ->m +> m 1 ->.clear +> .clear Clearing context... ->m +> m 'test' > ``` @@ -374,6 +374,7 @@ added: v0.1.91 * `options` {Object | String} * `prompt` {String} The input prompt to display. Defaults to `> `. + (with a trailing space). * `input` {Readable} The Readable stream from which REPL input will be read. Defaults to `process.stdin`. * `output` {Writable} The Writable stream to which REPL output will be From 9bef034e167a6fde5a975f48ca610db46233bce7 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:15:04 +0200 Subject: [PATCH 77/84] doc: fix an output example in repl.md Make `_` reassignment example match more with the current output. Extend the example for more clarity. --- doc/api/repl.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 2a0f42fc7a0229..4b10be53787f7b 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -141,6 +141,7 @@ global or scoped variable, the input `fs` will be evaluated on-demand as The default evaluator will, by default, assign the result of the most recently evaluated expression to the special variable `_` (underscore). +Explicitly setting `_` to a value will disable this behavior. ```js > [ 'a', 'b', 'c' ] @@ -148,11 +149,14 @@ evaluated expression to the special variable `_` (underscore). > _.length 3 > _ += 1 +Expression assignment to _ now disabled. +4 +> 1 + 1 +2 +> _ 4 ``` -Explicitly setting `_` to a value will disable this behavior. - ### Custom Evaluation Functions When a new `repl.REPLServer` is created, a custom evaluation function may be From 67c2a7da46dcde1fac8c83b81415d2ae738f0310 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:20:35 +0200 Subject: [PATCH 78/84] doc: fix a function name in repl.md `eval` => `myEval` to not shadow the global `eval` --- doc/api/repl.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 4b10be53787f7b..2ffdfd701148c5 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -187,7 +187,7 @@ multi-line input, the eval function can return an instance of `repl.Recoverable` to the provided callback function: ```js -function eval(cmd, context, filename, callback) { +function myEval(cmd, context, filename, callback) { let result; try { result = vm.runInThisContext(cmd); From d665e3920263a7808c174a1bb3511dfb3d987d4d Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:22:35 +0200 Subject: [PATCH 79/84] doc: name anonymous functions in repl.md --- doc/api/repl.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 2ffdfd701148c5..62560a68ae1f50 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -329,14 +329,14 @@ const repl = require('repl'); const replServer = repl.start({prompt: '> '}); replServer.defineCommand('sayhello', { help: 'Say hello', - action: function(name) { + action: function sayHello(name) { this.lineParser.reset(); this.bufferedCommand = ''; console.log(`Hello, ${name}!`); this.displayPrompt(); } }); -replServer.defineCommand('saybye', function() { +replServer.defineCommand('saybye', function sayBye() { console.log('Goodbye!'); this.close(); }); From 5f31da7990a0f1870d1d4b49f2429c9d1cf6930a Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:27:47 +0200 Subject: [PATCH 80/84] doc: document argument variant in repl.md `options` in the `repl.start([options])` can be a string. --- doc/api/repl.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/api/repl.md b/doc/api/repl.md index 62560a68ae1f50..97bb92b41b9e9b 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -417,6 +417,8 @@ added: v0.1.91 `SIGINT` is received, i.e. `Ctrl+C` is pressed. This cannot be used together with a custom `eval` function. Defaults to `false`. +If `options` is a string, then it specifies the input prompt. + The `repl.start()` method creates and starts a `repl.REPLServer` instance. If `options` is a string, then it specifies the input prompt: From d5861adb5a78bba4bd0871a5967819acba7ab389 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 01:31:05 +0200 Subject: [PATCH 81/84] doc: add the valid link for curl(1) in repl.md The current autoinserted link leads to 404 page. --- doc/api/repl.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index 97bb92b41b9e9b..b12f0552daa59f 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -552,10 +552,11 @@ possible to connect to a long-running Node.js process without restarting it. For an example of running a "full-featured" (`terminal`) REPL over a `net.Server` and `net.Socket` instance, see: https://gist.github.com/2209310 -For an example of running a REPL instance over curl(1), +For an example of running a REPL instance over [curl(1)][], see: https://gist.github.com/2053342 [stream]: stream.html [`util.inspect()`]: util.html#util_util_inspect_object_options [`readline.Interface`]: readline.html#readline_class_interface [`readline.InterfaceCompleter`]: readline.html#readline_use_of_the_completer_function +[curl(1)]: https://curl.haxx.se/docs/manpage.html From 0c32eb5c759108349374730763b56e302d16f21b Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 03:35:17 +0200 Subject: [PATCH 82/84] doc: replace anonymous functions in repl.md Replaced with an object shorthand and an arrow function. --- doc/api/repl.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index b12f0552daa59f..b57b00a05660e6 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -329,14 +329,14 @@ const repl = require('repl'); const replServer = repl.start({prompt: '> '}); replServer.defineCommand('sayhello', { help: 'Say hello', - action: function sayHello(name) { + action(name) { this.lineParser.reset(); this.bufferedCommand = ''; console.log(`Hello, ${name}!`); this.displayPrompt(); } }); -replServer.defineCommand('saybye', function sayBye() { +replServer.defineCommand('saybye', () => { console.log('Goodbye!'); this.close(); }); From 936263e1cd694180644970cd068e194c35ea1f98 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Wed, 7 Dec 2016 20:39:10 +0200 Subject: [PATCH 83/84] doc: reword variable declaration note in repl.md --- doc/api/repl.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index b57b00a05660e6..d138bf39f0fc4f 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -84,10 +84,9 @@ undefined 3 ``` -Unless otherwise scoped within blocks (e.g. `{ ... }`) or functions, variables -declared either implicitly or using the `const` or `let` keywords -are declared at the `global` scope (as well as with the `var` keyword -outside of functions). +Unless otherwise scoped within blocks or functions, variables declared +either implicitly, or using the const, let, or var keywords +are declared at the global scope. #### Global and Local Scope From 588a5d575b9d73b5e959dd2973587fd1fa30af72 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Fri, 9 Dec 2016 20:52:01 +0200 Subject: [PATCH 84/84] doc: revert documenting argument form in repl.md It will be postponed for more careful evaluating in a separate PR. --- doc/api/repl.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/doc/api/repl.md b/doc/api/repl.md index d138bf39f0fc4f..4bc987730370cf 100644 --- a/doc/api/repl.md +++ b/doc/api/repl.md @@ -416,8 +416,6 @@ added: v0.1.91 `SIGINT` is received, i.e. `Ctrl+C` is pressed. This cannot be used together with a custom `eval` function. Defaults to `false`. -If `options` is a string, then it specifies the input prompt. - The `repl.start()` method creates and starts a `repl.REPLServer` instance. If `options` is a string, then it specifies the input prompt: