From 18f41dda902da8524a2da697ebecb085ca45b8e8 Mon Sep 17 00:00:00 2001 From: Bryan Azofeifa Date: Thu, 19 Apr 2018 17:29:27 -0600 Subject: [PATCH 01/43] test: remove message from strictEqual assertions When an AssertionError happens, the value of headers['set-cookie'] is not reported. That information is useful for debugging. Hence removed the value passed as the message in deepStrictEqual assertions of test/parallel/test-http2-cookies.js PR-URL: https://github.com/nodejs/node/pull/20174 Reviewed-By: Rich Trott Reviewed-By: Luigi Pinca Reviewed-By: Richard Lau Reviewed-By: Trivikram Kamat --- test/parallel/test-http2-cookies.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/parallel/test-http2-cookies.js b/test/parallel/test-http2-cookies.js index 8b371d8c426..275fbf05e79 100644 --- a/test/parallel/test-http2-cookies.js +++ b/test/parallel/test-http2-cookies.js @@ -48,8 +48,7 @@ server.on('listening', common.mustCall(() => { req.on('response', common.mustCall((headers) => { assert(Array.isArray(headers['set-cookie'])); - assert.deepStrictEqual(headers['set-cookie'], setCookie, - 'set-cookie header does not match'); + assert.deepStrictEqual(headers['set-cookie'], setCookie); })); req.on('end', common.mustCall(() => { From b4e86f12f8a78356f0f1f94187bd6a3b2e2ea10b Mon Sep 17 00:00:00 2001 From: Divyanshu Singh Date: Mon, 2 Apr 2018 22:10:01 +0530 Subject: [PATCH 02/43] test: resolve process.setgid() error on Ubuntu When the tests are run as root in Ubuntu, process.setgid() is called with 'nobody' as an argument. This throws an error in Ubuntu and is because, in Ubuntu, the equivalent of 'nobody' group is named as 'nogroup'. This commit sets gid to 'nobody' first and if it throws a "group id does not exist" error, it attempts to set gid to 'nogroup'. If it still causes an error, the error is thrown. PR-URL: https://github.com/nodejs/node/pull/19755 Refs: https://github.com/nodejs/node/issues/19594 Reviewed-By: Rich Trott Reviewed-By: Gireesh Punathil Reviewed-By: Luigi Pinca --- test/parallel/test-process-uid-gid.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-process-uid-gid.js b/test/parallel/test-process-uid-gid.js index d3aac29decf..d044c45a327 100644 --- a/test/parallel/test-process-uid-gid.js +++ b/test/parallel/test-process-uid-gid.js @@ -61,7 +61,14 @@ if (process.getuid() !== 0) { // If we are running as super user... const oldgid = process.getgid(); -process.setgid('nobody'); +try { + process.setgid('nobody'); +} catch (err) { + if (err.message !== 'setgid group id does not exist') { + throw err; + } + process.setgid('nogroup'); +} const newgid = process.getgid(); assert.notStrictEqual(newgid, oldgid); From 3d7605561f62fb84f0108a8cf621cf130bc5e718 Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 20 Apr 2018 22:43:42 -0400 Subject: [PATCH 03/43] doc: remove "For example" expression in N-API doc PR-URL: https://github.com/nodejs/node/pull/20187 Reviewed-By: Ben Noordhuis Reviewed-By: Rich Trott Reviewed-By: Luigi Pinca Reviewed-By: Richard Lau Reviewed-By: Vse Mozhet Byt Reviewed-By: Trivikram Kamat --- doc/api/n-api.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/doc/api/n-api.md b/doc/api/n-api.md index 14ef277f535..826b83e2251 100644 --- a/doc/api/n-api.md +++ b/doc/api/n-api.md @@ -927,8 +927,8 @@ convenience. If `Init` returns NULL, the parameter passed as `exports` is exported by the module. N-API modules cannot modify the `module` object but can specify anything as the `exports` property of the module. -For example, to add the method `hello` as a function so that it can be called -as a method provided by the addon: +To add the method `hello` as a function so that it can be called as a method +provided by the addon: ```C napi_value Init(napi_env env, napi_value exports) { @@ -942,7 +942,7 @@ napi_value Init(napi_env env, napi_value exports) { } ``` -For example, to set a function to be returned by the `require()` for the addon: +To set a function to be returned by the `require()` for the addon: ```C napi_value Init(napi_env env, napi_value exports) { @@ -954,8 +954,8 @@ napi_value Init(napi_env env, napi_value exports) { } ``` -For example, to define a class so that new instances can be created -(often used with [Object Wrap][]): +To define a class so that new instances can be created (often used with +[Object Wrap][]): ```C // NOTE: partial example, not all referenced code is included From 0a3876d02566a4ce2cfbfc463f6651ce7903de4f Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Sat, 21 Apr 2018 17:34:35 +0300 Subject: [PATCH 04/43] tools: delete redundant .eslintignore rule The `node_modules` rule below is treated as a glob pattern, so it includes the `tools/node_modules` folder. PR-URL: https://github.com/nodejs/node/pull/20203 Refs: https://eslint.org/docs/user-guide/configuring#ignoring-files-and-directories Refs: https://git-scm.com/docs/gitignore#_pattern_format Reviewed-By: Rich Trott Reviewed-By: Colin Ihrig Reviewed-By: Gus Caplan --- .eslintignore | 1 - 1 file changed, 1 deletion(-) diff --git a/.eslintignore b/.eslintignore index 78ae310ce12..1ca60626119 100644 --- a/.eslintignore +++ b/.eslintignore @@ -5,7 +5,6 @@ test/addons/??_* test/es-module/test-esm-dynamic-import.js test/fixtures test/message/esm_display_syntax_error.mjs -tools/node_modules tools/icu tools/remark-* node_modules From 0799b60f502f1e2cdc28b91721ba96ca62c78a00 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Sat, 21 Apr 2018 10:58:12 +0530 Subject: [PATCH 05/43] doc,http2: add parameters for Http2Session:connect event Add parameters for the callback for the Http2Session:connect event inline with the pattern in the rest of the documentation. Refs: https://github.com/nodejs/help/issues/877#issuecomment-381253464 PR-URL: https://github.com/nodejs/node/pull/20193 Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Matteo Collina Reviewed-By: Vse Mozhet Byt --- doc/api/http2.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/api/http2.md b/doc/api/http2.md index 9a0b5595346..a893cda97ef 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -136,6 +136,9 @@ listener does not expect any arguments. added: v8.4.0 --> +* `session` {Http2Session} +* `socket` {net.Socket} + The `'connect'` event is emitted once the `Http2Session` has been successfully connected to the remote peer and communication may begin. From a4975cab41aaf5c036b97ad054177fcd5b7ac7f7 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Sat, 21 Apr 2018 17:07:28 +0300 Subject: [PATCH 06/43] doc: detail CI sub-tasks rerunning PR-URL: https://github.com/nodejs/node/pull/20200 Fixes: https://github.com/nodejs/node/issues/20192 Reviewed-By: Rich Trott Reviewed-By: Ruben Bridgewater --- COLLABORATOR_GUIDE.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/COLLABORATOR_GUIDE.md b/COLLABORATOR_GUIDE.md index aac77f07f4f..a7b1e68d727 100644 --- a/COLLABORATOR_GUIDE.md +++ b/COLLABORATOR_GUIDE.md @@ -199,11 +199,14 @@ status indicator. Do not land any Pull Requests without passing (green or yellow) CI runs. If you believe any failed (red or grey) CI sub-tasks are unrelated to the change in the -Pull Request, you may re-run the sub-task to try to see if it passes. If re-runs -of all failed sub-tasks pass, it is permissible to land the Pull Request but -only if the initial failures are believed in good faith to be unrelated to the -changes in the Pull Request. Otherwise, reasonable steps must be taken to -confirm that the changes are not resulting in an unreliable test. +Pull Request, you may re-run the sub-task to try to see if it passes (just open +the failed sub-task page and press the "Rebuild" button; be sure you are still +logged in for this action). If re-runs of all failed sub-tasks pass (do not +forget to provide the links for successfully rerun sub-tasks), it is permissible +to land the Pull Request but only if the initial failures are believed in good +faith to be unrelated to the changes in the Pull Request. Otherwise, reasonable +steps must be taken to confirm that the changes are not resulting in an +unreliable test. #### Useful CI Jobs From f4a559b240f3a006f328255510940c23eb5d0207 Mon Sep 17 00:00:00 2001 From: "Christine E. Taylor" Date: Fri, 20 Apr 2018 20:21:44 -0700 Subject: [PATCH 07/43] test: add strictEqual method to assert Adds strictEqual method to assert on stream.session.alpnProtocol to verify expected value 'h2'. This changes 'h2' from an assertion error message to the value expected from stream.session.alpnProtocol. PR-URL: https://github.com/nodejs/node/pull/20189 Reviewed-By: Rich Trott Reviewed-By: Richard Lau Reviewed-By: Trivikram Kamat Reviewed-By: Luigi Pinca Reviewed-By: Benjamin Gruenbaum Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: Ruben Bridgewater --- test/parallel/test-http2-create-client-secure-session.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-http2-create-client-secure-session.js b/test/parallel/test-http2-create-client-secure-session.js index 1f20ec8e42a..8b2aa1c168c 100644 --- a/test/parallel/test-http2-create-client-secure-session.js +++ b/test/parallel/test-http2-create-client-secure-session.js @@ -21,7 +21,7 @@ function onStream(stream, headers) { const socket = stream.session[kSocket]; assert(stream.session.encrypted); - assert(stream.session.alpnProtocol, 'h2'); + assert.strictEqual(stream.session.alpnProtocol, 'h2'); const originSet = stream.session.originSet; assert(Array.isArray(originSet)); assert.strictEqual(originSet[0], From c449eb5e8a6acdb7a00cb5c6d41a9f802603bb9c Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 14:57:28 +0200 Subject: [PATCH 08/43] http: simplify isCookieField Handrolling and checking char by char is no longer faster than just using toLowerCase and strict comparison. PR-URL: https://github.com/nodejs/node/pull/20131 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Trivikram Kamat Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Yuta Hiroto Reviewed-By: Ruben Bridgewater --- lib/_http_outgoing.js | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index b62dea4a24d..8ed49401960 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -58,24 +58,10 @@ var RE_CONN_VALUES = /(?:^|\W)close|upgrade(?:$|\W)/ig; var RE_TE_CHUNKED = common.chunkExpression; // isCookieField performs a case-insensitive comparison of a provided string -// against the word "cookie." This method (at least as of V8 5.4) is faster than -// the equivalent case-insensitive regexp, even if isCookieField does not get -// inlined. +// against the word "cookie." As of V8 6.6 this is faster than handrolling or +// using a case-insensitive RegExp. function isCookieField(s) { - if (s.length !== 6) return false; - var ch = s.charCodeAt(0); - if (ch !== 99 && ch !== 67) return false; - ch = s.charCodeAt(1); - if (ch !== 111 && ch !== 79) return false; - ch = s.charCodeAt(2); - if (ch !== 111 && ch !== 79) return false; - ch = s.charCodeAt(3); - if (ch !== 107 && ch !== 75) return false; - ch = s.charCodeAt(4); - if (ch !== 105 && ch !== 73) return false; - ch = s.charCodeAt(5); - if (ch !== 101 && ch !== 69) return false; - return true; + return s.length === 6 && s.toLowerCase() === 'cookie'; } function noopPendingOutput(amount) {} From 4fe1b60c5d09b9a0d397b0365cd981f4c9890f0d Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 15:45:45 +0200 Subject: [PATCH 09/43] http: use switch in matchHeader Using a switch improves clarity of the code but also performance for all but a few edge cases which remain on par with the old code. PR-URL: https://github.com/nodejs/node/pull/20131 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Trivikram Kamat Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Yuta Hiroto Reviewed-By: Ruben Bridgewater --- lib/_http_outgoing.js | 43 ++++++++++++++++++++----------------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 8ed49401960..8406c9cd8b0 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -52,8 +52,6 @@ const { utcDate } = internalHttp; const kIsCorked = Symbol('isCorked'); -var RE_FIELDS = - /^(?:Connection|Transfer-Encoding|Content-Length|Date|Expect|Trailer|Upgrade)$/i; var RE_CONN_VALUES = /(?:^|\W)close|upgrade(?:$|\W)/ig; var RE_TE_CHUNKED = common.chunkExpression; @@ -449,28 +447,27 @@ function matchConnValue(self, state, value) { } function matchHeader(self, state, field, value) { - var m = RE_FIELDS.exec(field); - if (!m) + if (field.length < 4 || field.length > 17) return; - var len = m[0].length; - if (len === 10) { - state.connection = true; - matchConnValue(self, state, value); - } else if (len === 17) { - state.te = true; - if (RE_TE_CHUNKED.test(value)) self.chunkedEncoding = true; - } else if (len === 14) { - state.contLen = true; - } else if (len === 4) { - state.date = true; - } else if (len === 6) { - state.expect = true; - } else if (len === 7) { - var ch = m[0].charCodeAt(0); - if (ch === 85 || ch === 117) - state.upgrade = true; - else - state.trailer = true; + field = field.toLowerCase(); + switch (field) { + case 'connection': + state.connection = true; + matchConnValue(self, state, value); + break; + case 'transfer-encoding': + state.te = true; + if (RE_TE_CHUNKED.test(value)) self.chunkedEncoding = true; + break; + case 'content-length': + state.contLen = true; + break; + case 'date': + case 'expect': + case 'trailer': + case 'upgrade': + state[field] = true; + break; } } From 28834542c8044a0e76176651d9886dc79907e3cd Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 16:39:27 +0200 Subject: [PATCH 10/43] http: simplify connection: close search Remove upgrade from the regular expression as it is no longer used and switch to using `.test()` instead of `.match()` as the value matched is irrelevant. PR-URL: https://github.com/nodejs/node/pull/20131 Reviewed-By: Richard Lau Reviewed-By: Luigi Pinca Reviewed-By: Trivikram Kamat Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Yuta Hiroto Reviewed-By: Ruben Bridgewater --- lib/_http_outgoing.js | 21 +++++---------------- 1 file changed, 5 insertions(+), 16 deletions(-) diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index 8406c9cd8b0..770e555d1ea 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -52,7 +52,7 @@ const { utcDate } = internalHttp; const kIsCorked = Symbol('isCorked'); -var RE_CONN_VALUES = /(?:^|\W)close|upgrade(?:$|\W)/ig; +var RE_CONN_CLOSE = /(?:^|\W)close(?:$|\W)/i; var RE_TE_CHUNKED = common.chunkExpression; // isCookieField performs a case-insensitive comparison of a provided string @@ -432,20 +432,6 @@ function storeHeader(self, state, key, value, validate) { matchHeader(self, state, key, value); } -function matchConnValue(self, state, value) { - var sawClose = false; - var m = RE_CONN_VALUES.exec(value); - while (m) { - if (m[0].length === 5) - sawClose = true; - m = RE_CONN_VALUES.exec(value); - } - if (sawClose) - self._last = true; - else - self.shouldKeepAlive = true; -} - function matchHeader(self, state, field, value) { if (field.length < 4 || field.length > 17) return; @@ -453,7 +439,10 @@ function matchHeader(self, state, field, value) { switch (field) { case 'connection': state.connection = true; - matchConnValue(self, state, value); + if (RE_CONN_CLOSE.test(value)) + self._last = true; + else + self.shouldKeepAlive = true; break; case 'transfer-encoding': state.te = true; From c61db33865254b7b615a5a0bd125288468d73447 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 12:15:00 +0200 Subject: [PATCH 11/43] test: fix long-running buffer benchmarks PR-URL: https://github.com/nodejs/node/pull/20125 Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott --- benchmark/buffers/buffer-base64-decode-wrapped.js | 6 +++--- benchmark/buffers/buffer-base64-decode.js | 5 +++-- benchmark/buffers/buffer-indexof.js | 8 ++++---- test/sequential/test-benchmark-buffer.js | 2 ++ 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/benchmark/buffers/buffer-base64-decode-wrapped.js b/benchmark/buffers/buffer-base64-decode-wrapped.js index 1e6f1fde0ae..7aee5a89c4f 100644 --- a/benchmark/buffers/buffer-base64-decode-wrapped.js +++ b/benchmark/buffers/buffer-base64-decode-wrapped.js @@ -3,12 +3,12 @@ const common = require('../common.js'); const bench = common.createBenchmark(main, { + charsPerLine: [76], + linesCount: [8 << 16], n: [32], }); -function main({ n }) { - const charsPerLine = 76; - const linesCount = 8 << 16; +function main({ charsPerLine, linesCount, n }) { const bytesCount = charsPerLine * linesCount / 4 * 3; const line = `${'abcd'.repeat(charsPerLine / 4)}\n`; diff --git a/benchmark/buffers/buffer-base64-decode.js b/benchmark/buffers/buffer-base64-decode.js index 1631ed4f089..0ac694fe8c4 100644 --- a/benchmark/buffers/buffer-base64-decode.js +++ b/benchmark/buffers/buffer-base64-decode.js @@ -4,10 +4,11 @@ const common = require('../common.js'); const bench = common.createBenchmark(main, { n: [32], + size: [8 << 20] }); -function main({ n }) { - const s = 'abcd'.repeat(8 << 20); +function main({ n, size }) { + const s = 'abcd'.repeat(size); // eslint-disable-next-line node-core/no-unescaped-regexp-dot s.match(/./); // Flatten string. assert.strictEqual(s.length % 4, 0); diff --git a/benchmark/buffers/buffer-indexof.js b/benchmark/buffers/buffer-indexof.js index c98d15320aa..26d82c506db 100644 --- a/benchmark/buffers/buffer-indexof.js +++ b/benchmark/buffers/buffer-indexof.js @@ -25,10 +25,10 @@ const bench = common.createBenchmark(main, { search: searchStrings, encoding: ['undefined', 'utf8', 'ucs2', 'binary'], type: ['buffer', 'string'], - iter: [100000] + n: [100000] }); -function main({ iter, search, encoding, type }) { +function main({ n, search, encoding, type }) { var aliceBuffer = fs.readFileSync( path.resolve(__dirname, '../fixtures/alice.html') ); @@ -46,8 +46,8 @@ function main({ iter, search, encoding, type }) { } bench.start(); - for (var i = 0; i < iter; i++) { + for (var i = 0; i < n; i++) { aliceBuffer.indexOf(search, 0, encoding); } - bench.end(iter); + bench.end(n); } diff --git a/test/sequential/test-benchmark-buffer.js b/test/sequential/test-benchmark-buffer.js index 5ac9ffb84a7..171f755647d 100644 --- a/test/sequential/test-benchmark-buffer.js +++ b/test/sequential/test-benchmark-buffer.js @@ -10,9 +10,11 @@ runBenchmark('buffers', 'args=1', 'buffer=fast', 'byteLength=1', + 'charsPerLine=6', 'encoding=utf8', 'endian=BE', 'len=2', + 'linesCount=1', 'method=', 'n=1', 'pieces=1', From 6278c4b268a4311fd4f47d1d8b56fdca8f966e8b Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 12:34:04 +0200 Subject: [PATCH 12/43] test: fix long-running http benchmarks PR-URL: https://github.com/nodejs/node/pull/20125 Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott --- benchmark/http/cluster.js | 4 ++-- test/sequential/test-benchmark-http.js | 4 +++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/benchmark/http/cluster.js b/benchmark/http/cluster.js index 56393fa1ab0..667e826f163 100644 --- a/benchmark/http/cluster.js +++ b/benchmark/http/cluster.js @@ -26,7 +26,7 @@ function main({ type, len, c }) { if (workers < 2) return; - setTimeout(function() { + setImmediate(function() { const path = `/${type}/${len}`; bench.http({ @@ -36,6 +36,6 @@ function main({ type, len, c }) { w1.destroy(); w2.destroy(); }); - }, 100); + }); }); } diff --git a/test/sequential/test-benchmark-http.js b/test/sequential/test-benchmark-http.js index e23a4a1753b..13d0e4d16aa 100644 --- a/test/sequential/test-benchmark-http.js +++ b/test/sequential/test-benchmark-http.js @@ -18,12 +18,14 @@ runBenchmark('http', 'chunkedEnc=true', 'chunks=0', 'dur=0.1', + 'input=keep-alive', 'key=""', 'len=1', 'method=write', 'n=1', 'res=normal', - 'type=asc' + 'type=asc', + 'value=X-Powered-By' ], { NODEJS_BENCHMARK_ZERO_ALLOWED: 1, From f572927147953d1bcf3e877194211f7d9f67a1aa Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 12:35:47 +0200 Subject: [PATCH 13/43] benchmark: do not multiply n by 1e6 in arrays PR-URL: https://github.com/nodejs/node/pull/20125 Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott --- benchmark/arrays/var-int.js | 4 ++-- benchmark/arrays/zero-float.js | 4 ++-- benchmark/arrays/zero-int.js | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/benchmark/arrays/var-int.js b/benchmark/arrays/var-int.js index e36a909a3b9..43073785631 100644 --- a/benchmark/arrays/var-int.js +++ b/benchmark/arrays/var-int.js @@ -14,14 +14,14 @@ const bench = common.createBenchmark(main, { 'Float32Array', 'Float64Array' ], - n: [25] + n: [25e6] }); function main({ type, n }) { const clazz = global[type]; bench.start(); - const arr = new clazz(n * 1e6); + const arr = new clazz(n); for (var i = 0; i < 10; ++i) { run(); } diff --git a/benchmark/arrays/zero-float.js b/benchmark/arrays/zero-float.js index 073460e0efb..de2e9bdba69 100644 --- a/benchmark/arrays/zero-float.js +++ b/benchmark/arrays/zero-float.js @@ -14,14 +14,14 @@ const bench = common.createBenchmark(main, { 'Float32Array', 'Float64Array' ], - n: [25] + n: [25e6] }); function main({ type, n }) { const clazz = global[type]; bench.start(); - const arr = new clazz(n * 1e6); + const arr = new clazz(n); for (var i = 0; i < 10; ++i) { run(); } diff --git a/benchmark/arrays/zero-int.js b/benchmark/arrays/zero-int.js index 78fd34ae6c0..548691d3739 100644 --- a/benchmark/arrays/zero-int.js +++ b/benchmark/arrays/zero-int.js @@ -14,14 +14,14 @@ const bench = common.createBenchmark(main, { 'Float32Array', 'Float64Array' ], - n: [25] + n: [25e6] }); function main({ type, n }) { const clazz = global[type]; bench.start(); - const arr = new clazz(n * 1e6); + const arr = new clazz(n); for (var i = 0; i < 10; ++i) { run(); } From 8c3e672543c85c81fe925755617b63dece6e0180 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 12:39:23 +0200 Subject: [PATCH 14/43] test: use correct arg name in assert benchmark PR-URL: https://github.com/nodejs/node/pull/20125 Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott --- test/parallel/test-benchmark-assert.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-benchmark-assert.js b/test/parallel/test-benchmark-assert.js index a83bbf8ad8c..3ca72df718c 100644 --- a/test/parallel/test-benchmark-assert.js +++ b/test/parallel/test-benchmark-assert.js @@ -8,5 +8,13 @@ require('../common'); const runBenchmark = require('../common/benchmark'); runBenchmark( - 'assert', ['len=1', 'method=', 'n=1', 'prim=null', 'size=1', 'type=Int8Array'] + 'assert', + [ + 'len=1', + 'method=', + 'n=1', + 'primitive=null', + 'size=1', + 'type=Int8Array' + ] ); From 77dc257f67a3bee07c31fb12cf0018563dedd1b6 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 12:41:10 +0200 Subject: [PATCH 15/43] test: use correct arg name in domains benchmark PR-URL: https://github.com/nodejs/node/pull/20125 Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott --- test/parallel/test-benchmark-domain.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-benchmark-domain.js b/test/parallel/test-benchmark-domain.js index b1b56d2b7f5..e7d8b60b716 100644 --- a/test/parallel/test-benchmark-domain.js +++ b/test/parallel/test-benchmark-domain.js @@ -4,4 +4,4 @@ require('../common'); const runBenchmark = require('../common/benchmark'); -runBenchmark('domain', ['n=1', 'arguments=0']); +runBenchmark('domain', ['n=1', 'args=0']); From 26a0a4659d5618fd5c1217132409ff7b2dd7300b Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 12:44:19 +0200 Subject: [PATCH 16/43] test: fix missing param in url benchmark PR-URL: https://github.com/nodejs/node/pull/20125 Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott --- test/parallel/test-benchmark-url.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-benchmark-url.js b/test/parallel/test-benchmark-url.js index e4bcf501700..92bb34de278 100644 --- a/test/parallel/test-benchmark-url.js +++ b/test/parallel/test-benchmark-url.js @@ -18,5 +18,6 @@ runBenchmark('url', 'to=ascii', 'prop=href', 'n=1', + 'param=one' ], { NODEJS_BENCHMARK_ZERO_ALLOWED: 1 }); From 6c90aee0b5bd757ca46e0be8a471ef5fce2e8e43 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 12:49:29 +0200 Subject: [PATCH 17/43] test: reduce duration in misc benchmark PR-URL: https://github.com/nodejs/node/pull/20125 Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott --- test/parallel/test-benchmark-misc.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-benchmark-misc.js b/test/parallel/test-benchmark-misc.js index b7e21602d5f..96b3fe1fa2e 100644 --- a/test/parallel/test-benchmark-misc.js +++ b/test/parallel/test-benchmark-misc.js @@ -6,6 +6,7 @@ const runBenchmark = require('../common/benchmark'); runBenchmark('misc', [ 'concat=0', + 'dur=0.1', 'method=', 'n=1', 'type=extend', From bc6965bdd24937d5a77acdc37c8efa280a08d5d0 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 12:50:29 +0200 Subject: [PATCH 18/43] test: fix arg names in benchmark string_decoder PR-URL: https://github.com/nodejs/node/pull/20125 Reviewed-By: Ruben Bridgewater Reviewed-By: Rich Trott --- test/parallel/test-benchmark-string_decoder.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/parallel/test-benchmark-string_decoder.js b/test/parallel/test-benchmark-string_decoder.js index a78390e03db..f2fd6abe4b8 100644 --- a/test/parallel/test-benchmark-string_decoder.js +++ b/test/parallel/test-benchmark-string_decoder.js @@ -4,7 +4,7 @@ require('../common'); const runBenchmark = require('../common/benchmark'); -runBenchmark('string_decoder', ['chunk=16', +runBenchmark('string_decoder', ['chunkLen=16', 'encoding=utf8', - 'inlen=32', + 'inLen=32', 'n=1']); From 881fca418c4363f540f7a25f3860fd9905a48fc2 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 19 Apr 2018 11:13:17 +0200 Subject: [PATCH 19/43] lib: remove unused binding const MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit removes the binding const as it is only used in one place which is in the following line. PR-URL: https://github.com/nodejs/node/pull/20144 Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Richard Lau Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: Yuta Hiroto Reviewed-By: Trivikram Kamat Reviewed-By: Tobias Nießen --- lib/_tls_common.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/_tls_common.js b/lib/_tls_common.js index fb6ac34d1e6..a9fe0d8f06a 100644 --- a/lib/_tls_common.js +++ b/lib/_tls_common.js @@ -34,8 +34,7 @@ const { SSL_OP_CIPHER_SERVER_PREFERENCE } = process.binding('constants').crypto; // Lazily loaded var crypto = null; -const binding = process.binding('crypto'); -const NativeSecureContext = binding.SecureContext; +const { SecureContext: NativeSecureContext } = process.binding('crypto'); function SecureContext(secureProtocol, secureOptions, context) { if (!(this instanceof SecureContext)) { From 80c46c1576710322f13ce061b07a5ca0cb22dbc6 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 17 Apr 2018 17:47:09 +0200 Subject: [PATCH 20/43] src: remove `MarkIndependent()` calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The method has been deprecated in upstream V8, with messaging indicating that it is the default for handles to be independent now anyway. PR-URL: https://github.com/nodejs/node/pull/20108 Refs: https://github.com/v8/v8/commit/71ad48fb8f214e80518ba0419796e4c571351255 Reviewed-By: Anatoli Papirovski Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell Reviewed-By: Tobias Nießen Reviewed-By: Colin Ihrig Reviewed-By: Matteo Collina --- src/base_object-inl.h | 1 - src/node_api.cc | 2 -- src/node_buffer.cc | 1 - src/node_contextify.cc | 1 - src/node_object_wrap.h | 1 - 5 files changed, 6 deletions(-) diff --git a/src/base_object-inl.h b/src/base_object-inl.h index 51ef4659966..5ff211f473b 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -70,7 +70,6 @@ inline void BaseObject::MakeWeak(Type* ptr) { v8::Local handle = object(); CHECK_GT(handle->InternalFieldCount(), 0); Wrap(handle, ptr); - persistent_handle_.MarkIndependent(); persistent_handle_.SetWeak(ptr, WeakCallback, v8::WeakCallbackType::kParameter); } diff --git a/src/node_api.cc b/src/node_api.cc index 3a02e5effa7..ea7ddba77dc 100644 --- a/src/node_api.cc +++ b/src/node_api.cc @@ -387,7 +387,6 @@ class Reference : private Finalizer { if (initial_refcount == 0) { _persistent.SetWeak( this, FinalizeCallback, v8::WeakCallbackType::kParameter); - _persistent.MarkIndependent(); } } @@ -431,7 +430,6 @@ class Reference : private Finalizer { if (--_refcount == 0) { _persistent.SetWeak( this, FinalizeCallback, v8::WeakCallbackType::kParameter); - _persistent.MarkIndependent(); } return _refcount; diff --git a/src/node_buffer.cc b/src/node_buffer.cc index b00886f5e68..38b473b8ced 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -144,7 +144,6 @@ CallbackInfo::CallbackInfo(Isolate* isolate, persistent_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter); persistent_.SetWrapperClassId(BUFFER_ID); - persistent_.MarkIndependent(); isolate->AdjustAmountOfExternalAllocatedMemory(sizeof(*this)); } diff --git a/src/node_contextify.cc b/src/node_contextify.cc index ce235765018..68251004d57 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -104,7 +104,6 @@ ContextifyContext::ContextifyContext( if (context_.IsEmpty()) return; context_.SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter); - context_.MarkIndependent(); } diff --git a/src/node_object_wrap.h b/src/node_object_wrap.h index 37c759fb894..b8ed2f99135 100644 --- a/src/node_object_wrap.h +++ b/src/node_object_wrap.h @@ -83,7 +83,6 @@ class ObjectWrap { inline void MakeWeak(void) { persistent().SetWeak(this, WeakCallback, v8::WeakCallbackType::kParameter); - persistent().MarkIndependent(); } /* Ref() marks the object as being attached to an event loop. From 8ff73aa82de0b09dbe9690bdd6f3a81ab5e25a50 Mon Sep 17 00:00:00 2001 From: musgravejw Date: Fri, 20 Apr 2018 19:32:30 -0400 Subject: [PATCH 21/43] doc: modify net.Server.listen arg list PR-URL: https://github.com/nodejs/node/pull/20142 Reviewed-By: Luigi Pinca Reviewed-By: Anatoli Papirovski Reviewed-By: Trivikram Kamat --- doc/api/net.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/net.md b/doc/api/net.md index 0575bb5f8f2..a60278ea02f 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -192,7 +192,7 @@ Possible signatures: * [`server.listen(options[, callback])`][`server.listen(options)`] * [`server.listen(path[, backlog][, callback])`][`server.listen(path)`] for [IPC][] servers -* [`server.listen([port][, host][, backlog][, callback])`][`server.listen(port, host)`] +* [`server.listen([[[port[, hostname[, backlog]]][, callback])`][`server.listen(port, host)`] for TCP servers This function is asynchronous. When the server starts listening, the @@ -264,7 +264,7 @@ added: v0.11.14 * Returns: {net.Server} If `port` is specified, it behaves the same as -[`server.listen([port][, hostname][, backlog][, callback])`][`server.listen(port, host)`]. +[`server.listen([[[port[, hostname[, backlog]]][, callback])`][`server.listen(port, host)`]. Otherwise, if `path` is specified, it behaves the same as [`server.listen(path[, backlog][, callback])`][`server.listen(path)`]. If none of them is specified, an error will be thrown. From e8ea61be4121c693a04ff1f9594b35e30ddd4a12 Mon Sep 17 00:00:00 2001 From: Daniel Bevenius Date: Thu, 19 Apr 2018 10:45:25 +0200 Subject: [PATCH 22/43] lib: remove unnecessary assignment of exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit removes the assignment of exports since it is not used in these files and there is no harm re-assigning module.exports. PR-URL: https://github.com/nodejs/node/pull/20143 Reviewed-By: Luigi Pinca Reviewed-By: Jackson Tian Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat Reviewed-By: Tobias Nießen --- lib/internal/errors.js | 2 +- lib/os.js | 2 +- lib/v8.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/internal/errors.js b/lib/internal/errors.js index 88c7f86eb91..09f58506c44 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -687,7 +687,7 @@ function isStackOverflowError(err) { err.message === maxStack_ErrorMessage; } -module.exports = exports = { +module.exports = { dnsException, errnoException, exceptionWithHostPort, diff --git a/lib/os.js b/lib/os.js index 5c83dbfab7b..08daa182e8b 100644 --- a/lib/os.js +++ b/lib/os.js @@ -159,7 +159,7 @@ function networkInterfaces() { }, {}); } -module.exports = exports = { +module.exports = { arch, cpus, endianness, diff --git a/lib/v8.js b/lib/v8.js index cce77c5062a..ed93b094ca7 100644 --- a/lib/v8.js +++ b/lib/v8.js @@ -213,7 +213,7 @@ function deserialize(buffer) { return der.readValue(); } -module.exports = exports = { +module.exports = { cachedDataVersionTag, getHeapStatistics, getHeapSpaceStatistics, From 51164dd6adeb69a1d1c7c47c944ce5e6572800b3 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Thu, 19 Apr 2018 17:18:07 +0800 Subject: [PATCH 23/43] src: CancelTerminateExecution before throwing errors Terminating the execution of a script would create a pending exception, therefore we should cancel the termination *before* throwing a new exception, otherwise the attempt to create a custom error may fail. PR-URL: https://github.com/nodejs/node/pull/20146 Reviewed-By: Ben Noordhuis Reviewed-By: Colin Ihrig Reviewed-By: Anna Henningsen Reviewed-By: James M Snell --- src/module_wrap.cc | 3 ++- src/node_contextify.cc | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/module_wrap.cc b/src/module_wrap.cc index 48af2daa139..9bcdb4dce75 100644 --- a/src/module_wrap.cc +++ b/src/module_wrap.cc @@ -279,7 +279,9 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo& args) { result = module->Evaluate(context); } + // Convert the termination exception into a regular exception. if (timed_out || received_signal) { + env->isolate()->CancelTerminateExecution(); // It is possible that execution was terminated by another timeout in // which this timeout is nested, so check whether one of the watchdogs // from this invocation is responsible for termination. @@ -288,7 +290,6 @@ void ModuleWrap::Evaluate(const FunctionCallbackInfo& args) { } else if (received_signal) { env->ThrowError("Script execution interrupted."); } - env->isolate()->CancelTerminateExecution(); } if (try_catch.HasCaught()) { diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 68251004d57..e07d5ebcd29 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -851,7 +851,9 @@ class ContextifyScript : public BaseObject { result = script->Run(env->context()); } + // Convert the termination exception into a regular exception. if (timed_out || received_signal) { + env->isolate()->CancelTerminateExecution(); // It is possible that execution was terminated by another timeout in // which this timeout is nested, so check whether one of the watchdogs // from this invocation is responsible for termination. @@ -860,7 +862,6 @@ class ContextifyScript : public BaseObject { } else if (received_signal) { env->ThrowError("Script execution interrupted."); } - env->isolate()->CancelTerminateExecution(); } if (try_catch.HasCaught()) { From d591a59ac1d654070cd1f0b8b7cb1ff479426f67 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Sun, 22 Apr 2018 03:35:12 +0300 Subject: [PATCH 24/43] meta: document commit msg exception for long URLs PR-URL: https://github.com/nodejs/node/pull/20207 Fixes: https://github.com/nodejs/node/issues/17116 Refs: https://github.com/nodejs/core-validate-commit/issues/24 Reviewed-By: Ruben Bridgewater Reviewed-By: Luigi Pinca Reviewed-By: Trivikram Kamat --- doc/guides/contributing/pull-requests.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/guides/contributing/pull-requests.md b/doc/guides/contributing/pull-requests.md index b6cc9eeeefc..f481a2cacb9 100644 --- a/doc/guides/contributing/pull-requests.md +++ b/doc/guides/contributing/pull-requests.md @@ -151,7 +151,7 @@ A good commit message should describe what changed and why. 2. Keep the second line blank. -3. Wrap all other lines at 72 columns. +3. Wrap all other lines at 72 columns (except for long URLs). 4. If your patch fixes an open issue, you can add a reference to it at the end of the log. Use the `Fixes:` prefix and the full issue URL. For other references From 4125a9f8dedaf63f9f3be89426ffbed2f3fba64f Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Sun, 22 Apr 2018 11:32:52 +0200 Subject: [PATCH 25/43] doc: fix incorrect net listen signature PR-URL: https://github.com/nodejs/node/pull/20209 Reviewed-By: Trivikram Kamat Reviewed-By: Luigi Pinca Reviewed-By: Vse Mozhet Byt Reviewed-By: Ruben Bridgewater Reviewed-By: Colin Ihrig --- doc/api/net.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/api/net.md b/doc/api/net.md index a60278ea02f..54f3c2f737a 100644 --- a/doc/api/net.md +++ b/doc/api/net.md @@ -192,7 +192,7 @@ Possible signatures: * [`server.listen(options[, callback])`][`server.listen(options)`] * [`server.listen(path[, backlog][, callback])`][`server.listen(path)`] for [IPC][] servers -* [`server.listen([[[port[, hostname[, backlog]]][, callback])`][`server.listen(port, host)`] +* [`server.listen([port[, host[, backlog]]][, callback])`][`server.listen(port, host)`] for TCP servers This function is asynchronous. When the server starts listening, the @@ -264,7 +264,7 @@ added: v0.11.14 * Returns: {net.Server} If `port` is specified, it behaves the same as -[`server.listen([[[port[, hostname[, backlog]]][, callback])`][`server.listen(port, host)`]. +[`server.listen([port[, host[, backlog]]][, callback])`][`server.listen(port, host)`]. Otherwise, if `path` is specified, it behaves the same as [`server.listen(path[, backlog][, callback])`][`server.listen(path)`]. If none of them is specified, an error will be thrown. @@ -296,7 +296,7 @@ added: v0.1.90 Start a [IPC][] server listening for connections on the given `path`. -#### server.listen([port][, host][, backlog][, callback]) +#### server.listen([port[, host[, backlog]]][, callback]) From 1bdd3b0dcfc85d7063859820bca0abc9eaa94100 Mon Sep 17 00:00:00 2001 From: Vse Mozhet Byt Date: Sat, 14 Apr 2018 12:45:06 +0300 Subject: [PATCH 26/43] tools: improve heading type detection in json.js PR-URL: https://github.com/nodejs/node/pull/20074 Reviewed-By: Luigi Pinca --- tools/doc/json.js | 55 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 47 insertions(+), 8 deletions(-) diff --git a/tools/doc/json.js b/tools/doc/json.js index b1a04c69380..44b67e3e284 100644 --- a/tools/doc/json.js +++ b/tools/doc/json.js @@ -553,15 +553,54 @@ function cloneValue(src) { } -// These parse out the contents of an H# tag. +// This section parse out the contents of an H# tag. + +// To reduse escape slashes in RegExp string components. +const r = String.raw; + +const eventPrefix = '^Event: +'; +const classPrefix = '^[Cc]lass: +'; +const ctorPrefix = '^(?:[Cc]onstructor: +)?new +'; +const classMethodPrefix = '^Class Method: +'; +const maybeClassPropertyPrefix = '(?:Class Property: +)?'; + +const maybeQuote = '[\'"]?'; +const notQuotes = '[^\'"]+'; + +// To include constructs like `readable\[Symbol.asyncIterator\]()` +// or `readable.\_read(size)` (with Markdown escapes). +const simpleId = r`(?:(?:\\?_)+|\b)\w+\b`; +const computedId = r`\\?\[[\w\.]+\\?\]`; +const id = `(?:${simpleId}|${computedId})`; +const classId = r`[A-Z]\w+`; + +const ancestors = r`(?:${id}\.?)+`; +const maybeAncestors = r`(?:${id}\.?)*`; + +const callWithParams = r`\([^)]*\)`; + +const noCallOrProp = '(?![.[(])'; + +const maybeExtends = `(?: +extends +${maybeAncestors}${classId})?`; + const headingExpressions = [ - { type: 'event', re: /^Event(?::|\s)+['"]?([^"']+).*$/i }, - { type: 'class', re: /^Class:\s*([^ ]+).*$/i }, - { type: 'property', re: /^[^.[]+(\[[^\]]+\])\s*$/ }, - { type: 'property', re: /^[^.]+\.([^ .()]+)\s*$/ }, - { type: 'classMethod', re: /^class\s*method\s*:?[^.]+\.([^ .()]+)\([^)]*\)\s*$/i }, - { type: 'method', re: /^(?:[^.]+\.)?([^ .()]+)\([^)]*\)\s*$/ }, - { type: 'ctor', re: /^new ([A-Z][a-zA-Z]+)\([^)]*\)\s*$/ }, + { type: 'event', re: RegExp( + `${eventPrefix}${maybeQuote}(${notQuotes})${maybeQuote}$`, 'i') }, + + { type: 'class', re: RegExp( + `${classPrefix}(${maybeAncestors}${classId})${maybeExtends}$`, '') }, + + { type: 'ctor', re: RegExp( + `${ctorPrefix}(${maybeAncestors}${classId})${callWithParams}$`, '') }, + + { type: 'classMethod', re: RegExp( + `${classMethodPrefix}${maybeAncestors}(${id})${callWithParams}$`, 'i') }, + + { type: 'method', re: RegExp( + `^${maybeAncestors}(${id})${callWithParams}$`, 'i') }, + + { type: 'property', re: RegExp( + `^${maybeClassPropertyPrefix}${ancestors}(${id})${noCallOrProp}$`, 'i') }, ]; function newSection({ text }) { From ffd57cd7b2771de70a0850b8893336380f37212b Mon Sep 17 00:00:00 2001 From: cjihrig Date: Sun, 22 Apr 2018 14:07:50 -0400 Subject: [PATCH 27/43] deps: upgrade to libuv 1.20.2 PR-URL: https://github.com/nodejs/node/pull/20129 Fixes: https://github.com/nodejs/node/issues/20112 Fixes: https://github.com/nodejs/node/issues/19903 Reviewed-By: Myles Borins Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Richard Lau --- deps/uv/AUTHORS | 5 + deps/uv/ChangeLog | 36 +++++ deps/uv/common.gypi | 6 +- deps/uv/configure.ac | 2 +- deps/uv/docs/src/fs.rst | 6 +- deps/uv/include/uv-os390.h | 2 +- deps/uv/include/uv-version.h | 2 +- deps/uv/src/unix/pipe.c | 35 +++-- deps/uv/src/unix/tcp.c | 2 + deps/uv/src/unix/thread.c | 201 +++++++++++++++++------- deps/uv/src/win/fs.c | 2 - deps/uv/src/win/util.c | 47 +++--- deps/uv/test/test-fs.c | 22 --- deps/uv/test/test-pipe-set-fchmod.c | 24 +++ deps/uv/test/test-udp-ipv6.c | 2 + deps/uv/test/test-udp-multicast-join6.c | 3 +- 16 files changed, 268 insertions(+), 129 deletions(-) diff --git a/deps/uv/AUTHORS b/deps/uv/AUTHORS index c704527a970..ba360e3f74e 100644 --- a/deps/uv/AUTHORS +++ b/deps/uv/AUTHORS @@ -332,3 +332,8 @@ Ryuichi KAWAMATA Joyee Cheung Michael Kilburn Ruslan Bekenev +Bob Burger +Thomas Versteeg +zzzjim +Alex Arslan +Kyle Farnung diff --git a/deps/uv/ChangeLog b/deps/uv/ChangeLog index 3ae9d2023e9..a7cd0ad2dc5 100644 --- a/deps/uv/ChangeLog +++ b/deps/uv/ChangeLog @@ -1,3 +1,39 @@ +2018.04.23, Version 1.20.2 (Stable), c51fd3f66bbb386a1efdeba6812789f35a372d1e + +Changes since version 1.20.1: + +* zos: use custom semaphore (jBarz) + +* win: fix registry API error handling (Kyle Farnung) + +* build: add support for 64-bit AIX (Richard Lau) + +* aix: guard STATIC_ASSERT for glibc work around (Richard Lau) + + +2018.04.19, Version 1.20.1 (Stable), 36ac2fc8edfd5ff3e9be529be1d4a3f0d5364e94 + +Changes since version 1.20.0: + +* doc,fs: improve documentation (Bob Burger) + +* win: return a floored double from uv_uptime() (Refael Ackermann) + +* doc: clarify platform specific pipe naming (Thomas Versteeg) + +* unix: fix uv_pipe_chmod() on macOS (zzzjim) + +* unix: work around glibc semaphore race condition (Anna Henningsen) + +* tcp,openbsd: disable Unix TCP check for IPV6_ONLY (Alex Arslan) + +* test,openbsd: use RETURN_SKIP in UDP IPv6 tests (Alex Arslan) + +* test,openbsd: fix multicast test (Alex Arslan) + +* Revert "win, fs: use FILE_WRITE_ATTRIBUTES when opening files" (cjihrig) + + 2018.04.03, Version 1.20.0 (Stable), 0012178ee2b04d9e4a2c66c27cf8891ad8325ceb Changes since version 1.19.2: diff --git a/deps/uv/common.gypi b/deps/uv/common.gypi index 572a1633b0b..2297bdf0fb0 100644 --- a/deps/uv/common.gypi +++ b/deps/uv/common.gypi @@ -134,7 +134,7 @@ }] ] }], - ['OS in "freebsd dragonflybsd linux openbsd solaris android"', { + ['OS in "freebsd dragonflybsd linux openbsd solaris android aix"', { 'cflags': [ '-Wall' ], 'cflags_cc': [ '-fno-rtti', '-fno-exceptions' ], 'target_conditions': [ @@ -162,6 +162,10 @@ 'cflags': [ '-pthread' ], 'ldflags': [ '-pthread' ], }], + [ 'OS=="aix" and target_arch=="ppc64"', { + 'cflags': [ '-maix64' ], + 'ldflags': [ '-maix64' ], + }], ], }], ['OS=="mac"', { diff --git a/deps/uv/configure.ac b/deps/uv/configure.ac index ee8571ea921..f4d97daaba0 100644 --- a/deps/uv/configure.ac +++ b/deps/uv/configure.ac @@ -13,7 +13,7 @@ # OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. AC_PREREQ(2.57) -AC_INIT([libuv], [1.20.0], [https://github.com/libuv/libuv/issues]) +AC_INIT([libuv], [1.20.2], [https://github.com/libuv/libuv/issues]) AC_CONFIG_MACRO_DIR([m4]) m4_include([m4/libuv-extra-automake-flags.m4]) m4_include([m4/as_case.m4]) diff --git a/deps/uv/docs/src/fs.rst b/deps/uv/docs/src/fs.rst index 29227be0d64..8e8fc2f1d5d 100644 --- a/deps/uv/docs/src/fs.rst +++ b/deps/uv/docs/src/fs.rst @@ -148,8 +148,8 @@ Public members .. c:member:: void* uv_fs_t.ptr - Stores the result of :c:func:`uv_fs_readlink` and serves as an alias to - `statbuf`. + Stores the result of :c:func:`uv_fs_readlink` and + :c:func:`uv_fs_realpath` and serves as an alias to `statbuf`. .. seealso:: The :c:type:`uv_req_t` members also apply. @@ -311,10 +311,12 @@ API .. c:function:: int uv_fs_readlink(uv_loop_t* loop, uv_fs_t* req, const char* path, uv_fs_cb cb) Equivalent to :man:`readlink(2)`. + The resulting string is stored in `req->ptr`. .. c:function:: int uv_fs_realpath(uv_loop_t* loop, uv_fs_t* req, const char* path, uv_fs_cb cb) Equivalent to :man:`realpath(3)` on Unix. Windows uses `GetFinalPathNameByHandle `_. + The resulting string is stored in `req->ptr`. .. warning:: This function has certain platform-specific caveats that were discovered when used in Node. diff --git a/deps/uv/include/uv-os390.h b/deps/uv/include/uv-os390.h index 39e7384db31..0267d74cbd0 100644 --- a/deps/uv/include/uv-os390.h +++ b/deps/uv/include/uv-os390.h @@ -22,7 +22,7 @@ #ifndef UV_MVS_H #define UV_MVS_H -#define UV_PLATFORM_SEM_T int +#define UV_PLATFORM_SEM_T long #define UV_PLATFORM_LOOP_FIELDS \ void* ep; \ diff --git a/deps/uv/include/uv-version.h b/deps/uv/include/uv-version.h index 392b4d64667..18f40da890b 100644 --- a/deps/uv/include/uv-version.h +++ b/deps/uv/include/uv-version.h @@ -32,7 +32,7 @@ #define UV_VERSION_MAJOR 1 #define UV_VERSION_MINOR 20 -#define UV_VERSION_PATCH 0 +#define UV_VERSION_PATCH 2 #define UV_VERSION_IS_RELEASE 1 #define UV_VERSION_SUFFIX "" diff --git a/deps/uv/src/unix/pipe.c b/deps/uv/src/unix/pipe.c index e640bf29fc1..2c578dcb359 100644 --- a/deps/uv/src/unix/pipe.c +++ b/deps/uv/src/unix/pipe.c @@ -319,21 +319,6 @@ int uv_pipe_chmod(uv_pipe_t* handle, int mode) { mode != (UV_WRITABLE | UV_READABLE)) return UV_EINVAL; - if (fstat(uv__stream_fd(handle), &pipe_stat) == -1) - return UV__ERR(errno); - - desired_mode = 0; - if (mode & UV_READABLE) - desired_mode |= S_IRUSR | S_IRGRP | S_IROTH; - if (mode & UV_WRITABLE) - desired_mode |= S_IWUSR | S_IWGRP | S_IWOTH; - - /* Exit early if pipe already has desired mode. */ - if ((pipe_stat.st_mode & desired_mode) == desired_mode) - return 0; - - pipe_stat.st_mode |= desired_mode; - /* Unfortunately fchmod does not work on all platforms, we will use chmod. */ name_len = 0; r = uv_pipe_getsockname(handle, NULL, &name_len); @@ -350,6 +335,26 @@ int uv_pipe_chmod(uv_pipe_t* handle, int mode) { return r; } + /* stat must be used as fstat has a bug on Darwin */ + if (stat(name_buffer, &pipe_stat) == -1) { + uv__free(name_buffer); + return -errno; + } + + desired_mode = 0; + if (mode & UV_READABLE) + desired_mode |= S_IRUSR | S_IRGRP | S_IROTH; + if (mode & UV_WRITABLE) + desired_mode |= S_IWUSR | S_IWGRP | S_IWOTH; + + /* Exit early if pipe already has desired mode. */ + if ((pipe_stat.st_mode & desired_mode) == desired_mode) { + uv__free(name_buffer); + return 0; + } + + pipe_stat.st_mode |= desired_mode; + r = chmod(name_buffer, pipe_stat.st_mode); uv__free(name_buffer); diff --git a/deps/uv/src/unix/tcp.c b/deps/uv/src/unix/tcp.c index 336d8e29205..9a46793fdbb 100644 --- a/deps/uv/src/unix/tcp.c +++ b/deps/uv/src/unix/tcp.c @@ -164,6 +164,7 @@ int uv__tcp_bind(uv_tcp_t* tcp, if (setsockopt(tcp->io_watcher.fd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on))) return UV__ERR(errno); +#ifndef __OpenBSD__ #ifdef IPV6_V6ONLY if (addr->sa_family == AF_INET6) { on = (flags & UV_TCP_IPV6ONLY) != 0; @@ -179,6 +180,7 @@ int uv__tcp_bind(uv_tcp_t* tcp, return UV__ERR(errno); } } +#endif #endif errno = 0; diff --git a/deps/uv/src/unix/thread.c b/deps/uv/src/unix/thread.c index 3def29457aa..303bc6ec84f 100644 --- a/deps/uv/src/unix/thread.c +++ b/deps/uv/src/unix/thread.c @@ -37,6 +37,10 @@ #include #endif +#ifdef __GLIBC__ +#include /* gnu_get_libc_version() */ +#endif + #undef NANOSEC #define NANOSEC ((uint64_t) 1e9) @@ -419,109 +423,145 @@ int uv_sem_trywait(uv_sem_t* sem) { return UV_EINVAL; /* Satisfy the compiler. */ } +#else /* !(defined(__APPLE__) && defined(__MACH__)) */ + +#ifdef __GLIBC__ + +/* Hack around https://sourceware.org/bugzilla/show_bug.cgi?id=12674 + * by providing a custom implementation for glibc < 2.21 in terms of other + * concurrency primitives. + * Refs: https://github.com/nodejs/node/issues/19903 */ + +/* To preserve ABI compatibility, we treat the uv_sem_t as storage for + * a pointer to the actual struct we're using underneath. */ + +static uv_once_t glibc_version_check_once = UV_ONCE_INIT; +static int platform_needs_custom_semaphore = 0; + +static void glibc_version_check(void) { + const char* version = gnu_get_libc_version(); + platform_needs_custom_semaphore = + version[0] == '2' && version[1] == '.' && + atoi(version + 2) < 21; +} + #elif defined(__MVS__) -int uv_sem_init(uv_sem_t* sem, unsigned int value) { - uv_sem_t semid; +#define platform_needs_custom_semaphore 1 + +#else /* !defined(__GLIBC__) && !defined(__MVS__) */ + +#define platform_needs_custom_semaphore 0 + +#endif + +typedef struct uv_semaphore_s { + uv_mutex_t mutex; + uv_cond_t cond; + unsigned int value; +} uv_semaphore_t; + +#if defined(__GLIBC__) || platform_needs_custom_semaphore +STATIC_ASSERT(sizeof(uv_sem_t) >= sizeof(uv_semaphore_t*)); +#endif + +static int uv__custom_sem_init(uv_sem_t* sem_, unsigned int value) { int err; - union { - int val; - struct semid_ds* buf; - unsigned short* array; - } arg; + uv_semaphore_t* sem; + sem = uv__malloc(sizeof(*sem)); + if (sem == NULL) + return UV_ENOMEM; - semid = semget(IPC_PRIVATE, 1, S_IRUSR | S_IWUSR); - if (semid == -1) - return UV__ERR(errno); + if ((err = uv_mutex_init(&sem->mutex)) != 0) { + uv__free(sem); + return err; + } - arg.val = value; - if (-1 == semctl(semid, 0, SETVAL, arg)) { - err = errno; - if (-1 == semctl(*sem, 0, IPC_RMID)) - abort(); - return UV__ERR(err); + if ((err = uv_cond_init(&sem->cond)) != 0) { + uv_mutex_destroy(&sem->mutex); + uv__free(sem); + return err; } - *sem = semid; + sem->value = value; + *(uv_semaphore_t**)sem_ = sem; return 0; } -void uv_sem_destroy(uv_sem_t* sem) { - if (-1 == semctl(*sem, 0, IPC_RMID)) - abort(); + +static void uv__custom_sem_destroy(uv_sem_t* sem_) { + uv_semaphore_t* sem; + + sem = *(uv_semaphore_t**)sem_; + uv_cond_destroy(&sem->cond); + uv_mutex_destroy(&sem->mutex); + uv__free(sem); } -void uv_sem_post(uv_sem_t* sem) { - struct sembuf buf; - buf.sem_num = 0; - buf.sem_op = 1; - buf.sem_flg = 0; +static void uv__custom_sem_post(uv_sem_t* sem_) { + uv_semaphore_t* sem; - if (-1 == semop(*sem, &buf, 1)) - abort(); + sem = *(uv_semaphore_t**)sem_; + uv_mutex_lock(&sem->mutex); + sem->value++; + if (sem->value == 1) + uv_cond_signal(&sem->cond); + uv_mutex_unlock(&sem->mutex); } -void uv_sem_wait(uv_sem_t* sem) { - struct sembuf buf; - int op_status; - - buf.sem_num = 0; - buf.sem_op = -1; - buf.sem_flg = 0; - do - op_status = semop(*sem, &buf, 1); - while (op_status == -1 && errno == EINTR); +static void uv__custom_sem_wait(uv_sem_t* sem_) { + uv_semaphore_t* sem; - if (op_status) - abort(); + sem = *(uv_semaphore_t**)sem_; + uv_mutex_lock(&sem->mutex); + while (sem->value == 0) + uv_cond_wait(&sem->cond, &sem->mutex); + sem->value--; + uv_mutex_unlock(&sem->mutex); } -int uv_sem_trywait(uv_sem_t* sem) { - struct sembuf buf; - int op_status; - buf.sem_num = 0; - buf.sem_op = -1; - buf.sem_flg = IPC_NOWAIT; +static int uv__custom_sem_trywait(uv_sem_t* sem_) { + uv_semaphore_t* sem; - do - op_status = semop(*sem, &buf, 1); - while (op_status == -1 && errno == EINTR); + sem = *(uv_semaphore_t**)sem_; + if (uv_mutex_trylock(&sem->mutex) != 0) + return UV_EAGAIN; - if (op_status) { - if (errno == EAGAIN) - return UV_EAGAIN; - abort(); + if (sem->value == 0) { + uv_mutex_unlock(&sem->mutex); + return UV_EAGAIN; } + sem->value--; + uv_mutex_unlock(&sem->mutex); + return 0; } -#else /* !(defined(__APPLE__) && defined(__MACH__)) */ - -int uv_sem_init(uv_sem_t* sem, unsigned int value) { +static int uv__sem_init(uv_sem_t* sem, unsigned int value) { if (sem_init(sem, 0, value)) return UV__ERR(errno); return 0; } -void uv_sem_destroy(uv_sem_t* sem) { +static void uv__sem_destroy(uv_sem_t* sem) { if (sem_destroy(sem)) abort(); } -void uv_sem_post(uv_sem_t* sem) { +static void uv__sem_post(uv_sem_t* sem) { if (sem_post(sem)) abort(); } -void uv_sem_wait(uv_sem_t* sem) { +static void uv__sem_wait(uv_sem_t* sem) { int r; do @@ -533,7 +573,7 @@ void uv_sem_wait(uv_sem_t* sem) { } -int uv_sem_trywait(uv_sem_t* sem) { +static int uv__sem_trywait(uv_sem_t* sem) { int r; do @@ -549,6 +589,49 @@ int uv_sem_trywait(uv_sem_t* sem) { return 0; } +int uv_sem_init(uv_sem_t* sem, unsigned int value) { +#ifdef __GLIBC__ + uv_once(&glibc_version_check_once, glibc_version_check); +#endif + + if (platform_needs_custom_semaphore) + return uv__custom_sem_init(sem, value); + else + return uv__sem_init(sem, value); +} + + +void uv_sem_destroy(uv_sem_t* sem) { + if (platform_needs_custom_semaphore) + uv__custom_sem_destroy(sem); + else + uv__sem_destroy(sem); +} + + +void uv_sem_post(uv_sem_t* sem) { + if (platform_needs_custom_semaphore) + uv__custom_sem_post(sem); + else + uv__sem_post(sem); +} + + +void uv_sem_wait(uv_sem_t* sem) { + if (platform_needs_custom_semaphore) + uv__custom_sem_wait(sem); + else + uv__sem_wait(sem); +} + + +int uv_sem_trywait(uv_sem_t* sem) { + if (platform_needs_custom_semaphore) + return uv__custom_sem_trywait(sem); + else + return uv__sem_trywait(sem); +} + #endif /* defined(__APPLE__) && defined(__MACH__) */ diff --git a/deps/uv/src/win/fs.c b/deps/uv/src/win/fs.c index b07d47cf56b..30b87ac5154 100644 --- a/deps/uv/src/win/fs.c +++ b/deps/uv/src/win/fs.c @@ -434,8 +434,6 @@ void fs__open(uv_fs_t* req) { access |= FILE_APPEND_DATA; } - access |= FILE_WRITE_ATTRIBUTES; - /* * Here is where we deviate significantly from what CRT's _open() * does. We indiscriminately use all the sharing modes, to match diff --git a/deps/uv/src/win/util.c b/deps/uv/src/win/util.c index 3c1d9bed1db..5d1c812dd75 100644 --- a/deps/uv/src/win/util.c +++ b/deps/uv/src/win/util.c @@ -587,8 +587,8 @@ int uv_uptime(double* uptime) { BYTE* address = (BYTE*) object_type + object_type->DefinitionLength + counter_definition->CounterOffset; uint64_t value = *((uint64_t*) address); - *uptime = (double) (object_type->PerfTime.QuadPart - value) / - (double) object_type->PerfFreq.QuadPart; + *uptime = floor((double) (object_type->PerfTime.QuadPart - value) / + (double) object_type->PerfFreq.QuadPart); uv__free(malloced_buffer); return 0; } @@ -615,7 +615,7 @@ int uv_cpu_info(uv_cpu_info_t** cpu_infos_ptr, int* cpu_count_ptr) { SYSTEM_PROCESSOR_PERFORMANCE_INFORMATION* sppi; DWORD sppi_size; SYSTEM_INFO system_info; - DWORD cpu_count, r, i; + DWORD cpu_count, i; NTSTATUS status; ULONG result_size; int err; @@ -670,34 +670,33 @@ int uv_cpu_info(uv_cpu_info_t** cpu_infos_ptr, int* cpu_count_ptr) { assert(len > 0 && len < ARRAY_SIZE(key_name)); - r = RegOpenKeyExW(HKEY_LOCAL_MACHINE, - key_name, - 0, - KEY_QUERY_VALUE, - &processor_key); - if (r != ERROR_SUCCESS) { - err = GetLastError(); + err = RegOpenKeyExW(HKEY_LOCAL_MACHINE, + key_name, + 0, + KEY_QUERY_VALUE, + &processor_key); + if (err != ERROR_SUCCESS) { goto error; } - if (RegQueryValueExW(processor_key, - L"~MHz", - NULL, - NULL, - (BYTE*) &cpu_speed, - &cpu_speed_size) != ERROR_SUCCESS) { - err = GetLastError(); + err = RegQueryValueExW(processor_key, + L"~MHz", + NULL, + NULL, + (BYTE*)&cpu_speed, + &cpu_speed_size); + if (err != ERROR_SUCCESS) { RegCloseKey(processor_key); goto error; } - if (RegQueryValueExW(processor_key, - L"ProcessorNameString", - NULL, - NULL, - (BYTE*) &cpu_brand, - &cpu_brand_size) != ERROR_SUCCESS) { - err = GetLastError(); + err = RegQueryValueExW(processor_key, + L"ProcessorNameString", + NULL, + NULL, + (BYTE*)&cpu_brand, + &cpu_brand_size); + if (err != ERROR_SUCCESS) { RegCloseKey(processor_key); goto error; } diff --git a/deps/uv/test/test-fs.c b/deps/uv/test/test-fs.c index 0075a02be66..000a151a64e 100644 --- a/deps/uv/test/test-fs.c +++ b/deps/uv/test/test-fs.c @@ -1367,28 +1367,6 @@ TEST_IMPL(fs_chmod) { check_permission("test_file", 0600); -#ifdef _WIN32 - /* Test clearing read-only flag from files with Archive flag cleared */ - /* Make the file read-only and clear archive flag */ - r = SetFileAttributes("test_file", FILE_ATTRIBUTE_READONLY); - ASSERT(r != 0); - check_permission("test_file", 0400); - - r = uv_fs_open(NULL, &req, "test_file", 0, 0, NULL); - ASSERT(r >= 0); - ASSERT(req.result >= 0); - uv_fs_req_cleanup(&req); - - r = uv_fs_fchmod(NULL, &req, file, 0600, NULL); - ASSERT(r == 0); - ASSERT(req.result == 0); - uv_fs_req_cleanup(&req); - - check_permission("test_file", 0600); - /* Restore Archive flag for rest of the tests */ - r = SetFileAttributes("test_file", FILE_ATTRIBUTE_ARCHIVE); - ASSERT(r != 0); -#endif #ifndef _WIN32 /* async chmod */ { diff --git a/deps/uv/test/test-pipe-set-fchmod.c b/deps/uv/test/test-pipe-set-fchmod.c index 59f0e6f5454..de4932dc1ae 100644 --- a/deps/uv/test/test-pipe-set-fchmod.c +++ b/deps/uv/test/test-pipe-set-fchmod.c @@ -27,6 +27,9 @@ TEST_IMPL(pipe_set_chmod) { uv_pipe_t pipe_handle; uv_loop_t* loop; int r; +#ifndef _WIN32 + struct stat stat_buf; +#endif loop = uv_default_loop(); @@ -44,12 +47,33 @@ TEST_IMPL(pipe_set_chmod) { RETURN_SKIP("Insufficient privileges to alter pipe fmode"); } ASSERT(r == 0); +#ifndef _WIN32 + stat(TEST_PIPENAME, &stat_buf); + ASSERT(stat_buf.st_mode & S_IRUSR); + ASSERT(stat_buf.st_mode & S_IRGRP); + ASSERT(stat_buf.st_mode & S_IROTH); +#endif r = uv_pipe_chmod(&pipe_handle, UV_WRITABLE); ASSERT(r == 0); +#ifndef _WIN32 + stat(TEST_PIPENAME, &stat_buf); + ASSERT(stat_buf.st_mode & S_IWUSR); + ASSERT(stat_buf.st_mode & S_IWGRP); + ASSERT(stat_buf.st_mode & S_IWOTH); +#endif r = uv_pipe_chmod(&pipe_handle, UV_WRITABLE | UV_READABLE); ASSERT(r == 0); +#ifndef _WIN32 + stat(TEST_PIPENAME, &stat_buf); + ASSERT(stat_buf.st_mode & S_IRUSR); + ASSERT(stat_buf.st_mode & S_IRGRP); + ASSERT(stat_buf.st_mode & S_IROTH); + ASSERT(stat_buf.st_mode & S_IWUSR); + ASSERT(stat_buf.st_mode & S_IWGRP); + ASSERT(stat_buf.st_mode & S_IWOTH); +#endif r = uv_pipe_chmod(NULL, UV_WRITABLE | UV_READABLE); ASSERT(r == UV_EBADF); diff --git a/deps/uv/test/test-udp-ipv6.c b/deps/uv/test/test-udp-ipv6.c index 00007918555..9d4db2bc4fe 100644 --- a/deps/uv/test/test-udp-ipv6.c +++ b/deps/uv/test/test-udp-ipv6.c @@ -174,6 +174,8 @@ TEST_IMPL(udp_dual_stack) { #if defined(__FreeBSD__) || defined(__FreeBSD_kernel__) || defined(__NetBSD__) if (!can_ipv6_ipv4_dual()) RETURN_SKIP("IPv6-IPv4 dual stack not supported"); +#elif defined(__OpenBSD__) + RETURN_SKIP("IPv6-IPv4 dual stack not supported"); #endif do_test(ipv6_recv_ok, 0); diff --git a/deps/uv/test/test-udp-multicast-join6.c b/deps/uv/test/test-udp-multicast-join6.c index 8814b5ad13c..cf316e107a0 100644 --- a/deps/uv/test/test-udp-multicast-join6.c +++ b/deps/uv/test/test-udp-multicast-join6.c @@ -123,7 +123,8 @@ TEST_IMPL(udp_multicast_join6) { defined(_AIX) || \ defined(__MVS__) || \ defined(__FreeBSD_kernel__) || \ - defined(__NetBSD__) + defined(__NetBSD__) || \ + defined(__OpenBSD__) r = uv_udp_set_membership(&client, "ff02::1", "::1%lo0", UV_JOIN_GROUP); #else r = uv_udp_set_membership(&client, "ff02::1", NULL, UV_JOIN_GROUP); From 2fd7284add63b3d7101be35c3de6da5e321f7d5b Mon Sep 17 00:00:00 2001 From: Bill Ticehurst Date: Wed, 18 Apr 2018 11:55:35 -0700 Subject: [PATCH 28/43] test: add test for loading read-only modules Adds a test-case to cover loading modules the user does not have permission to write to. Covers issue logged in https://github.com/nodejs/node/issues/20112 PR-URL: https://github.com/nodejs/node/pull/20138 Refs: https://github.com/nodejs/node/issues/20112 Reviewed-By: Rich Trott Reviewed-By: Richard Lau Reviewed-By: Vse Mozhet Byt Reviewed-By: Bartosz Sosnowski Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- test/parallel/test-module-readonly.js | 48 +++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 test/parallel/test-module-readonly.js diff --git a/test/parallel/test-module-readonly.js b/test/parallel/test-module-readonly.js new file mode 100644 index 00000000000..fa12471a37c --- /dev/null +++ b/test/parallel/test-module-readonly.js @@ -0,0 +1,48 @@ +'use strict'; + +const common = require('../common'); + +if (!common.isWindows) { + // TODO: Similar checks on *nix-like systems (e.g using chmod or the like) + common.skip('test only runs on Windows'); +} + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const cp = require('child_process'); + +const tmpdir = require('../common/tmpdir'); +tmpdir.refresh(); + +// Create readOnlyMod.js and set to read only +const readOnlyMod = path.join(tmpdir.path, 'readOnlyMod'); +const readOnlyModRelative = path.relative(__dirname, readOnlyMod); +const readOnlyModFullPath = `${readOnlyMod}.js`; + +fs.writeFileSync(readOnlyModFullPath, 'module.exports = 42;'); + +// Removed any inherited ACEs, and any explicitly granted ACEs for the +// current user +cp.execSync( + `icacls.exe "${readOnlyModFullPath}" /inheritance:r /remove "%USERNAME%"`); + +// Grant the current user read & execute only +cp.execSync(`icacls.exe "${readOnlyModFullPath}" /grant "%USERNAME%":RX`); + +let except = null; +try { + // Attempt to load the module. Will fail if write access is required + require(readOnlyModRelative); +} catch (err) { + except = err; +} + +// Remove the expliclty granted rights, and reenable inheritance +cp.execSync( + `icacls.exe "${readOnlyModFullPath}" /remove "%USERNAME%" /inheritance:e`); + +// Delete the test module (note: tmpdir should get cleaned anyway) +fs.unlinkSync(readOnlyModFullPath); + +assert.ifError(except); From f274e6921ff1385d5acfca7ac404c9429a4e3f7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Nie=C3=9Fen?= Date: Sat, 21 Apr 2018 10:56:47 +0200 Subject: [PATCH 29/43] crypto: fix explanation in CipherBase::SetAuthTag PR-URL: https://github.com/nodejs/node/pull/20197 Reviewed-By: Anna Henningsen Reviewed-By: Luigi Pinca Reviewed-By: Colin Ihrig --- src/node_crypto.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/node_crypto.cc b/src/node_crypto.cc index 7b427e37b01..111227e665c 100644 --- a/src/node_crypto.cc +++ b/src/node_crypto.cc @@ -2911,7 +2911,7 @@ void CipherBase::SetAuthTag(const FunctionCallbackInfo& args) { } } - // Note: we don't use std::max() here to work around a header conflict. + // Note: we don't use std::min() here to work around a header conflict. cipher->auth_tag_len_ = tag_len; if (cipher->auth_tag_len_ > sizeof(cipher->auth_tag_)) cipher->auth_tag_len_ = sizeof(cipher->auth_tag_); From bb3ead8ba6bbc4222c735945ac63b8170843b5f6 Mon Sep 17 00:00:00 2001 From: Dhansuhu Uzumaki Date: Fri, 20 Apr 2018 20:31:26 +0530 Subject: [PATCH 30/43] test: update non-string header names should throw PR-URL: https://github.com/nodejs/node/pull/20172 Reviewed-By: Anna Henningsen Reviewed-By: Tiancheng "Timothy" Gu Reviewed-By: James M Snell Reviewed-By: Trivikram Kamat Reviewed-By: Ujjwal Sharma Reviewed-By: Colin Ihrig --- test/parallel/test-http-write-head.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/parallel/test-http-write-head.js b/test/parallel/test-http-write-head.js index f2ac7cb7b39..83dfcfa20bd 100644 --- a/test/parallel/test-http-write-head.js +++ b/test/parallel/test-http-write-head.js @@ -31,14 +31,15 @@ const s = http.createServer(common.mustCall((req, res) => { res.setHeader('test', '1'); // toLowerCase() is used on the name argument, so it must be a string. - let threw = false; - try { - res.setHeader(0xf00, 'bar'); - } catch (e) { - assert.ok(e instanceof TypeError); - threw = true; - } - assert.ok(threw, 'Non-string names should throw'); + // Non-String header names should throw + common.expectsError( + () => res.setHeader(0xf00, 'bar'), + { + code: 'ERR_INVALID_HTTP_TOKEN', + type: TypeError, + message: 'Header name must be a valid HTTP token ["3840"]' + } + ); // undefined value should throw, via 979d0ca8 common.expectsError( From b7d1e19e30c0e7454ddcd7dda8d81e62acf7b3e1 Mon Sep 17 00:00:00 2001 From: Beni von Cheni Date: Tue, 17 Apr 2018 00:40:22 -0400 Subject: [PATCH 31/43] doc: update trace events categories description PR-URL: https://github.com/nodejs/node/pull/20092 Fixes: https://github.com/nodejs/node/issues/16315 Reviewed-By: Vse Mozhet Byt Reviewed-By: Trivikram Kamat --- doc/api/tracing.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/doc/api/tracing.md b/doc/api/tracing.md index e07320a0163..0b46abf81b1 100644 --- a/doc/api/tracing.md +++ b/doc/api/tracing.md @@ -13,15 +13,17 @@ a list of comma-separated category names. The available categories are: -* `node` -* `node.async_hooks` - Enables capture of detailed async_hooks trace data. +* `node` - An empty placeholder. +* `node.async_hooks` - Enables capture of detailed [async_hooks] trace data. + The [async_hooks] events have a unique `asyncId` and a special triggerId + `triggerAsyncId` property. * `node.bootstrap` - Enables capture of Node.js bootstrap milestones. * `node.perf` - Enables capture of [Performance API] measurements. * `node.perf.usertiming` - Enables capture of only Performance API User Timing measures and marks. * `node.perf.timerify` - Enables capture of only Performance API timerify measurements. -* `v8` +* `v8` - The [V8] events are GC, compiling, and execution related. By default the `node`, `node.async_hooks`, and `v8` categories are enabled. @@ -193,3 +195,5 @@ console.log(trace_events.getEnabledCategories()); ``` [Performance API]: perf_hooks.html +[V8]: v8.html +[async_hooks]: async_hooks.html From 65e62e5665839dec1f93160c68b5ac2083afb06f Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 7 Apr 2018 16:57:22 +0800 Subject: [PATCH 32/43] fs: return stats to JS in sync methods - Reduce reference to the global `statValues` by returning the changed stats array from the synchronous methods. Having a local returned value also makes the future integration of BigInt easier. - Also returns the filled array from node::FillGlobalStatsArray and node::FillStatsArray in the C++ side. PR-URL: https://github.com/nodejs/node/pull/20167 Reviewed-By: Ruben Bridgewater Reviewed-By: Anna Henningsen Reviewed-By: Khaidi Chu Reviewed-By: Gus Caplan --- lib/fs.js | 63 ++++++++++++++++++++-------------------- lib/internal/fs.js | 6 ---- src/node_file.cc | 30 ++++++++++++------- src/node_internals.h | 12 ++++---- src/node_stat_watcher.cc | 4 +-- 5 files changed, 60 insertions(+), 55 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 6c8417030b7..d71e31565fb 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -57,7 +57,6 @@ const { preprocessSymlinkDestination, Stats, getStatsFromBinding, - getStatsFromGlobalBinding, stringToFlags, stringToSymlinkType, toUnixTimestamp, @@ -158,10 +157,10 @@ function isFd(path) { fs.Stats = Stats; -function isFileType(fileType) { +function isFileType(stats, fileType) { // Use stats array directly to avoid creating an fs.Stats instance just for // our internal use. - return (statValues[1/* mode */] & S_IFMT) === fileType; + return (stats[1/* mode */] & S_IFMT) === fileType; } // Don't allow mode to accidentally be overwritten. @@ -330,15 +329,15 @@ function readFileAfterOpen(err, fd) { binding.fstat(fd, req); } -function readFileAfterStat(err) { +function readFileAfterStat(err, stats) { var context = this.context; if (err) return context.close(err); var size; - if (isFileType(S_IFREG)) - size = context.size = statValues[8]; + if (isFileType(stats, S_IFREG)) + size = context.size = stats[8]; else size = context.size = 0; @@ -411,11 +410,12 @@ function readFileAfterClose(err) { function tryStatSync(fd, isUserFd) { const ctx = {}; - binding.fstat(fd, undefined, ctx); + const stats = binding.fstat(fd, undefined, ctx); if (ctx.errno !== undefined && !isUserFd) { fs.closeSync(fd); throw errors.uvException(ctx); } + return stats; } function tryCreateBuffer(size, fd, isUserFd) { @@ -450,10 +450,10 @@ fs.readFileSync = function(path, options) { var isUserFd = isFd(path); // file descriptor ownership var fd = isUserFd ? path : fs.openSync(path, options.flag || 'r', 0o666); - tryStatSync(fd, isUserFd); + const stats = tryStatSync(fd, isUserFd); var size; - if (isFileType(S_IFREG)) - size = statValues[8]; + if (isFileType(stats, S_IFREG)) + size = stats[8]; else size = 0; var pos = 0; @@ -890,27 +890,29 @@ fs.stat = function(path, callback) { fs.fstatSync = function(fd) { validateUint32(fd, 'fd'); const ctx = { fd }; - binding.fstat(fd, undefined, ctx); + const stats = binding.fstat(fd, undefined, ctx); handleErrorFromBinding(ctx); - return getStatsFromGlobalBinding(); + return getStatsFromBinding(stats); }; fs.lstatSync = function(path) { path = getPathFromURL(path); validatePath(path); const ctx = { path }; - binding.lstat(pathModule.toNamespacedPath(path), undefined, ctx); + const stats = binding.lstat(pathModule.toNamespacedPath(path), + undefined, ctx); handleErrorFromBinding(ctx); - return getStatsFromGlobalBinding(); + return getStatsFromBinding(stats); }; fs.statSync = function(path) { path = getPathFromURL(path); validatePath(path); const ctx = { path }; - binding.stat(pathModule.toNamespacedPath(path), undefined, ctx); + const stats = binding.stat(pathModule.toNamespacedPath(path), + undefined, ctx); handleErrorFromBinding(ctx); - return getStatsFromGlobalBinding(); + return getStatsFromBinding(stats); }; fs.readlink = function(path, options, callback) { @@ -1439,7 +1441,7 @@ function StatWatcher() { this._handle.onchange = function(newStatus, stats) { if (oldStatus === -1 && newStatus === -1 && - statValues[2/* new nlink */] === statValues[16/* old nlink */]) return; + stats[2/* new nlink */] === stats[16/* old nlink */]) return; oldStatus = newStatus; self.emit('change', getStatsFromBinding(stats), @@ -1666,7 +1668,8 @@ fs.realpathSync = function realpathSync(p, options) { // continue if not a symlink, break if a pipe/socket if (knownHard[base] || (cache && cache.get(base) === base)) { - if (isFileType(S_IFIFO) || isFileType(S_IFSOCK)) { + if (isFileType(statValues, S_IFIFO) || + isFileType(statValues, S_IFSOCK)) { break; } continue; @@ -1682,10 +1685,10 @@ fs.realpathSync = function realpathSync(p, options) { var baseLong = pathModule.toNamespacedPath(base); const ctx = { path: base }; - binding.lstat(baseLong, undefined, ctx); + const stats = binding.lstat(baseLong, undefined, ctx); handleErrorFromBinding(ctx); - if (!isFileType(S_IFLNK)) { + if (!isFileType(stats, S_IFLNK)) { knownHard[base] = true; if (cache) cache.set(base, base); continue; @@ -1696,8 +1699,8 @@ fs.realpathSync = function realpathSync(p, options) { var linkTarget = null; var id; if (!isWindows) { - var dev = statValues[0].toString(32); - var ino = statValues[7].toString(32); + var dev = stats[0].toString(32); + var ino = stats[7].toString(32); id = `${dev}:${ino}`; if (seenLinks[id]) { linkTarget = seenLinks[id]; @@ -1778,7 +1781,7 @@ fs.realpath = function realpath(p, options, callback) { // On windows, check that the root exists. On unix there is no need. if (isWindows && !knownHard[base]) { - fs.lstat(base, function(err) { + fs.lstat(base, function(err, stats) { if (err) return callback(err); knownHard[base] = true; LOOP(); @@ -1811,7 +1814,8 @@ fs.realpath = function realpath(p, options, callback) { // continue if not a symlink, break if a pipe/socket if (knownHard[base]) { - if (isFileType(S_IFIFO) || isFileType(S_IFSOCK)) { + if (isFileType(statValues, S_IFIFO) || + isFileType(statValues, S_IFSOCK)) { return callback(null, encodeRealpathResult(p, options)); } return process.nextTick(LOOP); @@ -1820,14 +1824,11 @@ fs.realpath = function realpath(p, options, callback) { return fs.lstat(base, gotStat); } - function gotStat(err) { + function gotStat(err, stats) { if (err) return callback(err); - // Use stats array directly to avoid creating an fs.Stats instance just for - // our internal use. - // if not a symlink, skip to the next path part - if (!isFileType(S_IFLNK)) { + if (!stats.isSymbolicLink()) { knownHard[base] = true; return process.nextTick(LOOP); } @@ -1837,8 +1838,8 @@ fs.realpath = function realpath(p, options, callback) { // dev/ino always return 0 on windows, so skip the check. let id; if (!isWindows) { - var dev = statValues[0].toString(32); - var ino = statValues[7].toString(32); + var dev = stats.dev.toString(32); + var ino = stats.ino.toString(32); id = `${dev}:${ino}`; if (seenLinks[id]) { return gotTarget(null, seenLinks[id], base); diff --git a/lib/internal/fs.js b/lib/internal/fs.js index b431cb910c2..0cfb89a9d3d 100644 --- a/lib/internal/fs.js +++ b/lib/internal/fs.js @@ -35,7 +35,6 @@ const { UV_FS_SYMLINK_DIR, UV_FS_SYMLINK_JUNCTION } = process.binding('constants').fs; -const { statValues } = process.binding('fs'); const isWindows = process.platform === 'win32'; @@ -217,10 +216,6 @@ function getStatsFromBinding(stats, offset = 0) { stats[12 + offset], stats[13 + offset]); } -function getStatsFromGlobalBinding(offset = 0) { - return getStatsFromBinding(statValues, offset); -} - function stringToFlags(flags) { if (typeof flags === 'number') { return flags; @@ -442,7 +437,6 @@ module.exports = { preprocessSymlinkDestination, realpathCacheKey: Symbol('realpathCacheKey'), getStatsFromBinding, - getStatsFromGlobalBinding, stringToFlags, stringToSymlinkType, Stats, diff --git a/src/node_file.cc b/src/node_file.cc index dcbdf86579d..89c53afc5b1 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -410,8 +410,7 @@ void FSReqWrap::Reject(Local reject) { } void FSReqWrap::ResolveStat(const uv_stat_t* stat) { - node::FillGlobalStatsArray(env(), stat); - Resolve(env()->fs_stats_field_array()->GetJSArray()); + Resolve(node::FillGlobalStatsArray(env(), stat)); } void FSReqWrap::Resolve(Local value) { @@ -832,10 +831,13 @@ static void Stat(const FunctionCallbackInfo& args) { FS_SYNC_TRACE_BEGIN(stat); int err = SyncCall(env, args[2], &req_wrap_sync, "stat", uv_fs_stat, *path); FS_SYNC_TRACE_END(stat); - if (err == 0) { - node::FillGlobalStatsArray(env, - static_cast(req_wrap_sync.req.ptr)); + if (err != 0) { + return; // error info is in ctx } + + Local arr = node::FillGlobalStatsArray(env, + static_cast(req_wrap_sync.req.ptr)); + args.GetReturnValue().Set(arr); } } @@ -859,10 +861,13 @@ static void LStat(const FunctionCallbackInfo& args) { int err = SyncCall(env, args[2], &req_wrap_sync, "lstat", uv_fs_lstat, *path); FS_SYNC_TRACE_END(lstat); - if (err == 0) { - node::FillGlobalStatsArray(env, - static_cast(req_wrap_sync.req.ptr)); + if (err != 0) { + return; // error info is in ctx } + + Local arr = node::FillGlobalStatsArray(env, + static_cast(req_wrap_sync.req.ptr)); + args.GetReturnValue().Set(arr); } } @@ -885,10 +890,13 @@ static void FStat(const FunctionCallbackInfo& args) { FS_SYNC_TRACE_BEGIN(fstat); int err = SyncCall(env, args[2], &req_wrap_sync, "fstat", uv_fs_fstat, fd); FS_SYNC_TRACE_END(fstat); - if (err == 0) { - node::FillGlobalStatsArray(env, - static_cast(req_wrap_sync.req.ptr)); + if (err != 0) { + return; // error info is in ctx } + + Local arr = node::FillGlobalStatsArray(env, + static_cast(req_wrap_sync.req.ptr)); + args.GetReturnValue().Set(arr); } } diff --git a/src/node_internals.h b/src/node_internals.h index 492a36296fb..50047e33072 100644 --- a/src/node_internals.h +++ b/src/node_internals.h @@ -313,7 +313,7 @@ v8::Maybe ProcessEmitDeprecationWarning(Environment* env, const char* deprecation_code); template -void FillStatsArray(AliasedBuffer* fields_ptr, +v8::Local FillStatsArray(AliasedBuffer* fields_ptr, const uv_stat_t* s, int offset = 0) { AliasedBuffer& fields = *fields_ptr; fields[offset + 0] = s->st_dev; @@ -347,12 +347,14 @@ void FillStatsArray(AliasedBuffer* fields_ptr, X(12, ctim) X(13, birthtim) #undef X + + return fields_ptr->GetJSArray(); } -inline void FillGlobalStatsArray(Environment* env, - const uv_stat_t* s, - int offset = 0) { - node::FillStatsArray(env->fs_stats_field_array(), s, offset); +inline v8::Local FillGlobalStatsArray(Environment* env, + const uv_stat_t* s, + int offset = 0) { + return node::FillStatsArray(env->fs_stats_field_array(), s, offset); } void SetupProcessObject(Environment* env, diff --git a/src/node_stat_watcher.cc b/src/node_stat_watcher.cc index 878d82b8dfe..41683a0dc1d 100644 --- a/src/node_stat_watcher.cc +++ b/src/node_stat_watcher.cc @@ -108,12 +108,12 @@ void StatWatcher::Callback(uv_fs_poll_t* handle, HandleScope handle_scope(env->isolate()); Context::Scope context_scope(env->context()); - node::FillGlobalStatsArray(env, curr); + Local arr = node::FillGlobalStatsArray(env, curr); node::FillGlobalStatsArray(env, prev, env->kFsStatsFieldsLength); Local argv[2] { Integer::New(env->isolate(), status), - env->fs_stats_field_array()->GetJSArray() + arr }; wrap->MakeCallback(env->onchange_string(), arraysize(argv), argv); } From 69495436e2efeaff1c22c17f2cb5c3b7ba84638f Mon Sep 17 00:00:00 2001 From: Gabriel Schulhof Date: Fri, 20 Apr 2018 22:27:43 -0400 Subject: [PATCH 33/43] src: cover extra load-via-special-symbol scenario We need to look for a special symbol even if the module self-registers when the module self-registers with the wrong version. PR-URL: https://github.com/nodejs/node/pull/20186 Reviewed-By: Ben Noordhuis Reviewed-By: Joyee Cheung --- src/node.cc | 7 +++++++ test/addons/hello-world/binding.cc | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/src/node.cc b/src/node.cc index b4853091b9d..90defba40ba 100644 --- a/src/node.cc +++ b/src/node.cc @@ -2292,6 +2292,13 @@ static void DLOpen(const FunctionCallbackInfo& args) { // -1 is used for N-API modules if ((mp->nm_version != -1) && (mp->nm_version != NODE_MODULE_VERSION)) { + // Even if the module did self-register, it may have done so with the wrong + // version. We must only give up after having checked to see if it has an + // appropriate initializer callback. + if (auto callback = GetInitializerCallback(&dlib)) { + callback(exports, module, context); + return; + } char errmsg[1024]; snprintf(errmsg, sizeof(errmsg), diff --git a/test/addons/hello-world/binding.cc b/test/addons/hello-world/binding.cc index ba6a22d7196..e267a3b2a76 100644 --- a/test/addons/hello-world/binding.cc +++ b/test/addons/hello-world/binding.cc @@ -15,3 +15,20 @@ extern "C" NODE_MODULE_EXPORT void INITIALIZER(v8::Local exports, v8::Local context) { NODE_SET_METHOD(exports, "hello", Method); } + +static void FakeInit(v8::Local exports, + v8::Local module, + v8::Local context) { + auto isolate = context->GetIsolate(); + auto exception = v8::Exception::Error(v8::String::NewFromUtf8(isolate, + "FakeInit should never run!", v8::NewStringType::kNormal) + .ToLocalChecked()); + isolate->ThrowException(exception); +} + +// Define a Node.js module, but with the wrong version. Node.js should still be +// able to load this module, multiple times even, because it exposes the +// specially named initializer above. +#undef NODE_MODULE_VERSION +#define NODE_MODULE_VERSION 3 +NODE_MODULE(NODE_GYP_MODULE_NAME, FakeInit) From c322fca2adb66ffcf96e81c21bf4b0eaf7f75f22 Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Wed, 4 Apr 2018 17:03:58 +0200 Subject: [PATCH 34/43] test: improve common.expectsError The output is now improved by showing most properties all at once. Besides that this adds a warning to use `assert.throws` instead due to a better output. PR-URL: https://github.com/nodejs/node/pull/19797 Reviewed-By: James M Snell Reviewed-By: Joyee Cheung --- test/common/index.js | 73 +++++++++++++------ .../parallel/test-console-assign-undefined.js | 8 +- test/parallel/test-internal-errors.js | 4 +- 3 files changed, 55 insertions(+), 30 deletions(-) diff --git a/test/common/index.js b/test/common/index.js index 7df3907f0e7..95bb8dd8048 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -690,6 +690,15 @@ Object.defineProperty(exports, 'hasSmallICU', { } }); +class Comparison { + constructor(obj, keys) { + for (const key of keys) { + if (key in obj) + this[key] = obj[key]; + } + } +} + // Useful for testing expected internal/error objects exports.expectsError = function expectsError(fn, settings, exact) { if (typeof fn !== 'function') { @@ -697,45 +706,63 @@ exports.expectsError = function expectsError(fn, settings, exact) { settings = fn; fn = undefined; } + function innerFn(error) { const descriptor = Object.getOwnPropertyDescriptor(error, 'message'); assert.strictEqual(descriptor.enumerable, false, 'The error message should be non-enumerable'); + + let innerSettings = settings; if ('type' in settings) { const type = settings.type; if (type !== Error && !Error.isPrototypeOf(type)) { throw new TypeError('`settings.type` must inherit from `Error`'); } - assert(error instanceof type, - `${error.name} is not instance of ${type.name}`); - let typeName = error.constructor.name; - if (typeName === 'NodeError' && type.name !== 'NodeError') { - typeName = Object.getPrototypeOf(error.constructor).name; + let constructor = error.constructor; + if (constructor.name === 'NodeError' && type.name !== 'NodeError') { + constructor = Object.getPrototypeOf(error.constructor); } - assert.strictEqual(typeName, type.name); + // Add the `type` to the error to properly compare and visualize it. + if (!('type' in error)) + error.type = constructor; } - if ('info' in settings) { - assert.deepStrictEqual(error.info, settings.info); - } - if ('message' in settings) { - const message = settings.message; - if (typeof message === 'string') { - assert.strictEqual(error.message, message); - } else { - assert(message.test(error.message), - `${error.message} does not match ${message}`); - } + + if ('message' in settings && + typeof settings.message === 'object' && + settings.message.test(error.message)) { + // Make a copy so we are able to modify the settings. + innerSettings = Object.create( + settings, Object.getOwnPropertyDescriptors(settings)); + // Visualize the message as identical in case of other errors. + innerSettings.message = error.message; } // Check all error properties. const keys = Object.keys(settings); for (const key of keys) { - if (key === 'message' || key === 'type' || key === 'info') - continue; - const actual = error[key]; - const expected = settings[key]; - assert.strictEqual(actual, expected, - `${key}: expected ${expected}, not ${actual}`); + if (!util.isDeepStrictEqual(error[key], innerSettings[key])) { + // Create placeholder objects to create a nice output. + const a = new Comparison(error, keys); + const b = new Comparison(innerSettings, keys); + + const tmpLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + const err = new assert.AssertionError({ + actual: a, + expected: b, + operator: 'strictEqual', + stackStartFn: assert.throws + }); + Error.stackTraceLimit = tmpLimit; + + throw new assert.AssertionError({ + actual: error, + expected: settings, + operator: 'common.expectsError', + message: err.message + }); + } + } return true; } diff --git a/test/parallel/test-console-assign-undefined.js b/test/parallel/test-console-assign-undefined.js index 1c47b3bda78..1021307b3c5 100644 --- a/test/parallel/test-console-assign-undefined.js +++ b/test/parallel/test-console-assign-undefined.js @@ -6,7 +6,7 @@ const tmp = global.console; global.console = 42; -const common = require('../common'); +require('../common'); const assert = require('assert'); // Originally the console had a getter. Test twice to verify it had no side @@ -14,11 +14,9 @@ const assert = require('assert'); assert.strictEqual(global.console, 42); assert.strictEqual(global.console, 42); -common.expectsError( +assert.throws( () => console.log('foo'), - { - type: TypeError - } + { name: 'TypeError' } ); global.console = 1; diff --git a/test/parallel/test-internal-errors.js b/test/parallel/test-internal-errors.js index a161a1db69e..cd2028c0f29 100644 --- a/test/parallel/test-internal-errors.js +++ b/test/parallel/test-internal-errors.js @@ -77,7 +77,7 @@ common.expectsError(() => { }, { code: 'TEST_ERROR_1', type: RangeError }); }, { code: 'ERR_ASSERTION', - message: /^.+ is not instance of \S/ + message: /- type: \[Function: TypeError]\n\+ type: \[Function: RangeError]/ }); common.expectsError(() => { @@ -89,7 +89,7 @@ common.expectsError(() => { }, { code: 'ERR_ASSERTION', type: assert.AssertionError, - message: /.+ does not match \S/ + message: /- message: 'Error for testing purposes: a'\n\+ message: \/\^Error/ }); // Test ERR_INVALID_FD_TYPE From d7ff4f07505f653a5bbdfb62521e35ec71cf38b5 Mon Sep 17 00:00:00 2001 From: Ashok Date: Sat, 14 Apr 2018 16:07:29 +0530 Subject: [PATCH 35/43] test: fix test when NODE_OPTIONS env var is set to --trace-warnings PR-URL: https://github.com/nodejs/node/pull/20027 Reviewed-By: Gibson Fahnestock Reviewed-By: James M Snell --- tools/test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/test.py b/tools/test.py index 8eaa2611d02..9bee9c2a222 100755 --- a/tools/test.py +++ b/tools/test.py @@ -54,6 +54,7 @@ VERBOSE = False +os.environ['NODE_OPTIONS'] = '' # --------------------------------------------- # --- P r o g r e s s I n d i c a t o r s --- From 6886dd1a6c67790b982a95f28dd126772fd67e79 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 09:26:16 +0200 Subject: [PATCH 36/43] http: cleanup _http_common.js Remove constant deep property access by storing a reference, use more const, make some conditionals stricter. PR-URL: https://github.com/nodejs/node/pull/20126 Reviewed-By: Luigi Pinca Reviewed-By: Daniel Bevenius Reviewed-By: Matteo Collina --- lib/_http_common.js | 51 +++++++++++++++++++++------------------------ 1 file changed, 24 insertions(+), 27 deletions(-) diff --git a/lib/_http_common.js b/lib/_http_common.js index 7f53aca246b..f34396ba8a8 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -65,12 +65,12 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method, const parser = this; const { socket } = parser; - if (!headers) { + if (headers === undefined) { headers = parser._headers; parser._headers = []; } - if (!url) { + if (url === undefined) { url = parser._url; parser._url = ''; } @@ -80,12 +80,12 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method, socket.server[kIncomingMessage]) || IncomingMessage; - parser.incoming = new ParserIncomingMessage(socket); - parser.incoming.httpVersionMajor = versionMajor; - parser.incoming.httpVersionMinor = versionMinor; - parser.incoming.httpVersion = `${versionMajor}.${versionMinor}`; - parser.incoming.url = url; - parser.incoming.upgrade = upgrade; + const incoming = parser.incoming = new ParserIncomingMessage(socket); + incoming.httpVersionMajor = versionMajor; + incoming.httpVersionMinor = versionMinor; + incoming.httpVersion = `${versionMajor}.${versionMinor}`; + incoming.url = url; + incoming.upgrade = upgrade; var n = headers.length; @@ -93,51 +93,48 @@ function parserOnHeadersComplete(versionMajor, versionMinor, headers, method, if (parser.maxHeaderPairs > 0) n = Math.min(n, parser.maxHeaderPairs); - parser.incoming._addHeaderLines(headers, n); + incoming._addHeaderLines(headers, n); if (typeof method === 'number') { // server only - parser.incoming.method = methods[method]; + incoming.method = methods[method]; } else { // client only - parser.incoming.statusCode = statusCode; - parser.incoming.statusMessage = statusMessage; + incoming.statusCode = statusCode; + incoming.statusMessage = statusMessage; } - return parser.onIncoming(parser.incoming, shouldKeepAlive); + return parser.onIncoming(incoming, shouldKeepAlive); } // XXX This is a mess. // TODO: http.Parser should be a Writable emits request/response events. function parserOnBody(b, start, len) { - var parser = this; - var stream = parser.incoming; + const stream = this.incoming; // if the stream has already been removed, then drop it. - if (!stream) + if (stream === null) return; - var socket = stream.socket; - // pretend this was the result of a stream._read call. if (len > 0 && !stream._dumped) { var slice = b.slice(start, start + len); var ret = stream.push(slice); if (!ret) - readStop(socket); + readStop(this.socket); } } function parserOnMessageComplete() { - var parser = this; - var stream = parser.incoming; + const parser = this; + const stream = parser.incoming; - if (stream) { + if (stream !== null) { stream.complete = true; // Emit any trailing headers. - var headers = parser._headers; - if (headers) { - parser.incoming._addHeaderLines(headers, headers.length); + const headers = parser._headers; + if (headers.length) { + stream._addHeaderLines(headers, headers.length); parser._headers = []; parser._url = ''; } @@ -151,8 +148,8 @@ function parserOnMessageComplete() { } -var parsers = new FreeList('parsers', 1000, function() { - var parser = new HTTPParser(HTTPParser.REQUEST); +const parsers = new FreeList('parsers', 1000, function() { + const parser = new HTTPParser(HTTPParser.REQUEST); parser._headers = []; parser._url = ''; From ea60148c1650d8b45214a31295b33356d6430a71 Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 09:28:16 +0200 Subject: [PATCH 37/43] http: remove duplicate comment A comment explaining kOnHeaders function of the parser was duplicated in two places but the second instance was more confusing than helpful. PR-URL: https://github.com/nodejs/node/pull/20126 Reviewed-By: Luigi Pinca Reviewed-By: Daniel Bevenius Reviewed-By: Matteo Collina --- lib/_http_common.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/lib/_http_common.js b/lib/_http_common.js index f34396ba8a8..c8998f4d85a 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -159,11 +159,6 @@ const parsers = new FreeList('parsers', 1000, function() { parser.incoming = null; parser.outgoing = null; - // Only called in the slow case where slow means - // that the request headers were either fragmented - // across multiple TCP packets or too large to be - // processed in a single run. This method is also - // called to process trailing HTTP headers. parser[kOnHeaders] = parserOnHeaders; parser[kOnHeadersComplete] = parserOnHeadersComplete; parser[kOnBody] = parserOnBody; From cda94b2bb82315e498f8dbfcb33126c7c04ed92e Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 10:17:06 +0200 Subject: [PATCH 38/43] http: cleanup parser properties Cleanup constructor and freeParser to manage all existing parser properties, not just some. PR-URL: https://github.com/nodejs/node/pull/20126 Reviewed-By: Luigi Pinca Reviewed-By: Daniel Bevenius Reviewed-By: Matteo Collina --- lib/_http_client.js | 4 ---- lib/_http_common.js | 7 +++++++ lib/_http_server.js | 4 ---- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/lib/_http_client.js b/lib/_http_client.js index c3ef9de2049..22ae85c9278 100644 --- a/lib/_http_client.js +++ b/lib/_http_client.js @@ -606,7 +606,6 @@ function tickOnSocket(req, socket) { req.connection = socket; parser.reinitialize(HTTPParser.RESPONSE); parser.socket = socket; - parser.incoming = null; parser.outgoing = req; req.parser = parser; @@ -619,9 +618,6 @@ function tickOnSocket(req, socket) { // Propagate headers limit from request object to parser if (typeof req.maxHeadersCount === 'number') { parser.maxHeaderPairs = req.maxHeadersCount << 1; - } else { - // Set default value because parser may be reused from FreeList - parser.maxHeaderPairs = 2000; } parser.onIncoming = parserOnIncomingClient; diff --git a/lib/_http_common.js b/lib/_http_common.js index c8998f4d85a..7eb37511bd1 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -41,6 +41,8 @@ const kOnBody = HTTPParser.kOnBody | 0; const kOnMessageComplete = HTTPParser.kOnMessageComplete | 0; const kOnExecute = HTTPParser.kOnExecute | 0; +const MAX_HEADER_PAIRS = 2000; + // Only called in the slow case where slow means // that the request headers were either fragmented // across multiple TCP packets or too large to be @@ -159,6 +161,9 @@ const parsers = new FreeList('parsers', 1000, function() { parser.incoming = null; parser.outgoing = null; + parser.maxHeaderPairs = MAX_HEADER_PAIRS; + + parser.onIncoming = null; parser[kOnHeaders] = parserOnHeaders; parser[kOnHeadersComplete] = parserOnHeadersComplete; parser[kOnBody] = parserOnBody; @@ -180,6 +185,8 @@ function closeParserInstance(parser) { parser.close(); } function freeParser(parser, req, socket) { if (parser) { parser._headers = []; + parser._url = ''; + parser.maxHeaderPairs = MAX_HEADER_PAIRS; parser.onIncoming = null; if (parser._consumed) parser.unconsume(); diff --git a/lib/_http_server.js b/lib/_http_server.js index c3abfd37bb5..1d6cada3270 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -342,14 +342,10 @@ function connectionListenerInternal(server, socket) { parser.reinitialize(HTTPParser.REQUEST); parser.socket = socket; socket.parser = parser; - parser.incoming = null; // Propagate headers limit from server instance to parser if (typeof server.maxHeadersCount === 'number') { parser.maxHeaderPairs = server.maxHeadersCount << 1; - } else { - // Set default value because parser may be reused from FreeList - parser.maxHeaderPairs = 2000; } var state = { From 49275c450a9108bdaa60fc4b2e24908c8e0f7c2f Mon Sep 17 00:00:00 2001 From: Anatoli Papirovski Date: Wed, 18 Apr 2018 10:20:13 +0200 Subject: [PATCH 39/43] http: remove duplicate parser unset freeParser already unsets parser property of socket if socket is passed in specifically. There's no need to do this twice. PR-URL: https://github.com/nodejs/node/pull/20126 Reviewed-By: Luigi Pinca Reviewed-By: Daniel Bevenius Reviewed-By: Matteo Collina --- lib/_http_common.js | 2 -- lib/_http_server.js | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/_http_common.js b/lib/_http_common.js index 7eb37511bd1..ab075dc817e 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -191,8 +191,6 @@ function freeParser(parser, req, socket) { if (parser._consumed) parser.unconsume(); parser._consumed = false; - if (parser.socket) - parser.socket.parser = null; parser.socket = null; parser.incoming = null; parser.outgoing = null; diff --git a/lib/_http_server.js b/lib/_http_server.js index 1d6cada3270..bf228de6434 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -522,7 +522,7 @@ function onParserExecuteCommon(server, socket, parser, state, ret, d) { socket.removeListener('error', socketOnError); unconsume(parser, socket); parser.finish(); - freeParser(parser, req, null); + freeParser(parser, req, socket); parser = null; var eventName = req.method === 'CONNECT' ? 'connect' : 'upgrade'; From a4cba2d7a4ad9f26541719cc052e92f6fec04646 Mon Sep 17 00:00:00 2001 From: Chris Miller Date: Wed, 18 Apr 2018 10:18:38 +0100 Subject: [PATCH 40/43] build: normalise test.py calls to use PARALLEL_ARGS PR-URL: https://github.com/nodejs/node/pull/20124 Fixes: https://github.com/nodejs/node/issues/20065 Reviewed-By: Anna Henningsen Reviewed-By: Joyee Cheung Reviewed-By: Richard Lau Reviewed-By: Ruben Bridgewater --- Makefile | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) diff --git a/Makefile b/Makefile index 00a3e46df88..f499788d9d8 100644 --- a/Makefile +++ b/Makefile @@ -18,6 +18,8 @@ PWD = $(CURDIR) ifdef JOBS PARALLEL_ARGS = -j $(JOBS) +else + PARALLEL_ARGS = -J endif ifdef ENABLE_V8_TAP @@ -232,7 +234,7 @@ v8: .PHONY: jstest jstest: build-addons build-addons-napi ## Runs addon tests and JS tests - $(PYTHON) tools/test.py --mode=release -J \ + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release \ $(CI_JS_SUITES) \ $(CI_NATIVE_SUITES) @@ -267,13 +269,13 @@ test-cov: all $(MAKE) lint test-parallel: all - $(PYTHON) tools/test.py --mode=release parallel -J + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release parallel test-valgrind: all - $(PYTHON) tools/test.py --mode=release --valgrind sequential parallel message + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release --valgrind sequential parallel message test-check-deopts: all - $(PYTHON) tools/test.py --mode=release --check-deopts parallel sequential -J + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release --check-deopts parallel sequential benchmark/misc/function_call/build/Release/binding.node: all \ benchmark/misc/function_call/binding.cc \ @@ -396,7 +398,7 @@ clear-stalled: .PHONY: test-gc test-gc: all test/gc/build/Release/binding.node - $(PYTHON) tools/test.py --mode=release gc + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release gc .PHONY: test-gc-clean test-gc-clean: @@ -408,10 +410,10 @@ test-build-addons-napi: all build-addons-napi .PHONY: test-all test-all: test-build test/gc/build/Release/binding.node ## Run everything in test/. - $(PYTHON) tools/test.py --mode=debug,release + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=debug,release test-all-valgrind: test-build - $(PYTHON) tools/test.py --mode=debug,release --valgrind + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=debug,release --valgrind CI_NATIVE_SUITES ?= addons addons-napi CI_JS_SUITES ?= default @@ -473,29 +475,29 @@ run-ci: build-ci $(MAKE) test-ci test-release: test-build - $(PYTHON) tools/test.py --mode=release + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release test-debug: test-build - $(PYTHON) tools/test.py --mode=debug + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=debug test-message: test-build - $(PYTHON) tools/test.py message + $(PYTHON) tools/test.py $(PARALLEL_ARGS) message test-simple: | cctest # Depends on 'all'. - $(PYTHON) tools/test.py parallel sequential + $(PYTHON) tools/test.py $(PARALLEL_ARGS) parallel sequential test-pummel: all - $(PYTHON) tools/test.py pummel + $(PYTHON) tools/test.py $(PARALLEL_ARGS) pummel test-internet: all - $(PYTHON) tools/test.py internet + $(PYTHON) tools/test.py $(PARALLEL_ARGS) internet test-node-inspect: $(NODE_EXE) USE_EMBEDDED_NODE_INSPECT=1 $(NODE) tools/test-npm-package \ --install deps/node-inspect test test-tick-processor: all - $(PYTHON) tools/test.py tick-processor + $(PYTHON) tools/test.py $(PARALLEL_ARGS) tick-processor .PHONY: test-hash-seed # Verifies the hash seed used by V8 for hashing is random. @@ -505,10 +507,10 @@ test-hash-seed: all .PHONY: test-doc test-doc: doc-only ## Builds, lints, and verifies the docs. $(MAKE) lint - $(PYTHON) tools/test.py $(CI_DOC) + $(PYTHON) tools/test.py $(PARALLEL_ARGS) $(CI_DOC) test-known-issues: all - $(PYTHON) tools/test.py known_issues + $(PYTHON) tools/test.py $(PARALLEL_ARGS) known_issues # Related CI job: node-test-npm test-npm: $(NODE_EXE) ## Run the npm test suite on deps/npm. @@ -519,7 +521,7 @@ test-npm-publish: $(NODE_EXE) .PHONY: test-addons-napi test-addons-napi: test-build-addons-napi - $(PYTHON) tools/test.py --mode=release addons-napi + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release addons-napi .PHONY: test-addons-napi-clean test-addons-napi-clean: @@ -528,7 +530,7 @@ test-addons-napi-clean: .PHONY: test-addons test-addons: test-build test-addons-napi - $(PYTHON) tools/test.py --mode=release addons + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release addons .PHONY: test-addons-clean test-addons-clean: @@ -539,19 +541,19 @@ test-addons-clean: test-timers: $(MAKE) --directory=tools faketime - $(PYTHON) tools/test.py --mode=release timers + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release timers test-timers-clean: $(MAKE) --directory=tools clean test-async-hooks: - $(PYTHON) tools/test.py --mode=release async-hooks + $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release async-hooks test-with-async-hooks: $(MAKE) build-addons $(MAKE) build-addons-napi $(MAKE) cctest - NODE_TEST_WITH_ASYNC_HOOKS=1 $(PYTHON) tools/test.py --mode=release -J \ + NODE_TEST_WITH_ASYNC_HOOKS=1 $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=release \ $(CI_JS_SUITES) \ $(CI_NATIVE_SUITES) From a342cd693cd8cb2c25dd5ff8a14e99f1043fa516 Mon Sep 17 00:00:00 2001 From: Santiago Gimeno Date: Thu, 12 Apr 2018 11:15:31 +0200 Subject: [PATCH 41/43] net: honor default values in Socket constructor Specifically `readable` and `writable` that default to `false`. PR-URL: https://github.com/nodejs/node/pull/19971 Fixes: https://github.com/libuv/libuv/issues/1794 Reviewed-By: Luigi Pinca Reviewed-By: Anatoli Papirovski --- lib/internal/child_process.js | 7 +++- lib/internal/wrap_js_stream.js | 2 + lib/net.js | 11 +++-- test/parallel/test-net-socket-constructor.js | 43 ++++++++++++++++++++ 4 files changed, 55 insertions(+), 8 deletions(-) create mode 100644 test/parallel/test-net-socket-constructor.js diff --git a/lib/internal/child_process.js b/lib/internal/child_process.js index 20ad9754ca7..8cca1c2f6b0 100644 --- a/lib/internal/child_process.js +++ b/lib/internal/child_process.js @@ -131,8 +131,11 @@ const handleConversion = { }, got: function(message, handle, emit) { - var socket = new net.Socket({ handle: handle }); - socket.readable = socket.writable = true; + var socket = new net.Socket({ + handle: handle, + readable: true, + writable: true + }); // if the socket was created by net.Server we will track the socket if (message.key) { diff --git a/lib/internal/wrap_js_stream.js b/lib/internal/wrap_js_stream.js index 9a7e9cd48ab..ec966028a02 100644 --- a/lib/internal/wrap_js_stream.js +++ b/lib/internal/wrap_js_stream.js @@ -72,6 +72,8 @@ class JSStreamWrap extends Socket { this.stream = stream; this[kCurrentWriteRequest] = null; this[kCurrentShutdownRequest] = null; + this.readable = stream.readable; + this.writable = stream.writable; // Start reading. this.read(0); diff --git a/lib/net.js b/lib/net.js index c367712686c..6c475619117 100644 --- a/lib/net.js +++ b/lib/net.js @@ -244,6 +244,8 @@ function Socket(options) { else options = util._extend({}, options); + options.readable = options.readable || false; + options.writable = options.writable || false; const allowHalfOpen = options.allowHalfOpen; // Prevent the "no-half-open enforcer" from being inherited from `Duplex`. @@ -283,9 +285,6 @@ function Socket(options) { value: 0, writable: true }); } - } else { - // these will be set once there is a connection - this.readable = this.writable = false; } // shut down the socket when we're finished with it. @@ -1537,10 +1536,10 @@ function onconnection(err, clientHandle) { var socket = new Socket({ handle: clientHandle, allowHalfOpen: self.allowHalfOpen, - pauseOnCreate: self.pauseOnConnect + pauseOnCreate: self.pauseOnConnect, + readable: true, + writable: true }); - socket.readable = socket.writable = true; - self._connections++; socket.server = self; diff --git a/test/parallel/test-net-socket-constructor.js b/test/parallel/test-net-socket-constructor.js new file mode 100644 index 00000000000..6758e286fb1 --- /dev/null +++ b/test/parallel/test-net-socket-constructor.js @@ -0,0 +1,43 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); + +function test(sock, readable, writable) { + let socket; + if (sock instanceof net.Socket) { + socket = sock; + } else { + socket = new net.Socket(sock); + socket.unref(); + } + + assert.strictEqual(socket.readable, readable); + assert.strictEqual(socket.writable, writable); +} + +test(undefined, false, false); + +const server = net.createServer(common.mustCall((socket) => { + test(socket, true, true); + test({ handle: socket._handle }, false, false); + test({ handle: socket._handle, readable: true, writable: true }, true, true); + if (socket._handle.fd >= 0) { + test(socket._handle.fd, false, false); + test({ fd: socket._handle.fd }, false, false); + test({ fd: socket._handle.fd, readable: true, writable: true }, true, true); + } + + server.close(); +})); + +server.listen(common.mustCall(() => { + const { port } = server.address(); + const socket = net.connect(port, common.mustCall(() => { + test(socket, true, true); + socket.end(); + })); + + test(socket, false, true); +})); From 0ddc06098d62c75f7e9f2e5fa393c37e6bc99a87 Mon Sep 17 00:00:00 2001 From: Ilya Sotov Date: Sat, 21 Apr 2018 13:41:10 +0300 Subject: [PATCH 42/43] test: improve http res write and end dont take array PR-URL: https://github.com/nodejs/node/pull/20199 Reviewed-By: Luigi Pinca Reviewed-By: Ruben Bridgewater Reviewed-By: Trivikram Kamat Reviewed-By: Rich Trott --- ...test-http-res-write-end-dont-take-array.js | 23 ++++++++++++++----- 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/test/parallel/test-http-res-write-end-dont-take-array.js b/test/parallel/test-http-res-write-end-dont-take-array.js index 94b105cb181..fbe71bd6fcb 100644 --- a/test/parallel/test-http-res-write-end-dont-take-array.js +++ b/test/parallel/test-http-res-write-end-dont-take-array.js @@ -35,15 +35,26 @@ server.once('request', common.mustCall((req, res) => { // write should accept buffer res.write(Buffer.from('asdf')); + const expectedError = { + code: 'ERR_INVALID_ARG_TYPE', + name: 'TypeError [ERR_INVALID_ARG_TYPE]', + }; + // write should not accept an Array - assert.throws(function() { - res.write(['array']); - }, TypeError, 'first argument must be a string or Buffer'); + assert.throws( + () => { + res.write(['array']); + }, + expectedError + ); // end should not accept an Array - assert.throws(function() { - res.end(['moo']); - }, TypeError, 'first argument must be a string or Buffer'); + assert.throws( + () => { + res.end(['moo']); + }, + expectedError + ); // end should accept string res.end('string'); From 4c6a47f7d7213c4f73a8390580024718adf1fbf6 Mon Sep 17 00:00:00 2001 From: Ujjwal Sharma Date: Sun, 22 Apr 2018 02:49:45 +0530 Subject: [PATCH 43/43] doc: add parameters for Http2Session:error event Add parameters for the callback for the Http2Session:error event inline with the pattern in the rest of the documentation. Refs: https://github.com/nodejs/help/issues/877#issuecomment-381253464 PR-URL: https://github.com/nodejs/node/pull/20206 Reviewed-By: Matteo Collina Reviewed-By: Vse Mozhet Byt Reviewed-By: Luigi Pinca Reviewed-By: Trivikram Kamat --- doc/api/http2.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/doc/api/http2.md b/doc/api/http2.md index a893cda97ef..f35e9b8b296 100644 --- a/doc/api/http2.md +++ b/doc/api/http2.md @@ -149,6 +149,8 @@ User code will typically not listen for this event directly. added: v8.4.0 --> +* `error` {Error} + The `'error'` event is emitted when an error occurs during the processing of an `Http2Session`.