From 240046ccc636d8b50b811f71b2f0d4a62b18e4e3 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Mon, 19 Oct 2015 14:06:51 -0600 Subject: [PATCH 01/51] doc: binary encoding is not deprecated When v8 implemented proper one-byte string support Node's internal "binary" encoding implementation was removed in favor of it. The result was that "binary" encoding effectively became "latin-1" encoding. Because of this and because one-byte strings are natively supported by v8 the buffer encoding is not deprecated and will not be removed. Ref: 83261e7 "deps: update v8 to 3.17.13" PR-URL: https://github.com/nodejs/node/pull/3441 Reviewed-By: Ben Noordhuis --- doc/api/buffer.markdown | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/doc/api/buffer.markdown b/doc/api/buffer.markdown index 22faef5563cb06..0f3a2ff2f72c4d 100644 --- a/doc/api/buffer.markdown +++ b/doc/api/buffer.markdown @@ -30,10 +30,9 @@ encoding method. Here are the different string encodings. * `'base64'` - Base64 string encoding. -* `'binary'` - A way of encoding raw binary data into strings by using only - the first 8 bits of each character. This encoding method is deprecated and - should be avoided in favor of `Buffer` objects where possible. This encoding - will be removed in future versions of Node.js. +* `'binary'` - A way of encoding the buffer into a one-byte (i.e. `latin-1`) + encoded string. The string `'latin-1'` is not supported. Instead simply pass + `'binary'` to use `'latin-1'` encoding. * `'hex'` - Encode each byte as two hexadecimal characters. From f6ea99f67b8c8a02695545d8b783204d8311c591 Mon Sep 17 00:00:00 2001 From: Yuval Brik Date: Sun, 30 Aug 2015 00:27:14 +0300 Subject: [PATCH 02/51] tls: TLSSocket options default isServer false Upon creating a TLSSocket object, set the default isServer option to false Updated tls docs and added test-tls-socket-default-options PR-URL: https://github.com/nodejs/node/pull/2614 Reviewed-By: Fedor Indutny --- doc/api/tls.markdown | 7 ++- lib/_tls_wrap.js | 7 ++- .../test-tls-socket-default-options.js | 56 +++++++++++++++++++ 3 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-tls-socket-default-options.js diff --git a/doc/api/tls.markdown b/doc/api/tls.markdown index eb98328fcebd66..e52ab6209c3910 100644 --- a/doc/api/tls.markdown +++ b/doc/api/tls.markdown @@ -449,18 +449,19 @@ Or Wrapper for instance of [net.Socket][], replaces internal socket read/write routines to perform transparent encryption/decryption of incoming/outgoing data. -## new tls.TLSSocket(socket, options) +## new tls.TLSSocket(socket[, options]) Construct a new TLSSocket object from existing TCP socket. `socket` is an instance of [net.Socket][] -`options` is an object that might contain following properties: +`options` is an optional object that might contain following properties: - `secureContext`: An optional TLS context object from `tls.createSecureContext( ... )` - - `isServer`: If true - TLS socket will be instantiated in server-mode + - `isServer`: If `true` - TLS socket will be instantiated in server-mode. + Default: `false` - `server`: An optional [net.Server][] instance diff --git a/lib/_tls_wrap.js b/lib/_tls_wrap.js index ae88bf6c18ebf6..661f695b6f1301 100644 --- a/lib/_tls_wrap.js +++ b/lib/_tls_wrap.js @@ -228,7 +228,10 @@ function initRead(tls, wrapped) { */ function TLSSocket(socket, options) { - this._tlsOptions = options; + if (options === undefined) + this._tlsOptions = {}; + else + this._tlsOptions = options; this._secureEstablished = false; this._securePending = false; this._newSessionPending = false; @@ -321,7 +324,7 @@ TLSSocket.prototype._wrapHandle = function(wrap) { tls.createSecureContext(); res = tls_wrap.wrap(handle._externalStream, context.context, - options.isServer); + !!options.isServer); res._parent = handle; res._parentWrap = wrap; res._secureContext = context; diff --git a/test/parallel/test-tls-socket-default-options.js b/test/parallel/test-tls-socket-default-options.js new file mode 100644 index 00000000000000..3af03a0ba9269a --- /dev/null +++ b/test/parallel/test-tls-socket-default-options.js @@ -0,0 +1,56 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); + +if (!common.hasCrypto) { + console.log('1..0 # Skipped: missing crypto'); + return; +} +const tls = require('tls'); + +const fs = require('fs'); +const net = require('net'); + +const sent = 'hello world'; + +const serverOptions = { + isServer: true, + key: fs.readFileSync(common.fixturesDir + '/keys/agent1-key.pem'), + cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem') +}; + +function testSocketOptions(socket, socketOptions) { + let received = ''; + const server = tls.createServer(serverOptions, function(s) { + s.on('data', function(chunk) { + received += chunk; + }); + + s.on('end', function() { + server.close(); + s.destroy(); + assert.equal(received, sent); + setImmediate(runTests); + }); + }).listen(common.PORT, function() { + let c = new tls.TLSSocket(socket, socketOptions); + c.connect(common.PORT, function() { + c.end(sent); + }); + }); + +} + +const testArgs = [ + [], + [undefined, {}] +]; + +let n = 0; +function runTests() { + if (n++ < testArgs.length) { + testSocketOptions.apply(null, testArgs[n]); + } +} + +runTests(); From 172cccfe0ca9f625b00f10f99069f360fb9824b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A1bio=20Santos?= Date: Wed, 7 Oct 2015 23:27:06 +0100 Subject: [PATCH 03/51] stream: fix signature of _write() in a comment This comment was a bit misleading, since it was missing the `encoding` argument. PR-URL: https://github.com/nodejs/node/pull/3248 Reviewed-By: Brendan Ashworth --- lib/_stream_writable.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index 19c0c8a006f62a..9c7e2630161cd9 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -1,5 +1,5 @@ // A bit simpler than readable streams. -// Implement an async ._write(chunk, cb), and it'll handle all +// Implement an async ._write(chunk, encoding, cb), and it'll handle all // the drain event emission and buffering. 'use strict'; From 282618e237e60f6852e3e634547d8ed15f5ef505 Mon Sep 17 00:00:00 2001 From: Kyle Smith Date: Wed, 7 Oct 2015 16:03:21 -0700 Subject: [PATCH 04/51] doc: clarify the use of `option.detached` This paragraph conveys that detached child processes do not stay running in the background in general. Instead clarify that this refers to the parent process exiting before the detached child process is complete. Reviewed-By: Trevor Norris Reviewed-By: Colin Ihrig Reviewed-By: James M Snell PR-URL: https://github.com/nodejs/node/pull/3250 --- doc/api/child_process.markdown | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/doc/api/child_process.markdown b/doc/api/child_process.markdown index ebbc8f4df2c6c9..bddf90ff85b625 100644 --- a/doc/api/child_process.markdown +++ b/doc/api/child_process.markdown @@ -535,9 +535,10 @@ file: child.unref(); When using the `detached` option to start a long-running process, the process -will not stay running in the background unless it is provided with a `stdio` -configuration that is not connected to the parent. If the parent's `stdio` is -inherited, the child will remain attached to the controlling terminal. +will not stay running in the background after the parent exits unless it is +provided with a `stdio` configuration that is not connected to the parent. +If the parent's `stdio` is inherited, the child will remain attached to the +controlling terminal. See also: [`child_process.exec()`](#child_process_child_process_exec_command_options_callback) and [`child_process.fork()`](#child_process_child_process_fork_modulepath_args_options) From 4afcf5732bb57acb4658f061394b41473c706c15 Mon Sep 17 00:00:00 2001 From: Martii Date: Wed, 7 Oct 2015 21:14:16 -0600 Subject: [PATCH 05/51] doc: clarify API buffer.concat * Add a simple example for buffer.concat * Change grammar slightly. Fixes: #3219 Reviewed-By: James M Snell Reviewed-By: Trevor Norris PR-URL: https://github.com/nodejs/node/pull/3255 --- doc/api/buffer.markdown | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/doc/api/buffer.markdown b/doc/api/buffer.markdown index 0f3a2ff2f72c4d..c678c4a2da8b13 100644 --- a/doc/api/buffer.markdown +++ b/doc/api/buffer.markdown @@ -126,7 +126,7 @@ Example: ### Class Method: Buffer.concat(list[, totalLength]) * `list` {Array} List of Buffer objects to concat -* `totalLength` {Number} Total length of the buffers when concatenated +* `totalLength` {Number} Total length of the buffers in the list when concatenated Returns a buffer which is the result of concatenating all the buffers in the list together. @@ -138,6 +138,32 @@ If totalLength is not provided, it is read from the buffers in the list. However, this adds an additional loop to the function, so it is faster to provide the length explicitly. +Example: build a single buffer from a list of three buffers: + + var buf1 = new Buffer(10); + var buf2 = new Buffer(14); + var buf3 = new Buffer(18); + + buf1.fill(0); + buf2.fill(0); + buf3.fill(0); + + var buffers = [buf1, buf2, buf3]; + + var totalLength = 0; + for (var i = 0; i < buffers.length; i++) { + totalLength += buffers[i].length; + } + + console.log(totalLength); + var bufA = Buffer.concat(buffers, totalLength); + console.log(bufA); + console.log(bufA.length); + + // 42 + // + // 42 + ### Class Method: Buffer.compare(buf1, buf2) * `buf1` {Buffer} From 5db1f070fed005b6d3ac9b99c0b2c643c5d009aa Mon Sep 17 00:00:00 2001 From: Calvin Metcalf Date: Fri, 9 Oct 2015 14:50:11 -0400 Subject: [PATCH 06/51] stream: avoid unnecessary concat of a single buffer. Avoids doing a buffer.concat on the internal buffer when that array has only a single thing in it. Reviewed-By: Chris Dickinson Reviewed-By: James M Snell PR-URL: https://github.com/nodejs/node/pull/3300 --- lib/_stream_readable.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/_stream_readable.js b/lib/_stream_readable.js index 5a375321615a7f..ab47830767c525 100644 --- a/lib/_stream_readable.js +++ b/lib/_stream_readable.js @@ -836,6 +836,8 @@ function fromList(n, state) { // read it all, truncate the array. if (stringMode) ret = list.join(''); + else if (list.length === 1) + ret = list[0]; else ret = Buffer.concat(list, length); list.length = 0; From 87f8ccfe35a6c76ce5308b4a9b71edc6a63e1571 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 13 Oct 2015 02:16:39 -0400 Subject: [PATCH 07/51] http: fix stalled pipeline bug This is a two-part fix: - Fix pending data notification in `OutgoingMessage` to notify server about flushed data too - Fix pause/resume behavior for the consumed socket. `resume` event is emitted on a next tick, and `socket._paused` can already be `true` at this time. Pause the socket again to avoid PAUSED error on parser. Fix: https://github.com/nodejs/node/issues/3332 PR-URL: https://github.com/nodejs/node/pull/3342 Reviewed-By: James M Snell Reviewed-By: Trevor Norris --- lib/_http_common.js | 4 ++ lib/_http_outgoing.js | 66 +++++++++---------- lib/_http_server.js | 39 +++++++++-- src/node_http_parser.cc | 17 +++-- test/parallel/test-http-pipeline-regr-3332.js | 41 ++++++++++++ 5 files changed, 120 insertions(+), 47 deletions(-) create mode 100644 test/parallel/test-http-pipeline-regr-3332.js diff --git a/lib/_http_common.js b/lib/_http_common.js index 757032929444b1..4b57460981617f 100644 --- a/lib/_http_common.js +++ b/lib/_http_common.js @@ -140,6 +140,7 @@ var parsers = new FreeList('parsers', 1000, function() { parser._headers = []; parser._url = ''; + parser._consumed = false; // Only called in the slow case where slow means // that the request headers were either fragmented @@ -167,6 +168,9 @@ function freeParser(parser, req, socket) { if (parser) { parser._headers = []; parser.onIncoming = null; + if (parser._consumed) + parser.unconsume(); + parser._consumed = false; if (parser.socket) parser.socket.parser = null; parser.socket = null; diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js index dd78ddf645f1e5..d130246b84dc58 100644 --- a/lib/_http_outgoing.js +++ b/lib/_http_outgoing.js @@ -131,7 +131,7 @@ OutgoingMessage.prototype._send = function(data, encoding, callback) { this.outputEncodings.unshift('binary'); this.outputCallbacks.unshift(null); this.outputSize += this._header.length; - if (this._onPendingData !== null) + if (typeof this._onPendingData === 'function') this._onPendingData(this._header.length); } this._headerSent = true; @@ -154,22 +154,7 @@ OutgoingMessage.prototype._writeRaw = function(data, encoding, callback) { // There might be pending data in the this.output buffer. var outputLength = this.output.length; if (outputLength > 0) { - var output = this.output; - var outputEncodings = this.outputEncodings; - var outputCallbacks = this.outputCallbacks; - connection.cork(); - for (var i = 0; i < outputLength; i++) { - connection.write(output[i], outputEncodings[i], - outputCallbacks[i]); - } - connection.uncork(); - - this.output = []; - this.outputEncodings = []; - this.outputCallbacks = []; - if (this._onPendingData !== null) - this._onPendingData(-this.outputSize); - this.outputSize = 0; + this._flushOutput(connection); } else if (data.length === 0) { if (typeof callback === 'function') process.nextTick(callback); @@ -194,7 +179,7 @@ OutgoingMessage.prototype._buffer = function(data, encoding, callback) { this.outputEncodings.push(encoding); this.outputCallbacks.push(callback); this.outputSize += data.length; - if (this._onPendingData !== null) + if (typeof this._onPendingData === 'function') this._onPendingData(data.length); return false; }; @@ -630,26 +615,11 @@ OutgoingMessage.prototype._finish = function() { // to attempt to flush any pending messages out to the socket. OutgoingMessage.prototype._flush = function() { var socket = this.socket; - var outputLength, ret; + var ret; if (socket && socket.writable) { // There might be remaining data in this.output; write it out - outputLength = this.output.length; - if (outputLength > 0) { - var output = this.output; - var outputEncodings = this.outputEncodings; - var outputCallbacks = this.outputCallbacks; - socket.cork(); - for (var i = 0; i < outputLength; i++) { - ret = socket.write(output[i], outputEncodings[i], - outputCallbacks[i]); - } - socket.uncork(); - - this.output = []; - this.outputEncodings = []; - this.outputCallbacks = []; - } + ret = this._flushOutput(socket); if (this.finished) { // This is a queue to the server or client to bring in the next this. @@ -661,6 +631,32 @@ OutgoingMessage.prototype._flush = function() { } }; +OutgoingMessage.prototype._flushOutput = function _flushOutput(socket) { + var ret; + var outputLength = this.output.length; + if (outputLength <= 0) + return ret; + + var output = this.output; + var outputEncodings = this.outputEncodings; + var outputCallbacks = this.outputCallbacks; + socket.cork(); + for (var i = 0; i < outputLength; i++) { + ret = socket.write(output[i], outputEncodings[i], + outputCallbacks[i]); + } + socket.uncork(); + + this.output = []; + this.outputEncodings = []; + this.outputCallbacks = []; + if (typeof this._onPendingData === 'function') + this._onPendingData(-this.outputSize); + this.outputSize = 0; + + return ret; +}; + OutgoingMessage.prototype.flushHeaders = function() { if (!this._header) { diff --git a/lib/_http_server.js b/lib/_http_server.js index c11d36912e27e7..dc7276d0ae729d 100644 --- a/lib/_http_server.js +++ b/lib/_http_server.js @@ -343,8 +343,10 @@ function connectionListener(socket) { socket.on = socketOnWrap; var external = socket._handle._externalStream; - if (external) + if (external) { + parser._consumed = true; parser.consume(external); + } external = null; parser[kOnExecute] = onParserExecute; @@ -382,7 +384,7 @@ function connectionListener(socket) { socket.removeListener('data', socketOnData); socket.removeListener('end', socketOnEnd); socket.removeListener('close', serverSocketCloseListener); - parser.unconsume(socket._handle._externalStream); + unconsume(parser, socket); parser.finish(); freeParser(parser, req, null); parser = null; @@ -530,13 +532,38 @@ function connectionListener(socket) { exports._connectionListener = connectionListener; function onSocketResume() { - if (this._handle) + // It may seem that the socket is resumed, but this is an enemy's trick to + // deceive us! `resume` is emitted asynchronously, and may be called from + // `incoming.readStart()`. Stop the socket again here, just to preserve the + // state. + // + // We don't care about stream semantics for the consumed socket anyway. + if (this._paused) { + this.pause(); + return; + } + + if (this._handle && !this._handle.reading) { + this._handle.reading = true; this._handle.readStart(); + } } function onSocketPause() { - if (this._handle) + if (this._handle && this._handle.reading) { + this._handle.reading = false; this._handle.readStop(); + } +} + +function unconsume(parser, socket) { + if (socket._handle) { + if (parser._consumed) + parser.unconsume(socket._handle._externalStream); + parser._consumed = false; + socket.removeListener('pause', onSocketPause); + socket.removeListener('resume', onSocketResume); + } } function socketOnWrap(ev, fn) { @@ -546,8 +573,8 @@ function socketOnWrap(ev, fn) { return res; } - if (this._handle && (ev === 'data' || ev === 'readable')) - this.parser.unconsume(this._handle._externalStream); + if (ev === 'data' || ev === 'readable') + unconsume(this.parser, this); return res; } diff --git a/src/node_http_parser.cc b/src/node_http_parser.cc index 5f831ec50696aa..ff3dfb26e529af 100644 --- a/src/node_http_parser.cc +++ b/src/node_http_parser.cc @@ -484,13 +484,18 @@ class Parser : public BaseObject { if (parser->prev_alloc_cb_.is_empty()) return; - CHECK(args[0]->IsExternal()); - Local stream_obj = args[0].As(); - StreamBase* stream = static_cast(stream_obj->Value()); - CHECK_NE(stream, nullptr); + // Restore stream's callbacks + if (args.Length() == 1 && args[0]->IsExternal()) { + Local stream_obj = args[0].As(); + StreamBase* stream = static_cast(stream_obj->Value()); + CHECK_NE(stream, nullptr); + + stream->set_alloc_cb(parser->prev_alloc_cb_); + stream->set_read_cb(parser->prev_read_cb_); + } - stream->set_alloc_cb(parser->prev_alloc_cb_); - stream->set_read_cb(parser->prev_read_cb_); + parser->prev_alloc_cb_.clear(); + parser->prev_read_cb_.clear(); } diff --git a/test/parallel/test-http-pipeline-regr-3332.js b/test/parallel/test-http-pipeline-regr-3332.js new file mode 100644 index 00000000000000..061e202d975f32 --- /dev/null +++ b/test/parallel/test-http-pipeline-regr-3332.js @@ -0,0 +1,41 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); +const net = require('net'); + +const big = new Buffer(16 * 1024); +big.fill('A'); + +const COUNT = 1e4; + +var received = 0; + +var client; +const server = http.createServer(function(req, res) { + res.end(big, function() { + if (++received === COUNT) { + server.close(); + client.end(); + } + }); +}).listen(common.PORT, function() { + var req = new Array(COUNT + 1).join('GET / HTTP/1.1\r\n\r\n'); + client = net.connect(common.PORT, function() { + client.write(req); + }); + + // Just let the test terminate instead of hanging + client.on('close', function() { + if (received !== COUNT) + server.close(); + }); + client.resume(); +}); + +process.on('exit', function() { + // The server should pause connection on pipeline flood, but it shoul still + // resume it and finish processing the requests, when its output queue will + // be empty again. + assert.equal(received, COUNT); +}); From 4023c7dc342cb59a5281090002416003c4a1066e Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 12 Oct 2015 13:58:00 -0700 Subject: [PATCH 08/51] doc: add information about Assert behavior and maintenance Assert is now locked. Userland alternatives should be used. Assert is for testing Node.js itself. Document potentially surprising use of enumerable properties only in deep equality assertions. Ref: https://github.com/nodejs/node/pull/3124 Ref: https://github.com/nodejs/node/issues/3122 PR-URL: https://github.com/nodejs/node/pull/3330 Reviewed-By: Colin Ihrig Reviewed-By: Fedor Indutny Reviewed-By: Rod Vagg --- doc/api/assert.markdown | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/doc/api/assert.markdown b/doc/api/assert.markdown index fd1130eebe72a4..7bf6ebb40fd30a 100644 --- a/doc/api/assert.markdown +++ b/doc/api/assert.markdown @@ -1,9 +1,10 @@ # Assert - Stability: 2 - Stable + Stability: 3 - Locked -This module is used for writing assertion tests. You can access it with -`require('assert')`. +This module is used so that Node.js can test itself. It can be accessed with +`require('assert')`. However, it is recommended that a userland assertion +library be used instead. ## assert.fail(actual, expected, message, operator) @@ -26,8 +27,17 @@ Tests shallow, coercive inequality with the not equal comparison operator ## assert.deepEqual(actual, expected[, message]) -Tests for deep equality. Primitive values are compared with the equal comparison -operator ( `==` ). Doesn't take object prototypes into account. +Tests for deep equality. Primitive values are compared with the equal +comparison operator ( `==` ). + +This only considers enumerable properties. It does not test object prototypes, +attached symbols, or non-enumerable properties. This can lead to some +potentially surprising results. For example, this does not throw an +`AssertionError` because the properties on the `Error` object are +non-enumerable: + + // WARNING: This does not throw an AssertionError! + assert.deepEqual(Error('a'), Error('b')); ## assert.notDeepEqual(actual, expected[, message]) From 925953d1bcda0dd0003cb922a63687094227b874 Mon Sep 17 00:00:00 2001 From: calebboyd Date: Mon, 19 Oct 2015 12:38:55 -0500 Subject: [PATCH 09/51] doc: show keylen in pbkdf2 as a byte length Ensure that keylen for pbkdf2 is documented as a length of bytes and not bits. PR-URL: https://github.com/nodejs/node/pull/3334 Reviewed-By: Fedor Indutny Reviewed-By: Jeremiah Senkpiel --- doc/api/crypto.markdown | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/api/crypto.markdown b/doc/api/crypto.markdown index 0379179de3593c..04d7af6a168334 100644 --- a/doc/api/crypto.markdown +++ b/doc/api/crypto.markdown @@ -621,13 +621,13 @@ Example (obtaining a shared secret): ## crypto.pbkdf2(password, salt, iterations, keylen[, digest], callback) Asynchronous PBKDF2 function. Applies the selected HMAC digest function -(default: SHA1) to derive a key of the requested length from the password, +(default: SHA1) to derive a key of the requested byte length from the password, salt and number of iterations. The callback gets two arguments: `(err, derivedKey)`. Example: - crypto.pbkdf2('secret', 'salt', 4096, 512, 'sha256', function(err, key) { + crypto.pbkdf2('secret', 'salt', 4096, 64, 'sha256', function(err, key) { if (err) throw err; console.log(key.toString('hex')); // 'c5e478d...1469e50' From e43c611be5c2af4538c6f54f9edb58a1a879cfb1 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 12 Oct 2015 20:48:41 -0700 Subject: [PATCH 10/51] test: add test-child-process-emfile fail message When the test fails (as it does frequently on FreeBSD unfortunately) provide a non-cryptic error message that also provides a line number. PR-URL: https://github.com/nodejs/node/pull/3335 Reviewed-By: Ben Noordhuis --- test/sequential/test-child-process-emfile.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/sequential/test-child-process-emfile.js b/test/sequential/test-child-process-emfile.js index c4c467c3392482..ae7964d9e7a7d6 100644 --- a/test/sequential/test-child-process-emfile.js +++ b/test/sequential/test-child-process-emfile.js @@ -27,8 +27,10 @@ proc.on('error', common.mustCall(function(err) { assert(err.code === 'EMFILE' || err.code === 'ENFILE'); })); -// 'exit' should not be emitted, the process was never spawned. -proc.on('exit', assert.fail); +proc.on('exit', function() { + const msg = '"exit" should not be emitted (the process never spawned!)'; + assert.fail(null, null, msg); +}); // close one fd for LSan if (openFds.length >= 1) { From 1888f84204a2f02a7db9e48b5e9d87318c50b12e Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Tue, 13 Oct 2015 21:39:16 -0700 Subject: [PATCH 11/51] lib: avoid REPL exit on completion error If a tab completion is attempted on an undefined reference inside of a function, the REPL was exiting without reporting an error or anything else. This change results in the REPL reporting the ReferenceError and continuing. Fixes: https://github.com/nodejs/node/issues/3346 PR-URL: https://github.com/nodejs/node/pull/3358 Reviewed-By: James M Snell --- lib/repl.js | 15 ++++--- test/parallel/test-repl-tab-complete-crash.js | 41 +++++++++++++++++++ 2 files changed, 51 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-repl-tab-complete-crash.js diff --git a/lib/repl.js b/lib/repl.js index 81afd27a4dbd00..46288db5639c59 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -32,6 +32,8 @@ const Console = require('console').Console; const domain = require('domain'); const debug = util.debuglog('repl'); +const replMap = new WeakMap(); + try { // hack for require.resolve("./relative") to work properly. module.filename = path.resolve('repl'); @@ -189,11 +191,12 @@ function REPLServer(prompt, self._domain.on('error', function(e) { debug('domain error'); - self.outputStream.write((e.stack || e) + '\n'); - self._currentStringLiteral = null; - self.bufferedCommand = ''; - self.lines.level = []; - self.displayPrompt(); + const top = replMap.get(self); + top.outputStream.write((e.stack || e) + '\n'); + top._currentStringLiteral = null; + top.bufferedCommand = ''; + top.lines.level = []; + top.displayPrompt(); }); if (!input && !output) { @@ -472,6 +475,7 @@ exports.start = function(prompt, ignoreUndefined, replMode); if (!exports.repl) exports.repl = repl; + replMap.set(repl, repl); return repl; }; @@ -601,6 +605,7 @@ REPLServer.prototype.complete = function(line, callback) { // all this is only profitable if the nested REPL // does not have a bufferedCommand if (!magic.bufferedCommand) { + replMap.set(magic, replMap.get(this)); return magic.complete(line, callback); } } diff --git a/test/parallel/test-repl-tab-complete-crash.js b/test/parallel/test-repl-tab-complete-crash.js new file mode 100644 index 00000000000000..85ab0577eb8601 --- /dev/null +++ b/test/parallel/test-repl-tab-complete-crash.js @@ -0,0 +1,41 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const util = require('util'); +const repl = require('repl'); + +var referenceErrorCount = 0; + +// A stream to push an array into a REPL +function ArrayStream() { + this.run = function(data) { + const self = this; + data.forEach(function(line) { + self.emit('data', line + '\n'); + }); + }; +} +util.inherits(ArrayStream, require('stream').Stream); +ArrayStream.prototype.readable = true; +ArrayStream.prototype.writable = true; +ArrayStream.prototype.resume = function() {}; +ArrayStream.prototype.write = function(msg) { + if (msg.startsWith('ReferenceError: ')) { + referenceErrorCount++; + } +}; + +const putIn = new ArrayStream(); +const testMe = repl.start('', putIn); + +// https://github.com/nodejs/node/issues/3346 +// Tab-completion for an undefined variable inside a function should report a +// ReferenceError. +putIn.run(['.clear']); +putIn.run(['function () {']); +testMe.complete('arguments.'); + +process.on('exit', function() { + assert.strictEqual(referenceErrorCount, 1); +}); From 31a9ecf504f60b2fcde0809a9606e623e204dab3 Mon Sep 17 00:00:00 2001 From: Timothy Gu Date: Tue, 13 Oct 2015 16:20:21 -0700 Subject: [PATCH 12/51] doc: fix typo in changelog PR-URL: https://github.com/nodejs/node/pull/3353 Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott Reviewed-By: Rod Vagg --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a9f1f4ea0a94d4..b2c989759d9b83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,7 @@ * [[`b3cbd13340`](https://github.com/nodejs/node/commit/b3cbd13340)] - **buffer**: fix assertion error in WeakCallback (Fedor Indutny) [#3329](https://github.com/nodejs/node/pull/3329) * [[`102cb7288c`](https://github.com/nodejs/node/commit/102cb7288c)] - **doc**: label v4.2.0 as LTS in changelog heading (Rod Vagg) [#3343](https://github.com/nodejs/node/pull/3343) -* [[`c245a199a7`](https://github.com/nodejs/node/commit/c245a199a7)] - **lib**: fix undefined timeout regression (Ryan Graham) [#3331](https://github.com/nodejs/node/pull/3331 +* [[`c245a199a7`](https://github.com/nodejs/node/commit/c245a199a7)] - **lib**: fix undefined timeout regression (Ryan Graham) [#3331](https://github.com/nodejs/node/pull/3331) ## 2015-10-07, Version 4.2.0 'Argon' (LTS), @jasnell From 41a4258f575d0321aa6e04667d8eb025a9fe7ca7 Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Mon, 12 Oct 2015 21:26:50 -0700 Subject: [PATCH 13/51] test: replace util with backtick strings Now that we have backticks we no longer need to use util.format to template strings! This commit was inspired by #3324, and it replaces instances of util.format with backtick strings in a number of tests Reviewed-By: Rich Trott Reviewed-By: Brian White Reviewed-By: James M Snell PR-URL: https://github.com/nodejs/node/pull/3359 --- test/parallel/test-child-process-spawnsync-input.js | 3 +-- test/parallel/test-child-process-spawnsync.js | 10 +++++----- test/parallel/test-repl-setprompt.js | 11 +++++------ test/sequential/test-child-process-execsync.js | 10 +++++----- 4 files changed, 16 insertions(+), 18 deletions(-) diff --git a/test/parallel/test-child-process-spawnsync-input.js b/test/parallel/test-child-process-spawnsync-input.js index 4118b74dc35f6c..f043d0c5e355f1 100644 --- a/test/parallel/test-child-process-spawnsync-input.js +++ b/test/parallel/test-child-process-spawnsync-input.js @@ -2,7 +2,6 @@ var common = require('../common'); var assert = require('assert'); var os = require('os'); -var util = require('util'); var spawnSync = require('child_process').spawnSync; @@ -15,7 +14,7 @@ var msgErrBuf = new Buffer(msgErr + '\n'); var args = [ '-e', - util.format('console.log("%s"); console.error("%s");', msgOut, msgErr) + `console.log("${msgOut}"); console.error("${msgErr}");` ]; var ret; diff --git a/test/parallel/test-child-process-spawnsync.js b/test/parallel/test-child-process-spawnsync.js index 5cb4ec20db3588..53e72c83ca7373 100644 --- a/test/parallel/test-child-process-spawnsync.js +++ b/test/parallel/test-child-process-spawnsync.js @@ -1,16 +1,16 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); +const common = require('../common'); +const assert = require('assert'); -var spawnSync = require('child_process').spawnSync; +const spawnSync = require('child_process').spawnSync; // Echo does different things on Windows and Unix, but in both cases, it does // more-or-less nothing if there are no parameters -var ret = spawnSync('sleep', ['0']); +const ret = spawnSync('sleep', ['0']); assert.strictEqual(ret.status, 0, 'exit status should be zero'); // Error test when command does not exist -var ret_err = spawnSync('command_does_not_exist', ['bar']).error; +const ret_err = spawnSync('command_does_not_exist', ['bar']).error; assert.strictEqual(ret_err.code, 'ENOENT'); assert.strictEqual(ret_err.errno, 'ENOENT'); diff --git a/test/parallel/test-repl-setprompt.js b/test/parallel/test-repl-setprompt.js index b89dd6c928ab0a..f2c65583b3656b 100644 --- a/test/parallel/test-repl-setprompt.js +++ b/test/parallel/test-repl-setprompt.js @@ -1,9 +1,8 @@ 'use strict'; -var common = require('../common'), - assert = require('assert'), - spawn = require('child_process').spawn, - os = require('os'), - util = require('util'); +const common = require('../common'); +const assert = require('assert'); +const spawn = require('child_process').spawn; +const os = require('os'); var args = [ '-e', @@ -19,7 +18,7 @@ child.stdout.setEncoding('utf8'); var data = ''; child.stdout.on('data', function(d) { data += d; }); -child.stdin.end(util.format("e.setPrompt('%s');%s", p, os.EOL)); +child.stdin.end(`e.setPrompt("${p}");${os.EOL}`); child.on('close', function(code, signal) { assert.strictEqual(code, 0); diff --git a/test/sequential/test-child-process-execsync.js b/test/sequential/test-child-process-execsync.js index 07b6ba4c341271..0bc4e02c65edb8 100644 --- a/test/sequential/test-child-process-execsync.js +++ b/test/sequential/test-child-process-execsync.js @@ -1,7 +1,6 @@ 'use strict'; var common = require('../common'); var assert = require('assert'); -var util = require('util'); var os = require('os'); var execSync = require('child_process').execSync; @@ -13,10 +12,10 @@ var SLEEP = 2000; var start = Date.now(); var err; var caught = false; + try { - var cmd = util.format('"%s" -e "setTimeout(function(){}, %d);"', - process.execPath, SLEEP); + var cmd = `"${process.execPath}" -e "setTimeout(function(){}, ${SLEEP});"`; var ret = execSync(cmd, {timeout: TIMER}); } catch (e) { caught = true; @@ -38,7 +37,8 @@ var msg = 'foobar'; var msgBuf = new Buffer(msg + '\n'); // console.log ends every line with just '\n', even on Windows. -cmd = util.format('"%s" -e "console.log(\'%s\');"', process.execPath, msg); + +cmd = `"${process.execPath}" -e "console.log(\'${msg}\');"`; var ret = execSync(cmd); @@ -51,7 +51,7 @@ assert.strictEqual(ret, msg + '\n', 'execSync encoding result should match'); var args = [ '-e', - util.format('console.log("%s");', msg) + `console.log("${msg}");` ]; ret = execFileSync(process.execPath, args); From e223fc7499f57a76fe0fc5573a597b40bc459931 Mon Sep 17 00:00:00 2001 From: Roman Reiss Date: Wed, 14 Oct 2015 21:45:12 +0200 Subject: [PATCH 14/51] doc: fix indent in tls resumption example Markdown requires 4-space indent to correctly format code blocks. This fixes the example so it's correctly presented as code. PR-URL: https://github.com/nodejs/node/pull/3372 Reviewed-By: Sakthipriyan Vairamani --- doc/api/tls.markdown | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/doc/api/tls.markdown b/doc/api/tls.markdown index e52ab6209c3910..724c537e76e581 100644 --- a/doc/api/tls.markdown +++ b/doc/api/tls.markdown @@ -606,14 +606,14 @@ established after addition of event listener. Here's an example for using TLS session resumption: - var tlsSessionStore = {}; - server.on('newSession', function(id, data, cb) { - tlsSessionStore[id.toString('hex')] = data; - cb(); - }); - server.on('resumeSession', function(id, cb) { - cb(null, tlsSessionStore[id.toString('hex')] || null); - }); + var tlsSessionStore = {}; + server.on('newSession', function(id, data, cb) { + tlsSessionStore[id.toString('hex')] = data; + cb(); + }); + server.on('resumeSession', function(id, cb) { + cb(null, tlsSessionStore[id.toString('hex')] || null); + }); ### Event: 'OCSPRequest' From d540666bfed3570228b4a5b16e74bd1c9833846d Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Wed, 14 Oct 2015 21:22:55 -0700 Subject: [PATCH 15/51] test: apply correct assert.fail() arguments The assert.fail function signature has the message as the third argument but, understandably, it is often assumed that it is the first argument (or at least the first argument if no other arguments are passed). This corrects the assert.fail() invocations in the Node.js tests. Before: assert.fail('message'); // result: AssertionError: 'message' undefined undefined After: assert.fail(null, null, 'message'); // result: AssertionError: message PR-URL: https://github.com/nodejs/node/pull/3378 Reviewed-By: Colin Ihrig --- test/internet/test-dgram-send-cb-quelches-error.js | 2 +- test/parallel/test-fs-write-stream.js | 2 +- test/parallel/test-http-header-response-splitting.js | 2 +- test/parallel/test-http-localaddress-bind-error.js | 2 +- test/parallel/test-https-localaddress-bind-error.js | 2 +- test/parallel/test-net-connect-paused-connection.js | 2 +- test/parallel/test-net-write-slow.js | 2 +- test/parallel/test-path-parse-format.js | 2 +- test/parallel/test-repl-reset-event.js | 2 +- test/parallel/test-repl.js | 2 +- test/parallel/test-spawn-cmd-named-pipe.js | 2 +- test/parallel/test-stream2-base64-single-char-read-end.js | 2 +- test/parallel/test-tick-processor.js | 2 +- 13 files changed, 13 insertions(+), 13 deletions(-) diff --git a/test/internet/test-dgram-send-cb-quelches-error.js b/test/internet/test-dgram-send-cb-quelches-error.js index e89306b2c29aa2..dbbd8fc9fd7f58 100644 --- a/test/internet/test-dgram-send-cb-quelches-error.js +++ b/test/internet/test-dgram-send-cb-quelches-error.js @@ -28,7 +28,7 @@ function callbackOnly(err) { } function onEvent(err) { - assert.fail('Error should not be emitted if there is callback'); + assert.fail(null, null, 'Error should not be emitted if there is callback'); } function onError(err) { diff --git a/test/parallel/test-fs-write-stream.js b/test/parallel/test-fs-write-stream.js index 3142e896325f6a..af3ae17ec7e7e1 100644 --- a/test/parallel/test-fs-write-stream.js +++ b/test/parallel/test-fs-write-stream.js @@ -24,7 +24,7 @@ common.refreshTmpDir(); var stream = fs.createWriteStream(file); stream.on('drain', function() { - assert.fail('\'drain\' event must not be emitted before ' + + assert.fail(null, null, '\'drain\' event must not be emitted before ' + 'stream.write() has been called at least once.'); }); stream.destroy(); diff --git a/test/parallel/test-http-header-response-splitting.js b/test/parallel/test-http-header-response-splitting.js index ad8cc9c5ba8e8f..437793a524f22f 100644 --- a/test/parallel/test-http-header-response-splitting.js +++ b/test/parallel/test-http-header-response-splitting.js @@ -27,7 +27,7 @@ var server = http.createServer(function(req, res) { res.writeHead(200, { b: header }); break; default: - assert.fail('unreachable'); + assert.fail(null, null, 'unreachable'); } res.write(responseBody); if (testIndex % 8 < 4) { diff --git a/test/parallel/test-http-localaddress-bind-error.js b/test/parallel/test-http-localaddress-bind-error.js index 80c7cab7c8e1ad..9a4bde26cd22d6 100644 --- a/test/parallel/test-http-localaddress-bind-error.js +++ b/test/parallel/test-http-localaddress-bind-error.js @@ -24,7 +24,7 @@ server.listen(common.PORT, '127.0.0.1', function() { method: 'GET', localAddress: invalidLocalAddress }, function(res) { - assert.fail('unexpectedly got response from server'); + assert.fail(null, null, 'unexpectedly got response from server'); }).on('error', function(e) { console.log('client got error: ' + e.message); gotError = true; diff --git a/test/parallel/test-https-localaddress-bind-error.js b/test/parallel/test-https-localaddress-bind-error.js index b3cb4ffe6b077b..66551a780aed9e 100644 --- a/test/parallel/test-https-localaddress-bind-error.js +++ b/test/parallel/test-https-localaddress-bind-error.js @@ -35,7 +35,7 @@ server.listen(common.PORT, '127.0.0.1', function() { method: 'GET', localAddress: invalidLocalAddress }, function(res) { - assert.fail('unexpectedly got response from server'); + assert.fail(null, null, 'unexpectedly got response from server'); }).on('error', function(e) { console.log('client got error: ' + e.message); gotError = true; diff --git a/test/parallel/test-net-connect-paused-connection.js b/test/parallel/test-net-connect-paused-connection.js index 60af9e44fb31f6..bb258ef009a657 100644 --- a/test/parallel/test-net-connect-paused-connection.js +++ b/test/parallel/test-net-connect-paused-connection.js @@ -11,5 +11,5 @@ net.createServer(function(conn) { net.connect(common.PORT, 'localhost').pause(); setTimeout(function() { - assert.fail('expected to exit'); + assert.fail(null, null, 'expected to exit'); }, 1000).unref(); diff --git a/test/parallel/test-net-write-slow.js b/test/parallel/test-net-write-slow.js index 4b8163984f556a..8ded93ee71a8d3 100644 --- a/test/parallel/test-net-write-slow.js +++ b/test/parallel/test-net-write-slow.js @@ -14,7 +14,7 @@ var server = net.createServer(function(socket) { socket.setNoDelay(); socket.setTimeout(1000); socket.on('timeout', function() { - assert.fail('flushed: ' + flushed + + assert.fail(null, null, 'flushed: ' + flushed + ', received: ' + received + '/' + SIZE * N); }); diff --git a/test/parallel/test-path-parse-format.js b/test/parallel/test-path-parse-format.js index 709ede698acebc..c8913d2dfc56e4 100644 --- a/test/parallel/test-path-parse-format.js +++ b/test/parallel/test-path-parse-format.js @@ -89,7 +89,7 @@ function checkErrors(path) { return; } - assert.fail('should have thrown'); + assert.fail(null, null, 'should have thrown'); }); } diff --git a/test/parallel/test-repl-reset-event.js b/test/parallel/test-repl-reset-event.js index e6157956d43a65..e6d4eed1385b10 100644 --- a/test/parallel/test-repl-reset-event.js +++ b/test/parallel/test-repl-reset-event.js @@ -45,7 +45,7 @@ function testResetGlobal(cb) { } var timeout = setTimeout(function() { - assert.fail('Timeout, REPL did not emit reset events'); + assert.fail(null, null, 'Timeout, REPL did not emit reset events'); }, 5000); testReset(function() { diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index 83a9025bec0151..ac4c9012e906f6 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -389,5 +389,5 @@ function unix_test() { unix_test(); timer = setTimeout(function() { - assert.fail('Timeout'); + assert.fail(null, null, 'Timeout'); }, 5000); diff --git a/test/parallel/test-spawn-cmd-named-pipe.js b/test/parallel/test-spawn-cmd-named-pipe.js index c664f7f358c37b..5371577e6c458c 100644 --- a/test/parallel/test-spawn-cmd-named-pipe.js +++ b/test/parallel/test-spawn-cmd-named-pipe.js @@ -39,7 +39,7 @@ if (!process.argv[2]) { const comspec = process.env['comspec']; if (!comspec || comspec.length === 0) { - assert.fail('Failed to get COMSPEC'); + assert.fail(null, null, 'Failed to get COMSPEC'); } const args = ['/c', process.execPath, __filename, 'child', diff --git a/test/parallel/test-stream2-base64-single-char-read-end.js b/test/parallel/test-stream2-base64-single-char-read-end.js index e50ea5a0cc1370..2d60877de83662 100644 --- a/test/parallel/test-stream2-base64-single-char-read-end.js +++ b/test/parallel/test-stream2-base64-single-char-read-end.js @@ -33,5 +33,5 @@ src.on('end', function() { src.pipe(dst); timeout = setTimeout(function() { - assert.fail('timed out waiting for _write'); + assert.fail(null, null, 'timed out waiting for _write'); }, 100); diff --git a/test/parallel/test-tick-processor.js b/test/parallel/test-tick-processor.js index d6bf642bba38ce..cd110e1a87ed3d 100644 --- a/test/parallel/test-tick-processor.js +++ b/test/parallel/test-tick-processor.js @@ -40,7 +40,7 @@ function runTest(pattern, code) { return /^isolate-/.test(file); }); if (matches.length != 1) { - assert.fail('There should be a single log file.'); + assert.fail(null, null, 'There should be a single log file.'); } var log = matches[0]; var out = cp.execSync(process.execPath + ' ' + processor + From d62d15843cfc128383344aa6e0b81981ddc656aa Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 16 Oct 2015 21:07:21 -0700 Subject: [PATCH 16/51] test: fix flaky test for symlinks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the symlink portion of the test was being skipped due to a combination of OS support and user privileges, then an assertion would always fail. This fixes that problem, improves assertion error reporting and splits the test to make it clear that it is a test for links and symlinks. Fixes: https://github.com/nodejs/node/issues/3311 PR-URL: https://github.com/nodejs/node/pull/3418 Reviewed-By: Johan Bergström --- test/parallel/test-fs-link.js | 20 +++++++ test/parallel/test-fs-symlink.js | 97 ++++++++++++-------------------- 2 files changed, 55 insertions(+), 62 deletions(-) create mode 100644 test/parallel/test-fs-link.js diff --git a/test/parallel/test-fs-link.js b/test/parallel/test-fs-link.js new file mode 100644 index 00000000000000..4e95d20f7b6959 --- /dev/null +++ b/test/parallel/test-fs-link.js @@ -0,0 +1,20 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); + +common.refreshTmpDir(); + +// test creating and reading hard link +const srcPath = path.join(common.fixturesDir, 'cycles', 'root.js'); +const dstPath = path.join(common.tmpDir, 'link1.js'); + +const callback = function(err) { + if (err) throw err; + const srcContent = fs.readFileSync(srcPath, 'utf8'); + const dstContent = fs.readFileSync(dstPath, 'utf8'); + assert.strictEqual(srcContent, dstContent); +}; + +fs.link(srcPath, dstPath, common.mustCall(callback)); diff --git a/test/parallel/test-fs-symlink.js b/test/parallel/test-fs-symlink.js index 199add4a1ba724..b506013b0a23f5 100644 --- a/test/parallel/test-fs-symlink.js +++ b/test/parallel/test-fs-symlink.js @@ -1,77 +1,50 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); -var path = require('path'); -var fs = require('fs'); -var exec = require('child_process').exec; -var completed = 0; -var expected_async = 4; +const common = require('../common'); +const assert = require('assert'); +const path = require('path'); +const fs = require('fs'); +const exec = require('child_process').exec; + var linkTime; var fileTime; -common.refreshTmpDir(); - -var runtest = function(skip_symlinks) { - if (!skip_symlinks) { - // test creating and reading symbolic link - var linkData = path.join(common.fixturesDir, '/cycles/root.js'); - var linkPath = path.join(common.tmpDir, 'symlink1.js'); - - fs.symlink(linkData, linkPath, function(err) { - if (err) throw err; - console.log('symlink done'); - - fs.lstat(linkPath, function(err, stats) { - if (err) throw err; - linkTime = stats.mtime.getTime(); - completed++; - }); - - fs.stat(linkPath, function(err, stats) { - if (err) throw err; - fileTime = stats.mtime.getTime(); - completed++; - }); - - fs.readlink(linkPath, function(err, destination) { - if (err) throw err; - assert.equal(destination, linkData); - completed++; - }); - }); - } - - // test creating and reading hard link - var srcPath = path.join(common.fixturesDir, 'cycles', 'root.js'); - var dstPath = path.join(common.tmpDir, 'link1.js'); - - fs.link(srcPath, dstPath, function(err) { - if (err) throw err; - console.log('hard link done'); - var srcContent = fs.readFileSync(srcPath, 'utf8'); - var dstContent = fs.readFileSync(dstPath, 'utf8'); - assert.equal(srcContent, dstContent); - completed++; - }); -}; - if (common.isWindows) { // On Windows, creating symlinks requires admin privileges. // We'll only try to run symlink test if we have enough privileges. exec('whoami /priv', function(err, o) { if (err || o.indexOf('SeCreateSymbolicLinkPrivilege') == -1) { - expected_async = 1; - runtest(true); - } else { - runtest(false); + console.log('1..0 # Skipped: insufficient privileges'); + return; } }); -} else { - runtest(false); } -process.on('exit', function() { - assert.equal(completed, expected_async); - assert(linkTime !== fileTime); +common.refreshTmpDir(); + +// test creating and reading symbolic link +const linkData = path.join(common.fixturesDir, '/cycles/root.js'); +const linkPath = path.join(common.tmpDir, 'symlink1.js'); + +fs.symlink(linkData, linkPath, function(err) { + if (err) throw err; + + fs.lstat(linkPath, common.mustCall(function(err, stats) { + if (err) throw err; + linkTime = stats.mtime.getTime(); + })); + + fs.stat(linkPath, common.mustCall(function(err, stats) { + if (err) throw err; + fileTime = stats.mtime.getTime(); + })); + + fs.readlink(linkPath, common.mustCall(function(err, destination) { + if (err) throw err; + assert.equal(destination, linkData); + })); }); + +process.on('exit', function() { + assert.notStrictEqual(linkTime, fileTime); +}); From 0c23e81d7a7448ac24d260c095e03bab627913e5 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 17 Oct 2015 18:52:15 -0700 Subject: [PATCH 17/51] test: remove flaky status from eval_messages test This test has not failed on armv7-wheezy in over 6 weeks. Let's remove its "flaky" status. Fixes: https://github.com/nodejs/node/issues/2672 Reviewed-By: Rod Vagg Reviewed-By: Jeremiah Senkpiel Reviewed-By: James M Snell PR-URL: https://github.com/nodejs/node/pull/3420 --- test/message/message.status | 1 - 1 file changed, 1 deletion(-) diff --git a/test/message/message.status b/test/message/message.status index 67c6de53424872..1a4a0e3adc2727 100644 --- a/test/message/message.status +++ b/test/message/message.status @@ -9,7 +9,6 @@ prefix message [$system==win32] [$system==linux] -eval_messages : PASS,FLAKY [$system==macos] From 0b324142ca69fa0031e4d91038c788597ea38434 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 18 Oct 2015 14:42:44 -0700 Subject: [PATCH 18/51] test: fix flaky test-child-process-emfile Require the test setup to obtain an EMFILE error and not ENFILE as ENFILE means there is a race condition with other processes that may close files before `spawn()` is called by the test. Fixes: https://github.com/nodejs/node/issues/2666 PR-URL: https://github.com/nodejs/node/pull/3430 Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell --- test/sequential/test-child-process-emfile.js | 32 +++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/test/sequential/test-child-process-emfile.js b/test/sequential/test-child-process-emfile.js index ae7964d9e7a7d6..2ac0b6c0b23a73 100644 --- a/test/sequential/test-child-process-emfile.js +++ b/test/sequential/test-child-process-emfile.js @@ -1,30 +1,46 @@ 'use strict'; -var common = require('../common'); -var assert = require('assert'); -var spawn = require('child_process').spawn; -var fs = require('fs'); +const common = require('../common'); +const assert = require('assert'); +const child_process = require('child_process'); +const fs = require('fs'); if (common.isWindows) { console.log('1..0 # Skipped: no RLIMIT_NOFILE on Windows'); return; } -var openFds = []; +const ulimit = Number(child_process.execSync('ulimit -Hn')); +if (ulimit > 64 || Number.isNaN(ulimit)) { + // Sorry about this nonsense. It can be replaced if + // https://github.com/nodejs/node-v0.x-archive/pull/2143#issuecomment-2847886 + // ever happens. + const result = child_process.spawnSync( + '/bin/sh', + ['-c', `ulimit -n 64 && '${process.execPath}' '${__filename}'`] + ); + assert.strictEqual(result.stdout.toString(), ''); + assert.strictEqual(result.stderr.toString(), ''); + assert.strictEqual(result.status, 0); + assert.strictEqual(result.error, undefined); + return; +} + +const openFds = []; for (;;) { try { openFds.push(fs.openSync(__filename, 'r')); } catch (err) { - assert(err.code === 'EMFILE' || err.code === 'ENFILE'); + assert(err.code === 'EMFILE'); break; } } // Should emit an error, not throw. -var proc = spawn(process.execPath, ['-e', '0']); +const proc = child_process.spawn(process.execPath, ['-e', '0']); proc.on('error', common.mustCall(function(err) { - assert(err.code === 'EMFILE' || err.code === 'ENFILE'); + assert.strictEqual(err.code, 'EMFILE'); })); proc.on('exit', function() { From 3d1f57d9dd59cef20500be728573beb0c7263d09 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Mon, 19 Oct 2015 11:57:55 -0400 Subject: [PATCH 19/51] test: repl-persistent-history is no longer flaky MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The race conditions were fixed in 286ef1daca1c1f3e3363ee162fb97fb823632352 PR-URL: https://github.com/nodejs/node/pull/3437 Reviewed By: Evan Lucas Reviewed-By: James M Snell Reviewed-By: Johan Bergström --- test/sequential/sequential.status | 1 - 1 file changed, 1 deletion(-) diff --git a/test/sequential/sequential.status b/test/sequential/sequential.status index e7f0ca2b323992..7d5e6f1c1e624f 100644 --- a/test/sequential/sequential.status +++ b/test/sequential/sequential.status @@ -5,7 +5,6 @@ prefix sequential # sample-test : PASS,FLAKY [true] # This section applies to all platforms -test-repl-persistent-history : PASS,FLAKY [$system==win32] From 47ab7c76f0670e0839af2b0425bbfde41fee686a Mon Sep 17 00:00:00 2001 From: fansworld-claudio Date: Mon, 19 Oct 2015 16:10:18 -0300 Subject: [PATCH 20/51] docs: add missing shell option to execSync Adds the "shell" option from child_process.exec to child_process.execSync on the api docs. Fixes: #3387 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig PR-URL: https://github.com/nodejs/node/pull/3440 --- doc/api/child_process.markdown | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/api/child_process.markdown b/doc/api/child_process.markdown index bddf90ff85b625..13997d65452909 100644 --- a/doc/api/child_process.markdown +++ b/doc/api/child_process.markdown @@ -716,6 +716,10 @@ process has exited. - `stderr` by default will be output to the parent process' stderr unless `stdio` is specified * `env` {Object} Environment key-value pairs + * `shell` {String} Shell to execute the command with + (Default: '/bin/sh' on UNIX, 'cmd.exe' on Windows, The shell should + understand the `-c` switch on UNIX or `/s /c` on Windows. On Windows, + command line parsing should be compatible with `cmd.exe`.) * `uid` {Number} Sets the user identity of the process. (See setuid(2).) * `gid` {Number} Sets the group identity of the process. (See setgid(2).) * `timeout` {Number} In milliseconds the maximum amount of time the process is allowed to run. (Default: undefined) From 49549c9b2a9ae33eaa370dfb8d2f55340da3b20e Mon Sep 17 00:00:00 2001 From: Junliang Yan Date: Mon, 19 Oct 2015 15:30:11 -0400 Subject: [PATCH 21/51] test: skip test-dns-ipv6.js if ipv6 is unavailable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Test should be skipped if ipv6 is unavailable on the running system. Reviewed-By: Jeremiah Senkpiel Reviewed-By: Evan Lucas Reviewed-By: Johan Bergström Reviewed-By: James M Snell PR-URL: https://github.com/nodejs/node/pull/3444 --- test/internet/test-dns-ipv6.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/internet/test-dns-ipv6.js b/test/internet/test-dns-ipv6.js index 27b25f7995e630..5a66f8db783f4b 100644 --- a/test/internet/test-dns-ipv6.js +++ b/test/internet/test-dns-ipv6.js @@ -12,6 +12,11 @@ var expected = 0, running = false, queue = []; +if (!common.hasIPv6) { + console.log('1..0 # Skipped: this test, no IPv6 support'); + return; +} + function TEST(f) { function next() { var f = queue.shift(); From 635f74366f3815a7fc5a9858876f3e58a55f707e Mon Sep 17 00:00:00 2001 From: Myles Borins Date: Tue, 20 Oct 2015 11:40:54 -0700 Subject: [PATCH 22/51] test: wrap assert.fail when passed to callback Currently there are many instances where assert.fail is directly passed to a callback for error handling. Unfortunately this will swallow the error as it is the third argument of assert.fail that sets the message not the first. This commit adds a new function to test/common.js that simply wraps assert.fail and calls it with the provided message. Tip of the hat to @trott for pointing me in the direction of this. PR-URL: https://github.com/nodejs/node/pull/3453 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Rich Trott --- test/common.js | 4 +++ .../test-child-process-recv-handle.js | 2 +- .../test-child-process-spawn-typeerror.js | 2 +- .../test-cluster-bind-privileged-port.js | 4 +-- test/parallel/test-cluster-bind-twice.js | 6 ++-- test/parallel/test-cluster-eaddrinuse.js | 8 +++--- test/parallel/test-cluster-net-listen.js | 2 +- test/parallel/test-cluster-rr-ref.js | 2 +- .../test-cluster-setup-master-argv.js | 2 +- .../test-cluster-shared-handle-bind-error.js | 6 ++-- ...ster-shared-handle-bind-privileged-port.js | 4 +-- test/parallel/test-crypto-pbkdf2.js | 8 +++--- .../test-dgram-error-message-address.js | 4 +-- test/parallel/test-dgram-oob-buffer.js | 12 ++++---- .../test-event-emitter-remove-listeners.js | 2 +- test/parallel/test-fs-null-bytes.js | 6 ++-- test/parallel/test-fs-read-stream-err.js | 2 +- .../test-http-client-unescaped-path.js | 2 +- test/parallel/test-http-set-trailers.js | 2 +- test/parallel/test-https-timeout-server-2.js | 2 +- test/parallel/test-https-timeout-server.js | 2 +- test/parallel/test-listen-fd-ebadf.js | 4 +-- ...t-net-better-error-messages-listen-path.js | 4 +-- .../test-net-better-error-messages-listen.js | 4 +-- .../test-net-better-error-messages-path.js | 2 +- ...net-better-error-messages-port-hostname.js | 2 +- .../test-net-better-error-messages-port.js | 2 +- test/parallel/test-net-dns-lookup-skip.js | 2 +- test/parallel/test-net-listen-fd0.js | 2 +- .../test-promises-unhandled-rejections.js | 28 +++++++++---------- ...-timers-unref-remove-other-unref-timers.js | 2 +- test/parallel/test-tls-cipher-list.js | 4 +-- test/parallel/test-tls-no-sslv3.js | 2 +- test/parallel/test-tls-timeout-server.js | 2 +- test/parallel/test-vm-debug-context.js | 8 +++--- test/pummel/test-fs-watch-non-recursive.js | 2 +- .../sequential/test-cluster-listening-port.js | 4 +-- 37 files changed, 81 insertions(+), 77 deletions(-) diff --git a/test/common.js b/test/common.js index c1c488c4ead15a..a9641641ce50fc 100644 --- a/test/common.js +++ b/test/common.js @@ -446,3 +446,7 @@ exports.fileExists = function(pathname) { return false; } }; + +exports.fail = function(msg) { + assert.fail(null, null, msg); +}; diff --git a/test/parallel/test-child-process-recv-handle.js b/test/parallel/test-child-process-recv-handle.js index b992445f186383..5c16c7a1c65850 100644 --- a/test/parallel/test-child-process-recv-handle.js +++ b/test/parallel/test-child-process-recv-handle.js @@ -24,7 +24,7 @@ function master() { }); proc.stdout.on('data', function(data) { assert.equal(data, 'ok\r\n'); - net.createServer(assert.fail).listen(common.PORT, function() { + net.createServer(common.fail).listen(common.PORT, function() { handle = this._handle; proc.send('one'); proc.send('two', handle); diff --git a/test/parallel/test-child-process-spawn-typeerror.js b/test/parallel/test-child-process-spawn-typeerror.js index 44d67e86086f0b..46216b63d569ef 100644 --- a/test/parallel/test-child-process-spawn-typeerror.js +++ b/test/parallel/test-child-process-spawn-typeerror.js @@ -13,7 +13,7 @@ const empty = common.fixturesDir + '/empty.js'; assert.throws(function() { var child = spawn(invalidcmd, 'this is not an array'); - child.on('error', assert.fail); + child.on('error', common.fail); }, TypeError); // verify that valid argument combinations do not throw diff --git a/test/parallel/test-cluster-bind-privileged-port.js b/test/parallel/test-cluster-bind-privileged-port.js index d173762575cb8f..56449eaaf2e02f 100644 --- a/test/parallel/test-cluster-bind-privileged-port.js +++ b/test/parallel/test-cluster-bind-privileged-port.js @@ -20,8 +20,8 @@ if (cluster.isMaster) { })); } else { - var s = net.createServer(assert.fail); - s.listen(42, assert.fail.bind(null, 'listen should have failed')); + var s = net.createServer(common.fail); + s.listen(42, common.fail.bind(null, 'listen should have failed')); s.on('error', common.mustCall(function(err) { assert.equal(err.code, 'EACCES'); process.disconnect(); diff --git a/test/parallel/test-cluster-bind-twice.js b/test/parallel/test-cluster-bind-twice.js index ec6faa83c1151b..b3046cd8d5e1cc 100644 --- a/test/parallel/test-cluster-bind-twice.js +++ b/test/parallel/test-cluster-bind-twice.js @@ -68,7 +68,7 @@ if (!id) { else if (id === 'one') { if (cluster.isMaster) return startWorker(); - var server = http.createServer(assert.fail).listen(common.PORT, function() { + var server = http.createServer(common.fail).listen(common.PORT, function() { process.send('READY'); }); @@ -84,12 +84,12 @@ else if (id === 'two') { assert(ok); }); - var server = http.createServer(assert.fail); + var server = http.createServer(common.fail); process.on('message', function(m) { if (typeof m === 'object') return; // ignore system messages if (m === 'QUIT') process.exit(); assert.equal(m, 'START'); - server.listen(common.PORT, assert.fail); + server.listen(common.PORT, common.fail); server.on('error', function(e) { assert.equal(e.code, 'EADDRINUSE'); process.send(e.code); diff --git a/test/parallel/test-cluster-eaddrinuse.js b/test/parallel/test-cluster-eaddrinuse.js index 509dbb664e1d5d..6ff68252d9d867 100644 --- a/test/parallel/test-cluster-eaddrinuse.js +++ b/test/parallel/test-cluster-eaddrinuse.js @@ -12,7 +12,7 @@ var net = require('net'); var id = '' + process.argv[2]; if (id === 'undefined') { - var server = net.createServer(assert.fail); + var server = net.createServer(common.fail); server.listen(common.PORT, function() { var worker = fork(__filename, ['worker']); worker.on('message', function(msg) { @@ -24,14 +24,14 @@ if (id === 'undefined') { }); } else if (id === 'worker') { - var server = net.createServer(assert.fail); - server.listen(common.PORT, assert.fail); + var server = net.createServer(common.fail); + server.listen(common.PORT, common.fail); server.on('error', common.mustCall(function(e) { assert(e.code, 'EADDRINUSE'); process.send('stop-listening'); process.once('message', function(msg) { if (msg !== 'stopped-listening') return; - server = net.createServer(assert.fail); + server = net.createServer(common.fail); server.listen(common.PORT, common.mustCall(function() { server.close(); })); diff --git a/test/parallel/test-cluster-net-listen.js b/test/parallel/test-cluster-net-listen.js index 741cacc75897e3..c79d4cf1a27dad 100644 --- a/test/parallel/test-cluster-net-listen.js +++ b/test/parallel/test-cluster-net-listen.js @@ -17,5 +17,5 @@ if (cluster.isMaster) { } else { // listen() without port should not trigger a libuv assert - net.createServer(assert.fail).listen(process.exit); + net.createServer(common.fail).listen(process.exit); } diff --git a/test/parallel/test-cluster-rr-ref.js b/test/parallel/test-cluster-rr-ref.js index 606ff708e94029..474e4d69f225c6 100644 --- a/test/parallel/test-cluster-rr-ref.js +++ b/test/parallel/test-cluster-rr-ref.js @@ -10,7 +10,7 @@ if (cluster.isMaster) { if (msg === 'done') this.kill(); }); } else { - const server = net.createServer(assert.fail); + const server = net.createServer(common.fail); server.listen(common.PORT, function() { server.unref(); server.ref(); diff --git a/test/parallel/test-cluster-setup-master-argv.js b/test/parallel/test-cluster-setup-master-argv.js index b406c76cbbfe0b..e111ba9ffa1548 100644 --- a/test/parallel/test-cluster-setup-master-argv.js +++ b/test/parallel/test-cluster-setup-master-argv.js @@ -3,7 +3,7 @@ var common = require('../common'); var assert = require('assert'); var cluster = require('cluster'); -setTimeout(assert.fail.bind(assert, 'setup not emitted'), 1000).unref(); +setTimeout(common.fail.bind(assert, 'setup not emitted'), 1000).unref(); cluster.on('setup', function() { var clusterArgs = cluster.settings.args; diff --git a/test/parallel/test-cluster-shared-handle-bind-error.js b/test/parallel/test-cluster-shared-handle-bind-error.js index a93b07ba30e27b..288b4b443a45bc 100644 --- a/test/parallel/test-cluster-shared-handle-bind-error.js +++ b/test/parallel/test-cluster-shared-handle-bind-error.js @@ -8,7 +8,7 @@ if (cluster.isMaster) { // Master opens and binds the socket and shares it with the worker. cluster.schedulingPolicy = cluster.SCHED_NONE; // Hog the TCP port so that when the worker tries to bind, it'll fail. - net.createServer(assert.fail).listen(common.PORT, function() { + net.createServer(common.fail).listen(common.PORT, function() { var server = this; var worker = cluster.fork(); worker.on('exit', common.mustCall(function(exitCode) { @@ -18,8 +18,8 @@ if (cluster.isMaster) { }); } else { - var s = net.createServer(assert.fail); - s.listen(common.PORT, assert.fail.bind(null, 'listen should have failed')); + var s = net.createServer(common.fail); + s.listen(common.PORT, common.fail.bind(null, 'listen should have failed')); s.on('error', common.mustCall(function(err) { assert.equal(err.code, 'EADDRINUSE'); process.disconnect(); diff --git a/test/parallel/test-cluster-shared-handle-bind-privileged-port.js b/test/parallel/test-cluster-shared-handle-bind-privileged-port.js index f524d6bda9d946..a02a2ef5b6b4f1 100644 --- a/test/parallel/test-cluster-shared-handle-bind-privileged-port.js +++ b/test/parallel/test-cluster-shared-handle-bind-privileged-port.js @@ -22,8 +22,8 @@ if (cluster.isMaster) { })); } else { - var s = net.createServer(assert.fail); - s.listen(42, assert.fail.bind(null, 'listen should have failed')); + var s = net.createServer(common.fail); + s.listen(42, common.fail.bind(null, 'listen should have failed')); s.on('error', common.mustCall(function(err) { assert.equal(err.code, 'EACCES'); process.disconnect(); diff --git a/test/parallel/test-crypto-pbkdf2.js b/test/parallel/test-crypto-pbkdf2.js index 51759ca8357ee7..39b98b38e2710e 100644 --- a/test/parallel/test-crypto-pbkdf2.js +++ b/test/parallel/test-crypto-pbkdf2.js @@ -62,28 +62,28 @@ assert.throws(function() { // Should not work with Infinity key length assert.throws(function() { - crypto.pbkdf2('password', 'salt', 1, Infinity, assert.fail); + crypto.pbkdf2('password', 'salt', 1, Infinity, common.fail); }, function(err) { return err instanceof Error && err.message === 'Bad key length'; }); // Should not work with negative Infinity key length assert.throws(function() { - crypto.pbkdf2('password', 'salt', 1, -Infinity, assert.fail); + crypto.pbkdf2('password', 'salt', 1, -Infinity, common.fail); }, function(err) { return err instanceof Error && err.message === 'Bad key length'; }); // Should not work with NaN key length assert.throws(function() { - crypto.pbkdf2('password', 'salt', 1, NaN, assert.fail); + crypto.pbkdf2('password', 'salt', 1, NaN, common.fail); }, function(err) { return err instanceof Error && err.message === 'Bad key length'; }); // Should not work with negative key length assert.throws(function() { - crypto.pbkdf2('password', 'salt', 1, -1, assert.fail); + crypto.pbkdf2('password', 'salt', 1, -1, common.fail); }, function(err) { return err instanceof Error && err.message === 'Bad key length'; }); diff --git a/test/parallel/test-dgram-error-message-address.js b/test/parallel/test-dgram-error-message-address.js index eca2ccce4f1b65..e307a23e2451db 100644 --- a/test/parallel/test-dgram-error-message-address.js +++ b/test/parallel/test-dgram-error-message-address.js @@ -6,7 +6,7 @@ var dgram = require('dgram'); // IPv4 Test var socket_ipv4 = dgram.createSocket('udp4'); -socket_ipv4.on('listening', assert.fail); +socket_ipv4.on('listening', common.fail); socket_ipv4.on('error', common.mustCall(function(e) { assert.equal(e.message, 'bind EADDRNOTAVAIL 1.1.1.1:' + common.PORT); @@ -22,7 +22,7 @@ socket_ipv4.bind(common.PORT, '1.1.1.1'); var socket_ipv6 = dgram.createSocket('udp6'); var family_ipv6 = 'IPv6'; -socket_ipv6.on('listening', assert.fail); +socket_ipv6.on('listening', common.fail); socket_ipv6.on('error', common.mustCall(function(e) { // EAFNOSUPPORT or EPROTONOSUPPORT means IPv6 is disabled on this system. diff --git a/test/parallel/test-dgram-oob-buffer.js b/test/parallel/test-dgram-oob-buffer.js index e873715e4e14de..6d0626fc2d40db 100644 --- a/test/parallel/test-dgram-oob-buffer.js +++ b/test/parallel/test-dgram-oob-buffer.js @@ -19,22 +19,22 @@ socket.send(buf, 3, 1, common.PORT, '127.0.0.1', ok); socket.send(buf, 4, 0, common.PORT, '127.0.0.1', ok); assert.throws(function() { - socket.send(buf, 0, 5, common.PORT, '127.0.0.1', assert.fail); + socket.send(buf, 0, 5, common.PORT, '127.0.0.1', common.fail); }); assert.throws(function() { - socket.send(buf, 2, 3, common.PORT, '127.0.0.1', assert.fail); + socket.send(buf, 2, 3, common.PORT, '127.0.0.1', common.fail); }); assert.throws(function() { - socket.send(buf, 4, 4, common.PORT, '127.0.0.1', assert.fail); + socket.send(buf, 4, 4, common.PORT, '127.0.0.1', common.fail); }); assert.throws(function() { - socket.send('abc', 4, 1, common.PORT, '127.0.0.1', assert.fail); + socket.send('abc', 4, 1, common.PORT, '127.0.0.1', common.fail); }); assert.throws(function() { - socket.send('abc', 0, 4, common.PORT, '127.0.0.1', assert.fail); + socket.send('abc', 0, 4, common.PORT, '127.0.0.1', common.fail); }); assert.throws(function() { - socket.send('abc', -1, 2, common.PORT, '127.0.0.1', assert.fail); + socket.send('abc', -1, 2, common.PORT, '127.0.0.1', common.fail); }); socket.close(); // FIXME should not be necessary diff --git a/test/parallel/test-event-emitter-remove-listeners.js b/test/parallel/test-event-emitter-remove-listeners.js index 409ccbebe25b86..8993aadf51a46e 100644 --- a/test/parallel/test-event-emitter-remove-listeners.js +++ b/test/parallel/test-event-emitter-remove-listeners.js @@ -39,7 +39,7 @@ assert.deepEqual([], e1.listeners('hello')); var e2 = new events.EventEmitter(); e2.on('hello', listener1); -e2.on('removeListener', assert.fail); +e2.on('removeListener', common.fail); e2.removeListener('hello', listener2); assert.deepEqual([listener1], e2.listeners('hello')); diff --git a/test/parallel/test-fs-null-bytes.js b/test/parallel/test-fs-null-bytes.js index 77228521e6fc9d..c80588486d97d6 100644 --- a/test/parallel/test-fs-null-bytes.js +++ b/test/parallel/test-fs-null-bytes.js @@ -43,10 +43,10 @@ check(fs.symlink, fs.symlinkSync, 'foo\u0000bar', 'foobar'); check(fs.symlink, fs.symlinkSync, 'foobar', 'foo\u0000bar'); check(fs.truncate, fs.truncateSync, 'foo\u0000bar'); check(fs.unlink, fs.unlinkSync, 'foo\u0000bar'); -check(null, fs.unwatchFile, 'foo\u0000bar', assert.fail); +check(null, fs.unwatchFile, 'foo\u0000bar', common.fail); check(fs.utimes, fs.utimesSync, 'foo\u0000bar', 0, 0); -check(null, fs.watch, 'foo\u0000bar', assert.fail); -check(null, fs.watchFile, 'foo\u0000bar', assert.fail); +check(null, fs.watch, 'foo\u0000bar', common.fail); +check(null, fs.watchFile, 'foo\u0000bar', common.fail); check(fs.writeFile, fs.writeFileSync, 'foo\u0000bar'); // an 'error' for exists means that it doesn't exist. diff --git a/test/parallel/test-fs-read-stream-err.js b/test/parallel/test-fs-read-stream-err.js index 1eb6aa57906b05..1bc2b6f0b0239c 100644 --- a/test/parallel/test-fs-read-stream-err.js +++ b/test/parallel/test-fs-read-stream-err.js @@ -39,5 +39,5 @@ fs.read = function() { }; stream.on('data', function(buf) { - stream.on('data', assert.fail); // no more 'data' events should follow + stream.on('data', common.fail); // no more 'data' events should follow }); diff --git a/test/parallel/test-http-client-unescaped-path.js b/test/parallel/test-http-client-unescaped-path.js index 1536916ae9b49c..e01df255a8042c 100644 --- a/test/parallel/test-http-client-unescaped-path.js +++ b/test/parallel/test-http-client-unescaped-path.js @@ -5,5 +5,5 @@ var http = require('http'); assert.throws(function() { // Path with spaces in it should throw. - http.get({ path: 'bad path' }, assert.fail); + http.get({ path: 'bad path' }, common.fail); }, /contains unescaped characters/); diff --git a/test/parallel/test-http-set-trailers.js b/test/parallel/test-http-set-trailers.js index 6f5c02f5605969..f3ee5b157f7915 100644 --- a/test/parallel/test-http-set-trailers.js +++ b/test/parallel/test-http-set-trailers.js @@ -53,7 +53,7 @@ server.on('listening', function() { c.on('connect', function() { outstanding_reqs++; c.write('GET / HTTP/1.1\r\n\r\n'); - tid = setTimeout(assert.fail, 2000, 'Couldn\'t find last chunk.'); + tid = setTimeout(common.fail, 2000, 'Couldn\'t find last chunk.'); }); c.on('data', function(chunk) { diff --git a/test/parallel/test-https-timeout-server-2.js b/test/parallel/test-https-timeout-server-2.js index 9970688fe7c1e8..df605958cf42df 100644 --- a/test/parallel/test-https-timeout-server-2.js +++ b/test/parallel/test-https-timeout-server-2.js @@ -18,7 +18,7 @@ var options = { cert: fs.readFileSync(common.fixturesDir + '/keys/agent1-cert.pem') }; -var server = https.createServer(options, assert.fail); +var server = https.createServer(options, common.fail); server.on('secureConnection', function(cleartext) { var s = cleartext.setTimeout(50, function() { diff --git a/test/parallel/test-https-timeout-server.js b/test/parallel/test-https-timeout-server.js index 0db7ad533d9868..ba175ecf763ecf 100644 --- a/test/parallel/test-https-timeout-server.js +++ b/test/parallel/test-https-timeout-server.js @@ -24,7 +24,7 @@ var options = { handshakeTimeout: 50 }; -var server = https.createServer(options, assert.fail); +var server = https.createServer(options, common.fail); server.on('clientError', function(err, conn) { // Don't hesitate to update the asserts if the internal structure of diff --git a/test/parallel/test-listen-fd-ebadf.js b/test/parallel/test-listen-fd-ebadf.js index db905dfa35e966..51e09a7907f18e 100644 --- a/test/parallel/test-listen-fd-ebadf.js +++ b/test/parallel/test-listen-fd-ebadf.js @@ -9,8 +9,8 @@ process.on('exit', function() { assert.equal(gotError, 2); }); -net.createServer(assert.fail).listen({fd:2}).on('error', onError); -net.createServer(assert.fail).listen({fd:42}).on('error', onError); +net.createServer(common.fail).listen({fd:2}).on('error', onError); +net.createServer(common.fail).listen({fd:42}).on('error', onError); function onError(ex) { assert.equal(ex.code, 'EINVAL'); diff --git a/test/parallel/test-net-better-error-messages-listen-path.js b/test/parallel/test-net-better-error-messages-listen-path.js index 8a7e0220b36dfa..41d22c3fb9a4b5 100644 --- a/test/parallel/test-net-better-error-messages-listen-path.js +++ b/test/parallel/test-net-better-error-messages-listen-path.js @@ -3,8 +3,8 @@ var common = require('../common'); var assert = require('assert'); var net = require('net'); var fp = '/blah/fadfa'; -var server = net.createServer(assert.fail); -server.listen(fp, assert.fail); +var server = net.createServer(common.fail); +server.listen(fp, common.fail); server.on('error', common.mustCall(function(e) { assert.equal(e.address, fp); })); diff --git a/test/parallel/test-net-better-error-messages-listen.js b/test/parallel/test-net-better-error-messages-listen.js index 7e5fad925aeb92..44adce71a7d541 100644 --- a/test/parallel/test-net-better-error-messages-listen.js +++ b/test/parallel/test-net-better-error-messages-listen.js @@ -3,8 +3,8 @@ var common = require('../common'); var assert = require('assert'); var net = require('net'); -var server = net.createServer(assert.fail); -server.listen(1, '1.1.1.1', assert.fail); +var server = net.createServer(common.fail); +server.listen(1, '1.1.1.1', common.fail); server.on('error', common.mustCall(function(e) { assert.equal(e.address, '1.1.1.1'); assert.equal(e.port, 1); diff --git a/test/parallel/test-net-better-error-messages-path.js b/test/parallel/test-net-better-error-messages-path.js index 06cfecbd7c6af1..9222a1cc758aaa 100644 --- a/test/parallel/test-net-better-error-messages-path.js +++ b/test/parallel/test-net-better-error-messages-path.js @@ -5,7 +5,7 @@ var assert = require('assert'); var fp = '/tmp/fadagagsdfgsdf'; var c = net.connect(fp); -c.on('connect', assert.fail); +c.on('connect', common.fail); c.on('error', common.mustCall(function(e) { assert.equal(e.code, 'ENOENT'); diff --git a/test/parallel/test-net-better-error-messages-port-hostname.js b/test/parallel/test-net-better-error-messages-port-hostname.js index bdca6c2b3ca00d..9db6fb26f57285 100644 --- a/test/parallel/test-net-better-error-messages-port-hostname.js +++ b/test/parallel/test-net-better-error-messages-port-hostname.js @@ -5,7 +5,7 @@ var assert = require('assert'); var c = net.createConnection(common.PORT, '...'); -c.on('connect', assert.fail); +c.on('connect', common.fail); c.on('error', common.mustCall(function(e) { assert.equal(e.code, 'ENOTFOUND'); diff --git a/test/parallel/test-net-better-error-messages-port.js b/test/parallel/test-net-better-error-messages-port.js index 0f90089c0509fe..514e317fbb0b15 100644 --- a/test/parallel/test-net-better-error-messages-port.js +++ b/test/parallel/test-net-better-error-messages-port.js @@ -5,7 +5,7 @@ var assert = require('assert'); var c = net.createConnection(common.PORT); -c.on('connect', assert.fail); +c.on('connect', common.fail); c.on('error', common.mustCall(function(e) { assert.equal(e.code, 'ECONNREFUSED'); diff --git a/test/parallel/test-net-dns-lookup-skip.js b/test/parallel/test-net-dns-lookup-skip.js index 1083ed9fc0ad8f..b293196ad45c7e 100644 --- a/test/parallel/test-net-dns-lookup-skip.js +++ b/test/parallel/test-net-dns-lookup-skip.js @@ -11,7 +11,7 @@ function check(addressType) { var address = addressType === 4 ? '127.0.0.1' : '::1'; server.listen(common.PORT, address, function() { - net.connect(common.PORT, address).on('lookup', assert.fail); + net.connect(common.PORT, address).on('lookup', common.fail); }); } diff --git a/test/parallel/test-net-listen-fd0.js b/test/parallel/test-net-listen-fd0.js index e326ac2b60beb1..1a6c4716eb6178 100644 --- a/test/parallel/test-net-listen-fd0.js +++ b/test/parallel/test-net-listen-fd0.js @@ -10,7 +10,7 @@ process.on('exit', function() { }); // this should fail with an async EINVAL error, not throw an exception -net.createServer(assert.fail).listen({fd:0}).on('error', function(e) { +net.createServer(common.fail).listen({fd:0}).on('error', function(e) { switch (e.code) { case 'EINVAL': case 'ENOTSOCK': diff --git a/test/parallel/test-promises-unhandled-rejections.js b/test/parallel/test-promises-unhandled-rejections.js index e5b3e6f35c0d45..e229bada3a10e2 100644 --- a/test/parallel/test-promises-unhandled-rejections.js +++ b/test/parallel/test-promises-unhandled-rejections.js @@ -164,7 +164,7 @@ asyncTest('Catching a promise rejection after setImmediate is not' + }); _reject(e); setImmediate(function() { - promise.then(assert.fail, function() {}); + promise.then(common.fail, function() {}); }); }); @@ -176,7 +176,7 @@ asyncTest('When re-throwing new errors in a promise catch, only the' + assert.strictEqual(e2, reason); assert.strictEqual(promise2, promise); }); - var promise2 = Promise.reject(e).then(assert.fail, function(reason) { + var promise2 = Promise.reject(e).then(common.fail, function(reason) { assert.strictEqual(e, reason); throw e2; }); @@ -206,7 +206,7 @@ asyncTest('When re-throwing new errors in a promise catch, only the ' + setTimeout(function() { reject(e); }, 1); - }).then(assert.fail, function(reason) { + }).then(common.fail, function(reason) { assert.strictEqual(e, reason); throw e2; }); @@ -225,7 +225,7 @@ asyncTest('When re-throwing new errors in a promise catch, only the re-thrown' + setTimeout(function() { reject(e); process.nextTick(function() { - promise2 = promise.then(assert.fail, function(reason) { + promise2 = promise.then(common.fail, function(reason) { assert.strictEqual(e, reason); throw e2; }); @@ -240,7 +240,7 @@ asyncTest('unhandledRejection should not be triggered if a promise catch is' + function(done) { var e = new Error(); onUnhandledFail(done); - Promise.reject(e).then(assert.fail, function() {}); + Promise.reject(e).then(common.fail, function() {}); }); asyncTest('unhandledRejection should not be triggered if a promise catch is' + @@ -250,7 +250,7 @@ asyncTest('unhandledRejection should not be triggered if a promise catch is' + onUnhandledFail(done); new Promise(function(_, reject) { reject(e); - }).then(assert.fail, function() {}); + }).then(common.fail, function() {}); }); asyncTest('Attaching a promise catch in a process.nextTick is soon enough to' + @@ -259,7 +259,7 @@ asyncTest('Attaching a promise catch in a process.nextTick is soon enough to' + onUnhandledFail(done); var promise = Promise.reject(e); process.nextTick(function() { - promise.then(assert.fail, function() {}); + promise.then(common.fail, function() {}); }); }); @@ -271,7 +271,7 @@ asyncTest('Attaching a promise catch in a process.nextTick is soon enough to' + reject(e); }); process.nextTick(function() { - promise.then(assert.fail, function() {}); + promise.then(common.fail, function() {}); }); }); @@ -302,7 +302,7 @@ asyncTest('catching a promise which is asynchronously rejected (via' + reject(e); }, 1); }); - }).then(assert.fail, function(reason) { + }).then(common.fail, function(reason) { assert.strictEqual(e, reason); }); }); @@ -313,7 +313,7 @@ asyncTest('Catching a rejected promise derived from throwing in a' + onUnhandledFail(done); Promise.resolve().then(function() { throw e; - }).then(assert.fail, function(reason) { + }).then(common.fail, function(reason) { assert.strictEqual(e, reason); }); }); @@ -325,7 +325,7 @@ asyncTest('Catching a rejected promise derived from returning a' + onUnhandledFail(done); Promise.resolve().then(function() { return Promise.reject(e); - }).then(assert.fail, function(reason) { + }).then(common.fail, function(reason) { assert.strictEqual(e, reason); }); }); @@ -380,7 +380,7 @@ asyncTest('Catching the Promise.all() of a collection that includes a' + 'rejected promise prevents unhandledRejection', function(done) { var e = new Error(); onUnhandledFail(done); - Promise.all([Promise.reject(e)]).then(assert.fail, function() {}); + Promise.all([Promise.reject(e)]).then(common.fail, function() {}); }); asyncTest('Catching the Promise.all() of a collection that includes a ' + @@ -395,7 +395,7 @@ asyncTest('Catching the Promise.all() of a collection that includes a ' + }); p = Promise.all([p]); process.nextTick(function() { - p.then(assert.fail, function() {}); + p.then(common.fail, function() {}); }); }); @@ -430,7 +430,7 @@ asyncTest('Waiting setTimeout(, 10) to catch a promise causes an' + throw e; }); setTimeout(function() { - thePromise.then(assert.fail, function(reason) { + thePromise.then(common.fail, function(reason) { assert.strictEqual(e, reason); }); }, 10); diff --git a/test/parallel/test-timers-unref-remove-other-unref-timers.js b/test/parallel/test-timers-unref-remove-other-unref-timers.js index f727d5f86fd7b3..8c1864f1a7da01 100644 --- a/test/parallel/test-timers-unref-remove-other-unref-timers.js +++ b/test/parallel/test-timers-unref-remove-other-unref-timers.js @@ -11,7 +11,7 @@ const assert = require('assert'); const timers = require('timers'); const foo = { - _onTimeout: assert.fail + _onTimeout: common.fail }; const bar = { diff --git a/test/parallel/test-tls-cipher-list.js b/test/parallel/test-tls-cipher-list.js index 9ae8fefa0f4351..f20a0a6a249aed 100644 --- a/test/parallel/test-tls-cipher-list.js +++ b/test/parallel/test-tls-cipher-list.js @@ -17,12 +17,12 @@ function doCheck(arg, check) { 'require("constants").defaultCipherList' ]); spawn(process.execPath, arg, {}). - on('error', assert.fail). + on('error', common.fail). stdout.on('data', function(chunk) { out += chunk; }).on('end', function() { assert.equal(out.trim(), check); - }).on('error', assert.fail); + }).on('error', common.fail); } // test the default unmodified version diff --git a/test/parallel/test-tls-no-sslv3.js b/test/parallel/test-tls-no-sslv3.js index 9777397758e3fb..ce5a9d395d6a47 100644 --- a/test/parallel/test-tls-no-sslv3.js +++ b/test/parallel/test-tls-no-sslv3.js @@ -18,7 +18,7 @@ if (common.opensslCli === false) { var cert = fs.readFileSync(common.fixturesDir + '/test_cert.pem'); var key = fs.readFileSync(common.fixturesDir + '/test_key.pem'); -var server = tls.createServer({ cert: cert, key: key }, assert.fail); +var server = tls.createServer({ cert: cert, key: key }, common.fail); server.listen(common.PORT, '127.0.0.1', function() { var address = this.address().address + ':' + this.address().port; diff --git a/test/parallel/test-tls-timeout-server.js b/test/parallel/test-tls-timeout-server.js index ee932c9b1fca6d..e3ed246386647f 100644 --- a/test/parallel/test-tls-timeout-server.js +++ b/test/parallel/test-tls-timeout-server.js @@ -23,7 +23,7 @@ var options = { handshakeTimeout: 50 }; -var server = tls.createServer(options, assert.fail); +var server = tls.createServer(options, common.fail); server.on('clientError', function(err, conn) { conn.destroy(); diff --git a/test/parallel/test-vm-debug-context.js b/test/parallel/test-vm-debug-context.js index 0f15d40ef14c59..2e86d136bea2fa 100644 --- a/test/parallel/test-vm-debug-context.js +++ b/test/parallel/test-vm-debug-context.js @@ -10,7 +10,7 @@ assert.throws(function() { }, /SyntaxError/); assert.throws(function() { - vm.runInDebugContext({ toString: assert.fail }); + vm.runInDebugContext({ toString: common.fail }); }, /AssertionError/); assert.throws(function() { @@ -58,7 +58,7 @@ assert.strictEqual(vm.runInDebugContext(undefined), undefined); var script = common.fixturesDir + '/vm-run-in-debug-context.js'; var proc = spawn(process.execPath, [script]); var data = []; -proc.stdout.on('data', assert.fail); +proc.stdout.on('data', common.fail); proc.stderr.on('data', data.push.bind(data)); proc.stderr.once('end', common.mustCall(function() { var haystack = Buffer.concat(data).toString('utf8'); @@ -70,8 +70,8 @@ proc.once('exit', common.mustCall(function(exitCode, signalCode) { })); var proc = spawn(process.execPath, [script, 'handle-fatal-exception']); -proc.stdout.on('data', assert.fail); -proc.stderr.on('data', assert.fail); +proc.stdout.on('data', common.fail); +proc.stderr.on('data', common.fail); proc.once('exit', common.mustCall(function(exitCode, signalCode) { assert.equal(exitCode, 42); assert.equal(signalCode, null); diff --git a/test/pummel/test-fs-watch-non-recursive.js b/test/pummel/test-fs-watch-non-recursive.js index 6adb193928e44f..2586aec59b2d94 100644 --- a/test/pummel/test-fs-watch-non-recursive.js +++ b/test/pummel/test-fs-watch-non-recursive.js @@ -19,7 +19,7 @@ try { fs.mkdirSync(testsubdir, 0o700); } catch (e) {} // Need a grace period, else the mkdirSync() above fires off an event. setTimeout(function() { - var watcher = fs.watch(testDir, { persistent: true }, assert.fail); + var watcher = fs.watch(testDir, { persistent: true }, common.fail); setTimeout(function() { fs.writeFileSync(filepath, 'test'); }, 100); diff --git a/test/sequential/test-cluster-listening-port.js b/test/sequential/test-cluster-listening-port.js index c9c65389036ec9..aaad1ff620b191 100644 --- a/test/sequential/test-cluster-listening-port.js +++ b/test/sequential/test-cluster-listening-port.js @@ -1,5 +1,5 @@ 'use strict'; -require('../common'); +var common = require('../common'); var assert = require('assert'); var cluster = require('cluster'); var net = require('net'); @@ -21,5 +21,5 @@ if (cluster.isMaster) { }); } else { - net.createServer(assert.fail).listen(0); + net.createServer(common.fail).listen(0); } From e99823bf5ac6a9690fcc2a3ab10a5de715c243c8 Mon Sep 17 00:00:00 2001 From: Imran Iqbal Date: Tue, 20 Oct 2015 16:58:16 -0400 Subject: [PATCH 23/51] test: fix test-net-keepalive for AIX Fixed an intermittent issue on AIX where the 100ms timeout was reached before the 'connection' event was fired. This resulted in a failure as serverConnection would be undefined and the assert.equal would throw an error. Changed the flow of the test so that the timeout is only set after a connection has been made. Reviewed-By: James M Snell Reviewed-By: Michael Dawson PR-URL: https://github.com/nodejs/node/pull/3458 --- test/parallel/test-net-keepalive.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/parallel/test-net-keepalive.js b/test/parallel/test-net-keepalive.js index efbbc5ea7986bb..b05537ba052e10 100644 --- a/test/parallel/test-net-keepalive.js +++ b/test/parallel/test-net-keepalive.js @@ -4,8 +4,17 @@ var assert = require('assert'); var net = require('net'); var serverConnection; +var clientConnection; var echoServer = net.createServer(function(connection) { serverConnection = connection; + setTimeout(function() { + // make sure both connections are still open + assert.equal(serverConnection.readyState, 'open'); + assert.equal(clientConnection.readyState, 'open'); + serverConnection.end(); + clientConnection.end(); + echoServer.close(); + }, common.platformTimeout(100)); connection.setTimeout(0); assert.notEqual(connection.setKeepAlive, undefined); // send a keepalive packet after 50 ms @@ -17,15 +26,6 @@ var echoServer = net.createServer(function(connection) { echoServer.listen(common.PORT); echoServer.on('listening', function() { - var clientConnection = net.createConnection(common.PORT); + clientConnection = net.createConnection(common.PORT); clientConnection.setTimeout(0); - - setTimeout(function() { - // make sure both connections are still open - assert.equal(serverConnection.readyState, 'open'); - assert.equal(clientConnection.readyState, 'open'); - serverConnection.end(); - clientConnection.end(); - echoServer.close(); - }, common.platformTimeout(100)); }); From 304eda46a4be3887fc589087e086b2e0f500fd1a Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Tue, 20 Oct 2015 18:35:34 -0400 Subject: [PATCH 24/51] test: harden test-child-process-fork-regr-gh-2847 test-child-process-fork-regr-gh-2847 could fail depending on timing and how messages were packed into tcp packets. If all of the requests fit into one packet then the test worked otherwise, otherwise errors could occur. This PR modifies the test to be tolerant while still validating that some of the connection can be made succesfully Reviewed-By: James M Snell PR-URL: https://github.com/nodejs/node/pull/3459 --- .../test-child-process-fork-regr-gh-2847.js | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/test/parallel/test-child-process-fork-regr-gh-2847.js b/test/parallel/test-child-process-fork-regr-gh-2847.js index 07ef6415a3c3e1..27b4d72d2fc612 100644 --- a/test/parallel/test-child-process-fork-regr-gh-2847.js +++ b/test/parallel/test-child-process-fork-regr-gh-2847.js @@ -7,6 +7,9 @@ const cluster = require('cluster'); const net = require('net'); const util = require('util'); +var connectcount = 0; +var sendcount = 0; + if (!cluster.isMaster) { // Exit on first received handle to leave the queue non-empty in master process.on('message', function() { @@ -25,6 +28,21 @@ var server = net.createServer(function(s) { function send(callback) { var s = net.connect(common.PORT, function() { worker.send({}, s, callback); + connectcount++; + }); + + // Errors can happen if the connections + // are still happening while the server has been closed. + // This can happen depending on how the messages are + // bundled into packets. If they all make it into the first + // one then no errors will occur, otherwise the server + // may have been closed by the time the later ones make + // it to the server side. + // We ignore any errors that occur after some connections + // get through + s.on('error', function(err) { + if (connectcount < 3) + console.log(err); }); } @@ -36,5 +54,9 @@ var server = net.createServer(function(s) { // Queue up several handles, to make `process.disconnect()` wait for (var i = 0; i < 100; i++) - send(); + send(function(err) { + if (err && sendcount < 3) + console.log(err); + sendcount++; + }); }); From bc2a80b493e1e6cbbacd78d3cc60df2da64967ae Mon Sep 17 00:00:00 2001 From: Michael Dawson Date: Thu, 22 Oct 2015 16:15:33 -0400 Subject: [PATCH 25/51] test: disable test-tick-processor - aix and be ppc This test is already partially disabled for several platforms with the comment that the required info is not provided at the C++ level. I'm adding AIX as and PPC BE linux as they currently fall into the same category. We are working to see if we can change that in v8 but it will be non-trivial if is possible at all so I don't want to leave the CI with failing tests until that point. PR-URL: https://github.com/nodejs/node/pull/3491 Reviewed-By: James M Snell Reviewed-By: Rich Trott --- test/common.js | 5 +++++ test/parallel/test-tick-processor.js | 8 +++++--- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/test/common.js b/test/common.js index a9641641ce50fc..750165ea8cc6c0 100644 --- a/test/common.js +++ b/test/common.js @@ -13,6 +13,11 @@ exports.tmpDirName = 'tmp'; exports.PORT = +process.env.NODE_COMMON_PORT || 12346; exports.isWindows = process.platform === 'win32'; exports.isAix = process.platform === 'aix'; +exports.isLinuxPPCBE = (process.platform === 'linux') && + (process.arch === 'ppc64') && + (os.endianness() === 'BE'); +exports.isSunOS = process.platform === 'sunos'; +exports.isFreeBSD = process.platform === 'freebsd'; function rimrafSync(p) { try { diff --git a/test/parallel/test-tick-processor.js b/test/parallel/test-tick-processor.js index cd110e1a87ed3d..65da7362d9a915 100644 --- a/test/parallel/test-tick-processor.js +++ b/test/parallel/test-tick-processor.js @@ -20,9 +20,11 @@ runTest(/LazyCompile.*\[eval\]:1|.*% UNKNOWN/, }; setTimeout(function() { process.exit(0); }, 2000); f();`); -if (process.platform === 'win32' || - process.platform === 'sunos' || - process.platform === 'freebsd') { +if (common.isWindows || + common.isSunOS || + common.isAix || + common.isLinuxPPCBE || + common.isFreeBSD) { console.log('1..0 # Skipped: C++ symbols are not mapped for this os.'); return; } From 287e8306fdf77268e447b30dc75f628976e02f81 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Fri, 23 Oct 2015 19:10:49 +0200 Subject: [PATCH 26/51] buffer: don't CHECK on zero-sized realloc malloc(0) and realloc(ptr, 0) have implementation-defined behavior in that the standard allows them to either return a unique pointer or a nullptr for zero-sized allocation requests. Normalize by always using a nullptr. Fixes: https://github.com/nodejs/node/issues/3496 PR-URL: https://github.com/nodejs/node/pull/3499 Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Trevor Norris --- src/node_buffer.cc | 30 +++++++++++++++++++++--------- test/parallel/test-buffer.js | 3 +++ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/src/node_buffer.cc b/src/node_buffer.cc index a472d0c95e9225..7d428b2b134b51 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -211,18 +211,30 @@ MaybeLocal New(Isolate* isolate, enum encoding enc) { EscapableHandleScope scope(isolate); - size_t length = StringBytes::Size(isolate, string, enc); - char* data = static_cast(malloc(length)); + const size_t length = StringBytes::Size(isolate, string, enc); + size_t actual = 0; + char* data = nullptr; + + // malloc(0) and realloc(ptr, 0) have implementation-defined behavior in + // that the standard allows them to either return a unique pointer or a + // nullptr for zero-sized allocation requests. Normalize by always using + // a nullptr. + if (length > 0) { + data = static_cast(malloc(length)); - if (data == nullptr) - return Local(); + if (data == nullptr) + return Local(); - size_t actual = StringBytes::Write(isolate, data, length, string, enc); - CHECK(actual <= length); + actual = StringBytes::Write(isolate, data, length, string, enc); + CHECK(actual <= length); - if (actual < length) { - data = static_cast(realloc(data, actual)); - CHECK_NE(data, nullptr); + if (actual == 0) { + free(data); + data = nullptr; + } else if (actual < length) { + data = static_cast(realloc(data, actual)); + CHECK_NE(data, nullptr); + } } Local buf; diff --git a/test/parallel/test-buffer.js b/test/parallel/test-buffer.js index 1be4f3b8424352..37c9e6431e9c86 100644 --- a/test/parallel/test-buffer.js +++ b/test/parallel/test-buffer.js @@ -550,6 +550,9 @@ for (var i = 0; i < segments.length; ++i) { } assert.equal(b.toString('binary', 0, pos), 'Madness?! This is node.js!'); +// Regression test for https://github.com/nodejs/node/issues/3496. +assert.equal(Buffer('=bad'.repeat(1e4), 'base64').length, 0); + // Creating buffers larger than pool size. var l = Buffer.poolSize + 5; var s = ''; From 6305d26f4085deb662b70c828e2de6a05a787bec Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Mon, 12 Oct 2015 09:50:56 -0700 Subject: [PATCH 27/51] test: add Symbol test for assert.deepEqual() The existing test never ran because typeof Symbol === 'function' and not 'symbol'. We have Symbols now so remove the check and just run the test. PR-URL: https://github.com/nodejs/node/pull/3327 Reviewed-By: James M Snell --- test/parallel/test-assert.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test/parallel/test-assert.js b/test/parallel/test-assert.js index ce19462a62387c..db36abc184b024 100644 --- a/test/parallel/test-assert.js +++ b/test/parallel/test-assert.js @@ -140,9 +140,7 @@ assert.throws(makeBlock(a.deepEqual, 'a', ['a']), a.AssertionError); assert.throws(makeBlock(a.deepEqual, 'a', {0: 'a'}), a.AssertionError); assert.throws(makeBlock(a.deepEqual, 1, {}), a.AssertionError); assert.throws(makeBlock(a.deepEqual, true, {}), a.AssertionError); -if (typeof Symbol === 'symbol') { - assert.throws(makeBlock(assert.deepEqual, Symbol(), {}), a.AssertionError); -} +assert.throws(makeBlock(a.deepEqual, Symbol(), {}), a.AssertionError); // primitive wrappers and object assert.doesNotThrow(makeBlock(a.deepEqual, new String('a'), ['a']), From 9fa8c32d357033ebe48115ed18a19cc0fc3f8bc7 Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Tue, 6 Oct 2015 23:00:31 -0700 Subject: [PATCH 28/51] test: cleanup, improve repl-persistent-history - Now cleans up the history file unless told otherwise. - Now also logs which test case failed. - Waits for flush after repl close if necessary. Fixes: https://github.com/nodejs/node/issues/2319 PR-URL: https://github.com/nodejs/node/pull/2356 Reviewed-By: Roman Reiss Reviewed By: Evan Lucas --- .../test-repl-persistent-history.js | 83 +++++++++++++++---- 1 file changed, 68 insertions(+), 15 deletions(-) diff --git a/test/sequential/test-repl-persistent-history.js b/test/sequential/test-repl-persistent-history.js index ef433912da5a65..7cebddbd6c20f5 100644 --- a/test/sequential/test-repl-persistent-history.js +++ b/test/sequential/test-repl-persistent-history.js @@ -70,6 +70,7 @@ const historyFixturePath = path.join(fixtures, '.node_repl_history'); const historyPath = path.join(common.tmpDir, '.fixture_copy_repl_history'); const oldHistoryPath = path.join(fixtures, 'old-repl-history-file.json'); const enoentHistoryPath = path.join(fixtures, 'enoent-repl-history-file.json'); +const defaultHistoryPath = path.join(common.tmpDir, '.node_repl_history'); const tests = [{ @@ -113,11 +114,7 @@ const tests = [{ }, { env: { NODE_REPL_HISTORY_FILE: oldHistoryPath }, - test: [UP, CLEAR, '\'42\'', ENTER/*, function(cb) { - // XXX(Fishrock123) Allow the REPL to save to disk. - // There isn't a way to do this programmatically right now. - setTimeout(cb, 50); - }*/], + test: [UP, CLEAR, '\'42\'', ENTER], expected: [prompt, convertMsg, prompt, prompt + '\'=^.^=\'', prompt, '\'', '4', '2', '\'', '\'42\'\n', prompt, prompt], after: function ensureHistoryFixture() { @@ -132,7 +129,7 @@ const tests = [{ '\'Stay Fresh~\'' + os.EOL); } }, -{ +{ // Requires the above testcase env: {}, test: [UP, UP, ENTER], expected: [prompt, prompt + '\'42\'', prompt + '\'=^.^=\'', '\'=^.^=\'\n', @@ -149,16 +146,45 @@ const tests = [{ test: [UP], expected: [prompt, homedirErr, prompt, replDisabled, prompt] }]; +const numtests = tests.length; + + +var testsNotRan = tests.length; +process.on('beforeExit', () => + assert.strictEqual(testsNotRan, 0) +); + +function cleanupTmpFile() { + try { + // Write over the file, clearing any history + fs.writeFileSync(defaultHistoryPath, ''); + } catch (err) { + if (err.code === 'ENOENT') return true; + throw err; + } + return true; +} // Copy our fixture to the tmp directory fs.createReadStream(historyFixturePath) - .pipe(fs.createWriteStream(historyPath)).on('unpipe', runTest); + .pipe(fs.createWriteStream(historyPath)).on('unpipe', () => runTest()); -function runTest() { +function runTest(assertCleaned) { const opts = tests.shift(); if (!opts) return; // All done + if (assertCleaned) { + try { + assert.strictEqual(fs.readFileSync(defaultHistoryPath, 'utf8'), ''); + } catch (e) { + if (e.code !== 'ENOENT') { + console.error(`Failed test # ${numtests - tests.length}`); + throw e; + } + } + } + const env = opts.env; const test = opts.test; const expected = opts.expected; @@ -177,7 +203,12 @@ function runTest() { if (output.charCodeAt(0) === 27 || /^[\r\n]+$/.test(output)) return next(); - assert.strictEqual(output, expected.shift()); + try { + assert.strictEqual(output, expected.shift()); + } catch (err) { + console.error(`Failed test # ${numtests - tests.length}`); + throw err; + } next(); } }), @@ -185,16 +216,38 @@ function runTest() { useColors: false, terminal: true }, function(err, repl) { - if (err) throw err; + if (err) { + console.error(`Failed test # ${numtests - tests.length}`); + throw err; + } - if (after) repl.on('close', after); + repl.once('close', () => { + if (repl._flushing) { + repl.once('flushHistory', onClose); + return; + } - repl.on('close', function() { - // Ensure everything that we expected was output - assert.strictEqual(expected.length, 0); - setImmediate(runTest); + onClose(); }); + function onClose() { + if (after) { + var cleaned = after(); + } else { + var cleaned = cleanupTmpFile(); + } + + try { + // Ensure everything that we expected was output + assert.strictEqual(expected.length, 0); + testsNotRan--; + setImmediate(runTest, cleaned); + } catch (err) { + console.error(`Failed test # ${numtests - tests.length}`); + throw err; + } + } + repl.inputStream.run(test); }); } From 594e8e7a54d5002d7b8d4a170e6e17aaf5589fff Mon Sep 17 00:00:00 2001 From: Jeremiah Senkpiel Date: Tue, 6 Oct 2015 23:01:28 -0700 Subject: [PATCH 29/51] repl: limit persistent history correctly on load Previously the wrong end of the history was limited on load. PR-URL: https://github.com/nodejs/node/pull/2356 Reviewed-By: Roman Reiss Reviewed By: Evan Lucas --- lib/internal/repl.js | 4 ++-- test/sequential/test-repl-persistent-history.js | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/internal/repl.js b/lib/internal/repl.js index c1beb85cefd795..0318a36098e31a 100644 --- a/lib/internal/repl.js +++ b/lib/internal/repl.js @@ -110,7 +110,7 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) { } if (data) { - repl.history = data.split(/[\n\r]+/).slice(-repl.historySize); + repl.history = data.split(/[\n\r]+/, repl.historySize); } else if (oldHistoryPath) { // Grab data from the older pre-v3.0 JSON NODE_REPL_HISTORY_FILE format. repl._writeToOutput( @@ -123,7 +123,7 @@ function setupHistory(repl, historyPath, oldHistoryPath, ready) { if (!Array.isArray(repl.history)) { throw new Error('Expected array, got ' + typeof repl.history); } - repl.history = repl.history.slice(-repl.historySize); + repl.history = repl.history.slice(0, repl.historySize); } catch (err) { if (err.code !== 'ENOENT') { return ready( diff --git a/test/sequential/test-repl-persistent-history.js b/test/sequential/test-repl-persistent-history.js index 7cebddbd6c20f5..7fd68d7d764a4f 100644 --- a/test/sequential/test-repl-persistent-history.js +++ b/test/sequential/test-repl-persistent-history.js @@ -135,6 +135,18 @@ const tests = [{ expected: [prompt, prompt + '\'42\'', prompt + '\'=^.^=\'', '\'=^.^=\'\n', prompt] }, +{ + env: { NODE_REPL_HISTORY: historyPath, + NODE_REPL_HISTORY_SIZE: 1 }, + test: [UP, UP, CLEAR], + expected: [prompt, prompt + '\'you look fabulous today\'', prompt] +}, +{ + env: { NODE_REPL_HISTORY_FILE: oldHistoryPath, + NODE_REPL_HISTORY_SIZE: 1 }, + test: [UP, UP, UP, CLEAR], + expected: [prompt, convertMsg, prompt, prompt + '\'=^.^=\'', prompt] +}, { // Make sure this is always the last test, since we change os.homedir() before: function mockHomedirFailure() { // Mock os.homedir() failure From fb3b8489de0a32e83921c825197334787d979b99 Mon Sep 17 00:00:00 2001 From: ronkorving Date: Fri, 18 Sep 2015 17:36:57 +0900 Subject: [PATCH 30/51] fs: reduced duplicate code in fs.write() PR-URL: https://github.com/nodejs/node/pull/2947 Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Trevor Norris --- lib/fs.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index fef4f7ba501b50..cd39cc9d83fcc1 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -614,17 +614,14 @@ fs.readSync = function(fd, buffer, offset, length, position) { // OR // fs.write(fd, string[, position[, encoding]], callback); fs.write = function(fd, buffer, offset, length, position, callback) { - function strWrapper(err, written) { + function wrapper(err, written) { // Retain a reference to buffer so that it can't be GC'ed too soon. callback(err, written || 0, buffer); } - function bufWrapper(err, written) { - // retain reference to string in case it's external - callback(err, written || 0, buffer); - } - var req = new FSReqWrap(); + req.oncomplete = wrapper; + if (buffer instanceof Buffer) { // if no position is passed then assume null if (typeof position === 'function') { @@ -632,7 +629,6 @@ fs.write = function(fd, buffer, offset, length, position, callback) { position = null; } callback = maybeCallback(callback); - req.oncomplete = strWrapper; return binding.writeBuffer(fd, buffer, offset, length, position, req); } @@ -648,7 +644,6 @@ fs.write = function(fd, buffer, offset, length, position, callback) { length = 'utf8'; } callback = maybeCallback(position); - req.oncomplete = bufWrapper; return binding.writeString(fd, buffer, offset, length, req); }; From 5222fcc11703b7d5acdfda32a329a5db4c640594 Mon Sep 17 00:00:00 2001 From: Julien Gilli Date: Tue, 13 Oct 2015 17:07:35 -0700 Subject: [PATCH 31/51] test: fix domain with abort-on-uncaught on PPC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add SIGTRAP and the corresponding exit code to the list of signals/exit codes that are expected when running tests that throw an uncaught error and have --abort-on-uncaught-exception enabled. Also refactor a bit related comments so that they better reflect what's actually happening. Fixes #3239. PR: #3354 PR-URL: https://github.com/nodejs/node/pull/3354 Reviewed-By: Colin Ihrig Reviewed-By: Johan Bergström --- ...domain-with-abort-on-uncaught-exception.js | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/test/parallel/test-domain-with-abort-on-uncaught-exception.js b/test/parallel/test-domain-with-abort-on-uncaught-exception.js index 9892ad6e75ba44..bef952b84354a2 100644 --- a/test/parallel/test-domain-with-abort-on-uncaught-exception.js +++ b/test/parallel/test-domain-with-abort-on-uncaught-exception.js @@ -135,20 +135,16 @@ if (process.argv[2] === 'child') { // --abort_on_uncaught_exception is passed on the command line, // the process must abort. // - // We use an array of values since the actual exit code can differ - // across compilers. // Depending on the compiler used, node will exit with either - // exit code 132 (SIGILL) or 134 (SIGABRT). - expectedExitCodes = [132, 134]; - - // On platforms using a non-GNU compiler, base::OS::Abort raises - // an illegal instruction signal. - // On platforms using a GNU compiler but with KSH being the - // default shell (like SmartOS), when a process aborts, KSH exits - // with an exit code that is greater than 256, and thus the exit - // code emitted with the 'exit' event is null and the signal is - // set to either SIGABRT or SIGILL. - expectedSignals = ['SIGABRT', 'SIGILL']; + // exit code 132 (SIGILL), 133 (SIGTRAP) or 134 (SIGABRT). + expectedExitCodes = [132, 133, 134]; + + // On platforms using KSH as the default shell (like SmartOS), + // when a process aborts, KSH exits with an exit code that is + // greater than 256, and thus the exit code emitted with the 'exit' + // event is null and the signal is set to either SIGILL, SIGTRAP, + // or SIGABRT (depending on the compiler). + expectedSignals = ['SIGILL', 'SIGTRAP', 'SIGABRT']; // On Windows, v8's base::OS::Abort triggers an access violation, // which corresponds to exit code 3221225477 (0xC0000005) From bc2120caa17e6f913c73f89504126af2f630259f Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Fri, 23 Oct 2015 12:34:11 -0600 Subject: [PATCH 32/51] buffer: fix value check for writeUInt{B,L}E Fixes: https://github.com/nodejs/node/issues/3497 PR-URL: https://github.com/nodejs/node/pull/3500 Reviewed-By: Ben Noordhuis --- lib/buffer.js | 12 ++++++++---- test/parallel/test-writeuint.js | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/lib/buffer.js b/lib/buffer.js index 0fdfcecfdf9360..52b362f1a43c22 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -836,8 +836,10 @@ Buffer.prototype.writeUIntLE = function(value, offset, byteLength, noAssert) { value = +value; offset = offset >>> 0; byteLength = byteLength >>> 0; - if (!noAssert) - checkInt(this, value, offset, byteLength, Math.pow(2, 8 * byteLength), 0); + if (!noAssert) { + const maxBytes = Math.pow(2, 8 * byteLength) - 1; + checkInt(this, value, offset, byteLength, maxBytes, 0); + } var mul = 1; var i = 0; @@ -853,8 +855,10 @@ Buffer.prototype.writeUIntBE = function(value, offset, byteLength, noAssert) { value = +value; offset = offset >>> 0; byteLength = byteLength >>> 0; - if (!noAssert) - checkInt(this, value, offset, byteLength, Math.pow(2, 8 * byteLength), 0); + if (!noAssert) { + const maxBytes = Math.pow(2, 8 * byteLength) - 1; + checkInt(this, value, offset, byteLength, maxBytes, 0); + } var i = byteLength - 1; var mul = 1; diff --git a/test/parallel/test-writeuint.js b/test/parallel/test-writeuint.js index 22579ceecaef2d..baa12aa57666f3 100644 --- a/test/parallel/test-writeuint.js +++ b/test/parallel/test-writeuint.js @@ -122,6 +122,25 @@ function test32(clazz) { } +function testUint(clazz) { + const data = new clazz(8); + var val = 1; + + // Test 0 to 5 bytes. + for (var i = 0; i <= 5; i++) { + const errmsg = `byteLength: ${i}`; + ASSERT.throws(function() { + data.writeUIntBE(val, 0, i); + }, /value is out of bounds/, errmsg); + ASSERT.throws(function() { + data.writeUIntLE(val, 0, i); + }, /value is out of bounds/, errmsg); + val *= 0x100; + } +} + + test8(Buffer); test16(Buffer); test32(Buffer); +testUint(Buffer); From 9e05af7d3108a3d7b00a4b3802c06c4cabc41d88 Mon Sep 17 00:00:00 2001 From: Junliang Yan Date: Fri, 23 Oct 2015 12:48:36 -0400 Subject: [PATCH 33/51] test: print helpful err msg on test-dns-ipv6.js The test sometimes fail on an assertion but no useful error message was generated for debugging. Modify the test to generate useful debugging message. PR-URL: https://github.com/nodejs/node/pull/3501 Reviewed-By: Trevor Norris Reviewed-By: Rich Trott --- test/internet/test-dns-ipv6.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/internet/test-dns-ipv6.js b/test/internet/test-dns-ipv6.js index 5a66f8db783f4b..27547edcd84b46 100644 --- a/test/internet/test-dns-ipv6.js +++ b/test/internet/test-dns-ipv6.js @@ -165,7 +165,8 @@ TEST(function test_lookup_all_ipv6(done) { assert.ok(ips.length > 0); ips.forEach(function(ip) { - assert.ok(isIPv6(ip.address)); + assert.ok(isIPv6(ip.address), + 'Invalid IPv6: ' + ip.address.toString()); assert.strictEqual(ip.family, 6); }); From 12d83858f7b124ebff2c6a57f106219fe81f4c63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sat, 24 Oct 2015 10:42:29 +0200 Subject: [PATCH 34/51] test: improve tests for util.inherits inherits is used in lib and tests but its functionality itself is not tested yet. PR-URL: https://github.com/nodejs/node/pull/3507 Reviewed-By: Sakthipriyan Vairamani --- test/parallel/test-util-inherits.js | 46 +++++++++++++++++++++++++++++ test/parallel/test-util.js | 7 ----- 2 files changed, 46 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-util-inherits.js diff --git a/test/parallel/test-util-inherits.js b/test/parallel/test-util-inherits.js new file mode 100644 index 00000000000000..baa77ea4405484 --- /dev/null +++ b/test/parallel/test-util-inherits.js @@ -0,0 +1,46 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); +const inherits = require('util').inherits; + +// super constructor +function A() { + this._a = 'a'; +} +A.prototype.a = function() { return this._a; }; + +// one level of inheritance +function B(value) { + A.call(this); + this._b = value; +} +inherits(B, A); +B.prototype.b = function() { return this._b; }; + +assert.strictEqual(B.super_, A); + +const b = new B('b'); +assert.strictEqual(b.a(), 'a'); +assert.strictEqual(b.b(), 'b'); +assert.strictEqual(b.constructor, B); + + // two levels of inheritance +function C() { + B.call(this, 'b'); + this._c = 'c'; +} +inherits(C, B); +C.prototype.c = function() { return this._c; }; +C.prototype.getValue = function() { return this.a() + this.b() + this.c(); }; + +assert.strictEqual(C.super_, B); + +const c = new C(); +assert.strictEqual(c.getValue(), 'abc'); +assert.strictEqual(c.constructor, C); + +// should throw with invalid arguments +assert.throws(function() { inherits(A, {}); }, TypeError); +assert.throws(function() { inherits(A, null); }, TypeError); +assert.throws(function() { inherits(null, A); }, TypeError); diff --git a/test/parallel/test-util.js b/test/parallel/test-util.js index 79e104546b7f37..700532bd5a397b 100644 --- a/test/parallel/test-util.js +++ b/test/parallel/test-util.js @@ -83,10 +83,3 @@ assert.deepEqual(util._extend({a:1}, true), {a:1}); assert.deepEqual(util._extend({a:1}, false), {a:1}); assert.deepEqual(util._extend({a:1}, {b:2}), {a:1, b:2}); assert.deepEqual(util._extend({a:1, b:2}, {b:3}), {a:1, b:3}); - -// inherits -var ctor = function() {}; -assert.throws(function() { util.inherits(ctor, {}); }, TypeError); -assert.throws(function() { util.inherits(ctor, null); }, TypeError); -assert.throws(function() { util.inherits(null, ctor); }, TypeError); -assert.doesNotThrow(function() { util.inherits(ctor, ctor); }, TypeError); From e4417a1bfa823a622ee7aba8240a392b678d1753 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 24 Oct 2015 11:51:10 -0700 Subject: [PATCH 35/51] lib: fix cluster handle leak It is possible to cause a resource leak in SharedHandle. This commit fixes the leak. Fixes: https://github.com/nodejs/node/issues/2510 PR-URL: https://github.com/nodejs/node/pull/3510 Reviewed-By: Ben Noordhuis --- lib/cluster.js | 5 ++- test/parallel/test-cluster-shared-leak.js | 53 +++++++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 test/parallel/test-cluster-shared-leak.js diff --git a/lib/cluster.js b/lib/cluster.js index 602cc8d60b9200..eef8bd25639dd2 100644 --- a/lib/cluster.js +++ b/lib/cluster.js @@ -345,7 +345,10 @@ function masterInit() { * if it has disconnected, otherwise we might * still want to access it. */ - if (!worker.isConnected()) removeWorker(worker); + if (!worker.isConnected()) { + removeHandlesForWorker(worker); + removeWorker(worker); + } worker.suicide = !!worker.suicide; worker.state = 'dead'; diff --git a/test/parallel/test-cluster-shared-leak.js b/test/parallel/test-cluster-shared-leak.js new file mode 100644 index 00000000000000..a4de1d33a29b8d --- /dev/null +++ b/test/parallel/test-cluster-shared-leak.js @@ -0,0 +1,53 @@ +// In Node 4.2.1 on operating systems other than Linux, this test triggers an +// assertion in cluster.js. The assertion protects against memory leaks. +// https://github.com/nodejs/node/pull/3510 + +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const net = require('net'); +const cluster = require('cluster'); +cluster.schedulingPolicy = cluster.SCHED_NONE; + +if (cluster.isMaster) { + var conn, worker1, worker2; + + worker1 = cluster.fork(); + worker1.on('message', common.mustCall(function() { + worker2 = cluster.fork(); + conn = net.connect(common.PORT, common.mustCall(function() { + worker1.send('die'); + worker2.send('die'); + })); + conn.on('error', function(e) { + // ECONNRESET is OK + if (e.code !== 'ECONNRESET') + throw e; + }); + })); + + cluster.on('exit', function(worker, exitCode, signalCode) { + assert(worker === worker1 || worker === worker2); + assert.strictEqual(exitCode, 0); + assert.strictEqual(signalCode, null); + if (Object.keys(cluster.workers).length === 0) + conn.destroy(); + }); + + return; +} + +var server = net.createServer(function(c) { + c.end('bye'); +}); + +server.listen(common.PORT, function() { + process.send('listening'); +}); + +process.on('message', function(msg) { + if (msg !== 'die') return; + server.close(function() { + setImmediate(() => process.disconnect()); + }); +}); From 9061aa2a75c1f230adc01ce24c7314f10f5ba0b5 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Tue, 27 Oct 2015 12:54:42 -0400 Subject: [PATCH 36/51] deps: backport 8d6a228 from the v8's upstream Original commit message: [heap] fix crash during the scavenge of ArrayBuffer Scavenger should not attempt to visit ArrayBuffer's storage, it is a user-supplied pointer that may have any alignment. Visiting it, may result in a crash. BUG= R=jochen Review URL: https://codereview.chromium.org/1406133003 Cr-Commit-Position: refs/heads/master@{#31611} PR-URL: https://github.com/nodejs/node/pull/3549 Reviewed-By: Trevor Norris --- deps/v8/src/heap/heap.cc | 97 +++++++++++++++++++++------------ deps/v8/src/heap/heap.h | 3 + deps/v8/test/cctest/test-api.cc | 26 +++++++++ 3 files changed, 92 insertions(+), 34 deletions(-) diff --git a/deps/v8/src/heap/heap.cc b/deps/v8/src/heap/heap.cc index 5bcc9097ee3369..6bc200a0e59289 100644 --- a/deps/v8/src/heap/heap.cc +++ b/deps/v8/src/heap/heap.cc @@ -2079,40 +2079,8 @@ Address Heap::DoScavenge(ObjectVisitor* scavenge_visitor, // for pointers to from semispace instead of looking for pointers // to new space. DCHECK(!target->IsMap()); - Address obj_address = target->address(); - - // We are not collecting slots on new space objects during mutation - // thus we have to scan for pointers to evacuation candidates when we - // promote objects. But we should not record any slots in non-black - // objects. Grey object's slots would be rescanned. - // White object might not survive until the end of collection - // it would be a violation of the invariant to record it's slots. - bool record_slots = false; - if (incremental_marking()->IsCompacting()) { - MarkBit mark_bit = Marking::MarkBitFrom(target); - record_slots = Marking::IsBlack(mark_bit); - } -#if V8_DOUBLE_FIELDS_UNBOXING - LayoutDescriptorHelper helper(target->map()); - bool has_only_tagged_fields = helper.all_fields_tagged(); - - if (!has_only_tagged_fields) { - for (int offset = 0; offset < size;) { - int end_of_region_offset; - if (helper.IsTagged(offset, size, &end_of_region_offset)) { - IterateAndMarkPointersToFromSpace( - record_slots, obj_address + offset, - obj_address + end_of_region_offset, &ScavengeObject); - } - offset = end_of_region_offset; - } - } else { -#endif - IterateAndMarkPointersToFromSpace( - record_slots, obj_address, obj_address + size, &ScavengeObject); -#if V8_DOUBLE_FIELDS_UNBOXING - } -#endif + + IteratePointersToFromSpace(target, size, &ScavengeObject); } } @@ -5263,6 +5231,67 @@ void Heap::IterateAndMarkPointersToFromSpace(bool record_slots, Address start, } +void Heap::IteratePointersToFromSpace(HeapObject* target, int size, + ObjectSlotCallback callback) { + Address obj_address = target->address(); + + // We are not collecting slots on new space objects during mutation + // thus we have to scan for pointers to evacuation candidates when we + // promote objects. But we should not record any slots in non-black + // objects. Grey object's slots would be rescanned. + // White object might not survive until the end of collection + // it would be a violation of the invariant to record it's slots. + bool record_slots = false; + if (incremental_marking()->IsCompacting()) { + MarkBit mark_bit = Marking::MarkBitFrom(target); + record_slots = Marking::IsBlack(mark_bit); + } + + // Do not scavenge JSArrayBuffer's contents + switch (target->ContentType()) { + case HeapObjectContents::kTaggedValues: { + IterateAndMarkPointersToFromSpace(record_slots, obj_address, + obj_address + size, callback); + break; + } + case HeapObjectContents::kMixedValues: { + if (target->IsFixedTypedArrayBase()) { + IterateAndMarkPointersToFromSpace( + record_slots, obj_address + FixedTypedArrayBase::kBasePointerOffset, + obj_address + FixedTypedArrayBase::kHeaderSize, callback); + } else if (target->IsJSArrayBuffer()) { + IterateAndMarkPointersToFromSpace( + record_slots, obj_address, + obj_address + JSArrayBuffer::kByteLengthOffset + kPointerSize, + callback); + IterateAndMarkPointersToFromSpace( + record_slots, obj_address + JSArrayBuffer::kSize, + obj_address + size, callback); +#if V8_DOUBLE_FIELDS_UNBOXING + } else if (FLAG_unbox_double_fields) { + LayoutDescriptorHelper helper(target->map()); + DCHECK(!helper.all_fields_tagged()); + + for (int offset = 0; offset < size;) { + int end_of_region_offset; + if (helper.IsTagged(offset, size, &end_of_region_offset)) { + IterateAndMarkPointersToFromSpace( + record_slots, obj_address + offset, + obj_address + end_of_region_offset, callback); + } + offset = end_of_region_offset; + } +#endif + } + break; + } + case HeapObjectContents::kRawValues: { + break; + } + } +} + + void Heap::IterateRoots(ObjectVisitor* v, VisitMode mode) { IterateStrongRoots(v, mode); IterateWeakRoots(v, mode); diff --git a/deps/v8/src/heap/heap.h b/deps/v8/src/heap/heap.h index 0f0cfc15fca472..0afac311c4816e 100644 --- a/deps/v8/src/heap/heap.h +++ b/deps/v8/src/heap/heap.h @@ -953,6 +953,9 @@ class Heap { // Iterate pointers to from semispace of new space found in memory interval // from start to end. + void IteratePointersToFromSpace(HeapObject* target, int size, + ObjectSlotCallback callback); + void IterateAndMarkPointersToFromSpace(bool record_slots, Address start, Address end, ObjectSlotCallback callback); diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index 88d4aef25e23b7..058241dfff7c89 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -14191,6 +14191,32 @@ THREADED_TEST(SkipArrayBufferBackingStoreDuringGC) { } +THREADED_TEST(SkipArrayBufferDuringScavenge) { + LocalContext env; + v8::Isolate* isolate = env->GetIsolate(); + v8::HandleScope handle_scope(isolate); + + // Make sure the pointer looks like a heap object + Local tmp = v8::Object::New(isolate); + uint8_t* store_ptr = + reinterpret_cast(*reinterpret_cast(*tmp)); + + // Make `store_ptr` point to from space + CcTest::heap()->CollectGarbage(i::NEW_SPACE); + + // Create ArrayBuffer with pointer-that-cannot-be-visited in the backing store + Local ab = v8::ArrayBuffer::New(isolate, store_ptr, 8); + + // Should not crash, + // i.e. backing store pointer should not be treated as a heap object pointer + CcTest::heap()->CollectGarbage(i::NEW_SPACE); // in survivor space now + CcTest::heap()->CollectGarbage(i::NEW_SPACE); // in old gen now + + // Use `ab` to silence compiler warning + CHECK_EQ(ab->GetContents().Data(), store_ptr); +} + + THREADED_TEST(SharedUint8Array) { i::FLAG_harmony_sharedarraybuffer = true; TypedArrayTestHelper Date: Thu, 10 Sep 2015 00:41:15 +0800 Subject: [PATCH 37/51] src: fix stuck debugger process The debug process running "node debug a.js" will be stuck when the script ends. This is because the debug handler has been unrefed. We shouldn't unref the debug handler to avoid this problem. PR-URL: https://github.com/nodejs/node/pull/2778 Reviewed-By: Ben Noordhuis --- src/node.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/node.cc b/src/node.cc index b80996e233a760..8766e3a1430f90 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3669,7 +3669,6 @@ void Init(int* argc, uv_async_init(uv_default_loop(), &dispatch_debug_messages_async, DispatchDebugMessagesAsyncCallback); - uv_unref(reinterpret_cast(&dispatch_debug_messages_async)); #if defined(NODE_V8_OPTIONS) // Should come before the call to V8::SetFlagsFromCommandLine() @@ -3976,8 +3975,11 @@ static void StartNodeInstance(void* arg) { env->set_trace_sync_io(trace_sync_io); // Enable debugger - if (instance_data->use_debug_agent()) + if (instance_data->use_debug_agent()) { EnableDebug(env); + } else { + uv_unref(reinterpret_cast(&dispatch_debug_messages_async)); + } { SealHandleScope seal(isolate); From 5bb6e0f64dc30364d406f7fbff037b66ec4edf9b Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sat, 26 Sep 2015 16:29:41 -0700 Subject: [PATCH 38/51] test: change call to deprecated util.isError() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit common.Error() is just util.isError() which is deprecated. Use assert.throws() instead. PR-URL: https://github.com/nodejs/node/pull/3084 Reviewed-By: Michaël Zasso Reviewed-By: Сковорода Никита Андреевич --- test/parallel/test-tty-stdout-end.js | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/test/parallel/test-tty-stdout-end.js b/test/parallel/test-tty-stdout-end.js index 190acae42870a9..57f9c03ec449e9 100644 --- a/test/parallel/test-tty-stdout-end.js +++ b/test/parallel/test-tty-stdout-end.js @@ -1,16 +1,15 @@ 'use strict'; // Can't test this when 'make test' doesn't assign a tty to the stdout. -var common = require('../common'); -var assert = require('assert'); +const common = require('../common'); +const assert = require('assert'); -var exceptionCaught = false; - -try { +const shouldThrow = function() { process.stdout.end(); -} catch (e) { - exceptionCaught = true; - assert.ok(common.isError(e)); - assert.equal('process.stdout cannot be closed.', e.message); -} +}; + +const validateError = function(e) { + return e instanceof Error && + e.message === 'process.stdout cannot be closed.'; +}; -assert.ok(exceptionCaught); +assert.throws(shouldThrow, validateError); From c76ef6c20cd4f8077b18f69e5c9e69b4c568e75c Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Thu, 8 Oct 2015 13:56:34 -0700 Subject: [PATCH 39/51] test: parallelize long-running test Fixes a persistently troublesome failing test by splitting it out into multiple parallel tests. Reviewed By: Evan Lucas Reviewed By: Trevor Norris Reviewed By: James M Snell PR-URL: https://github.com/nodejs/node/pull/3287 --- .../test-stringbytes-external-at-max.js | 22 ++++++ ...st-stringbytes-external-exceed-max-by-1.js | 52 ++++++++++++++ ...st-stringbytes-external-exceed-max-by-2.js | 22 ++++++ .../test-stringbytes-external-exceed-max.js | 23 +++++++ test/parallel/test-stringbytes-external.js | 69 ------------------- 5 files changed, 119 insertions(+), 69 deletions(-) create mode 100644 test/parallel/test-stringbytes-external-at-max.js create mode 100644 test/parallel/test-stringbytes-external-exceed-max-by-1.js create mode 100644 test/parallel/test-stringbytes-external-exceed-max-by-2.js create mode 100644 test/parallel/test-stringbytes-external-exceed-max.js diff --git a/test/parallel/test-stringbytes-external-at-max.js b/test/parallel/test-stringbytes-external-at-max.js new file mode 100644 index 00000000000000..6678e5355224b3 --- /dev/null +++ b/test/parallel/test-stringbytes-external-at-max.js @@ -0,0 +1,22 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +// v8 fails silently if string length > v8::String::kMaxLength +// v8::String::kMaxLength defined in v8.h +const kStringMaxLength = process.binding('buffer').kStringMaxLength; + +try { + new Buffer(kStringMaxLength * 3); +} catch(e) { + assert.equal(e.message, 'Invalid array buffer length'); + console.log( + '1..0 # Skipped: intensive toString tests due to memory confinements'); + return; +} + +const buf = new Buffer(kStringMaxLength); + +const maxString = buf.toString('binary'); +assert.equal(maxString.length, kStringMaxLength); diff --git a/test/parallel/test-stringbytes-external-exceed-max-by-1.js b/test/parallel/test-stringbytes-external-exceed-max-by-1.js new file mode 100644 index 00000000000000..8e2a5bf01f2596 --- /dev/null +++ b/test/parallel/test-stringbytes-external-exceed-max-by-1.js @@ -0,0 +1,52 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +// v8 fails silently if string length > v8::String::kMaxLength +// v8::String::kMaxLength defined in v8.h +const kStringMaxLength = process.binding('buffer').kStringMaxLength; + +try { + new Buffer(kStringMaxLength * 3); +} catch(e) { + assert.equal(e.message, 'Invalid array buffer length'); + console.log( + '1..0 # Skipped: intensive toString tests due to memory confinements'); + return; +} + +const buf1 = new Buffer(kStringMaxLength + 1); + +assert.throws(function() { + buf1.toString(); +}, /toString failed|Invalid array buffer length/); + +assert.throws(function() { + buf1.toString('ascii'); +}, /toString failed/); + +assert.throws(function() { + buf1.toString('utf8'); +}, /toString failed/); + +assert.throws(function() { + buf1.toString('binary'); +}, /toString failed/); + +assert.throws(function() { + buf1.toString('base64'); +}, /toString failed/); + +assert.throws(function() { + buf1.toString('hex'); +}, /toString failed/); + +var maxString = buf1.toString('binary', 1); +assert.equal(maxString.length, kStringMaxLength); +maxString = undefined; + +maxString = buf1.toString('binary', 0, kStringMaxLength); +assert.equal(maxString.length, kStringMaxLength); +// Free the memory early instead of at the end of the next assignment +maxString = undefined; diff --git a/test/parallel/test-stringbytes-external-exceed-max-by-2.js b/test/parallel/test-stringbytes-external-exceed-max-by-2.js new file mode 100644 index 00000000000000..879e4ae91dfb60 --- /dev/null +++ b/test/parallel/test-stringbytes-external-exceed-max-by-2.js @@ -0,0 +1,22 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +// v8 fails silently if string length > v8::String::kMaxLength +// v8::String::kMaxLength defined in v8.h +const kStringMaxLength = process.binding('buffer').kStringMaxLength; + +try { + new Buffer(kStringMaxLength * 3); +} catch(e) { + assert.equal(e.message, 'Invalid array buffer length'); + console.log( + '1..0 # Skipped: intensive toString tests due to memory confinements'); + return; +} + +const buf2 = new Buffer(kStringMaxLength + 2); + +const maxString = buf2.toString('utf16le'); +assert.equal(maxString.length, (kStringMaxLength + 2) / 2); diff --git a/test/parallel/test-stringbytes-external-exceed-max.js b/test/parallel/test-stringbytes-external-exceed-max.js new file mode 100644 index 00000000000000..0a6f585b66f3ad --- /dev/null +++ b/test/parallel/test-stringbytes-external-exceed-max.js @@ -0,0 +1,23 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +// v8 fails silently if string length > v8::String::kMaxLength +// v8::String::kMaxLength defined in v8.h +const kStringMaxLength = process.binding('buffer').kStringMaxLength; + +try { + new Buffer(kStringMaxLength * 3); +} catch(e) { + assert.equal(e.message, 'Invalid array buffer length'); + console.log( + '1..0 # Skipped: intensive toString tests due to memory confinements'); + return; +} + +const buf0 = new Buffer(kStringMaxLength * 2 + 2); + +assert.throws(function() { + buf0.toString('utf16le'); +}, /toString failed/); diff --git a/test/parallel/test-stringbytes-external.js b/test/parallel/test-stringbytes-external.js index ddbbcd599fa4da..ba3f0c5d1d9ade 100644 --- a/test/parallel/test-stringbytes-external.js +++ b/test/parallel/test-stringbytes-external.js @@ -107,72 +107,3 @@ var PRE_3OF4_APEX = Math.ceil((EXTERN_APEX / 4) * 3) - RADIOS; assert.equal(a, b); assert.equal(b, c); })(); - -// v8 fails silently if string length > v8::String::kMaxLength -(function() { - // v8::String::kMaxLength defined in v8.h - const kStringMaxLength = process.binding('buffer').kStringMaxLength; - - try { - new Buffer(kStringMaxLength * 3); - } catch(e) { - assert.equal(e.message, 'Invalid array buffer length'); - console.log( - '1..0 # Skipped: intensive toString tests due to memory confinements'); - return; - } - - const buf0 = new Buffer(kStringMaxLength * 2 + 2); - const buf1 = buf0.slice(0, kStringMaxLength + 1); - const buf2 = buf0.slice(0, kStringMaxLength); - const buf3 = buf0.slice(0, kStringMaxLength + 2); - - assert.throws(function() { - buf1.toString(); - }, /toString failed|Invalid array buffer length/); - - assert.throws(function() { - buf1.toString('ascii'); - }, /toString failed/); - - assert.throws(function() { - buf1.toString('utf8'); - }, /toString failed/); - - assert.throws(function() { - buf0.toString('utf16le'); - }, /toString failed/); - - assert.throws(function() { - buf1.toString('binary'); - }, /toString failed/); - - assert.throws(function() { - buf1.toString('base64'); - }, /toString failed/); - - assert.throws(function() { - buf1.toString('hex'); - }, /toString failed/); - - var maxString = buf2.toString(); - assert.equal(maxString.length, kStringMaxLength); - // Free the memory early instead of at the end of the next assignment - maxString = undefined; - - maxString = buf2.toString('binary'); - assert.equal(maxString.length, kStringMaxLength); - maxString = undefined; - - maxString = buf1.toString('binary', 1); - assert.equal(maxString.length, kStringMaxLength); - maxString = undefined; - - maxString = buf1.toString('binary', 0, kStringMaxLength); - assert.equal(maxString.length, kStringMaxLength); - maxString = undefined; - - maxString = buf3.toString('utf16le'); - assert.equal(maxString.length, (kStringMaxLength + 2) / 2); - maxString = undefined; -})(); From a8425abfe53efc010dd18aee168fdf6f6f7b47ff Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 11 Oct 2015 20:53:31 -0700 Subject: [PATCH 40/51] test: split up buffer tests for reliability The Pi 1's in CI don't always fail on the buffer.toString() tests. But they time out sometimes, so let's split the tests up so they don't. PR-URL: https://github.com/nodejs/node/pull/3323 Reviewed By: Evan Lucas Reviewed-By: Brian White Reviewed By: Trevor Norris --- ...ingbytes-external-exceed-max-by-1-ascii.js | 23 ++++++++++++++ ...ngbytes-external-exceed-max-by-1-base64.js | 23 ++++++++++++++ ...gbytes-external-exceed-max-by-1-binary.js} | 31 +++---------------- ...tringbytes-external-exceed-max-by-1-hex.js | 23 ++++++++++++++ ...ringbytes-external-exceed-max-by-1-utf8.js | 27 ++++++++++++++++ 5 files changed, 101 insertions(+), 26 deletions(-) create mode 100644 test/parallel/test-stringbytes-external-exceed-max-by-1-ascii.js create mode 100644 test/parallel/test-stringbytes-external-exceed-max-by-1-base64.js rename test/parallel/{test-stringbytes-external-exceed-max-by-1.js => test-stringbytes-external-exceed-max-by-1-binary.js} (54%) create mode 100644 test/parallel/test-stringbytes-external-exceed-max-by-1-hex.js create mode 100644 test/parallel/test-stringbytes-external-exceed-max-by-1-utf8.js diff --git a/test/parallel/test-stringbytes-external-exceed-max-by-1-ascii.js b/test/parallel/test-stringbytes-external-exceed-max-by-1-ascii.js new file mode 100644 index 00000000000000..e982cf9eb092fd --- /dev/null +++ b/test/parallel/test-stringbytes-external-exceed-max-by-1-ascii.js @@ -0,0 +1,23 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +// v8 fails silently if string length > v8::String::kMaxLength +// v8::String::kMaxLength defined in v8.h +const kStringMaxLength = process.binding('buffer').kStringMaxLength; + +try { + new Buffer(kStringMaxLength * 3); +} catch(e) { + assert.equal(e.message, 'Invalid array buffer length'); + console.log( + '1..0 # Skipped: intensive toString tests due to memory confinements'); + return; +} + +const buf = new Buffer(kStringMaxLength + 1); + +assert.throws(function() { + buf.toString('ascii'); +}, /toString failed/); diff --git a/test/parallel/test-stringbytes-external-exceed-max-by-1-base64.js b/test/parallel/test-stringbytes-external-exceed-max-by-1-base64.js new file mode 100644 index 00000000000000..43a3358759e249 --- /dev/null +++ b/test/parallel/test-stringbytes-external-exceed-max-by-1-base64.js @@ -0,0 +1,23 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +// v8 fails silently if string length > v8::String::kMaxLength +// v8::String::kMaxLength defined in v8.h +const kStringMaxLength = process.binding('buffer').kStringMaxLength; + +try { + new Buffer(kStringMaxLength * 3); +} catch(e) { + assert.equal(e.message, 'Invalid array buffer length'); + console.log( + '1..0 # Skipped: intensive toString tests due to memory confinements'); + return; +} + +const buf = new Buffer(kStringMaxLength + 1); + +assert.throws(function() { + buf.toString('base64'); +}, /toString failed/); diff --git a/test/parallel/test-stringbytes-external-exceed-max-by-1.js b/test/parallel/test-stringbytes-external-exceed-max-by-1-binary.js similarity index 54% rename from test/parallel/test-stringbytes-external-exceed-max-by-1.js rename to test/parallel/test-stringbytes-external-exceed-max-by-1-binary.js index 8e2a5bf01f2596..9d0d2c3e897057 100644 --- a/test/parallel/test-stringbytes-external-exceed-max-by-1.js +++ b/test/parallel/test-stringbytes-external-exceed-max-by-1-binary.js @@ -16,37 +16,16 @@ try { return; } -const buf1 = new Buffer(kStringMaxLength + 1); +const buf = new Buffer(kStringMaxLength + 1); assert.throws(function() { - buf1.toString(); -}, /toString failed|Invalid array buffer length/); - -assert.throws(function() { - buf1.toString('ascii'); -}, /toString failed/); - -assert.throws(function() { - buf1.toString('utf8'); + buf.toString('binary'); }, /toString failed/); -assert.throws(function() { - buf1.toString('binary'); -}, /toString failed/); - -assert.throws(function() { - buf1.toString('base64'); -}, /toString failed/); - -assert.throws(function() { - buf1.toString('hex'); -}, /toString failed/); - -var maxString = buf1.toString('binary', 1); +var maxString = buf.toString('binary', 1); assert.equal(maxString.length, kStringMaxLength); +// Free the memory early instead of at the end of the next assignment maxString = undefined; -maxString = buf1.toString('binary', 0, kStringMaxLength); +maxString = buf.toString('binary', 0, kStringMaxLength); assert.equal(maxString.length, kStringMaxLength); -// Free the memory early instead of at the end of the next assignment -maxString = undefined; diff --git a/test/parallel/test-stringbytes-external-exceed-max-by-1-hex.js b/test/parallel/test-stringbytes-external-exceed-max-by-1-hex.js new file mode 100644 index 00000000000000..2937b4aab85813 --- /dev/null +++ b/test/parallel/test-stringbytes-external-exceed-max-by-1-hex.js @@ -0,0 +1,23 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +// v8 fails silently if string length > v8::String::kMaxLength +// v8::String::kMaxLength defined in v8.h +const kStringMaxLength = process.binding('buffer').kStringMaxLength; + +try { + new Buffer(kStringMaxLength * 3); +} catch(e) { + assert.equal(e.message, 'Invalid array buffer length'); + console.log( + '1..0 # Skipped: intensive toString tests due to memory confinements'); + return; +} + +const buf = new Buffer(kStringMaxLength + 1); + +assert.throws(function() { + buf.toString('hex'); +}, /toString failed/); diff --git a/test/parallel/test-stringbytes-external-exceed-max-by-1-utf8.js b/test/parallel/test-stringbytes-external-exceed-max-by-1-utf8.js new file mode 100644 index 00000000000000..ee297a880dacbd --- /dev/null +++ b/test/parallel/test-stringbytes-external-exceed-max-by-1-utf8.js @@ -0,0 +1,27 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +// v8 fails silently if string length > v8::String::kMaxLength +// v8::String::kMaxLength defined in v8.h +const kStringMaxLength = process.binding('buffer').kStringMaxLength; + +try { + new Buffer(kStringMaxLength * 3); +} catch(e) { + assert.equal(e.message, 'Invalid array buffer length'); + console.log( + '1..0 # Skipped: intensive toString tests due to memory confinements'); + return; +} + +const buf = new Buffer(kStringMaxLength + 1); + +assert.throws(function() { + buf.toString(); +}, /toString failed|Invalid array buffer length/); + +assert.throws(function() { + buf.toString('utf8'); +}, /toString failed/); From 9bffceb336003c23bd5cd5848057d1d0bdc2cfa6 Mon Sep 17 00:00:00 2001 From: Brian White Date: Tue, 13 Oct 2015 17:18:15 -0400 Subject: [PATCH 41/51] src: fix exception message encoding on Windows The printf family of functions do not properly display UTF8 strings well on Windows. Use the appropriate wide character API instead if stderr is a tty. PR-URL: https://github.com/nodejs/node/pull/3288 Fixes: https://github.com/nodejs/node/issues/3284 Reviewed-By: Bert Belder --- src/node.cc | 58 +++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/src/node.cc b/src/node.cc index 8766e3a1430f90..1b49fad970ab20 100644 --- a/src/node.cc +++ b/src/node.cc @@ -51,6 +51,7 @@ #include #include #include +#include #if defined(NODE_HAVE_I18N_SUPPORT) #include @@ -156,6 +157,38 @@ static Isolate* node_isolate = nullptr; static v8::Platform* default_platform; +static void PrintErrorString(const char* format, ...) { + va_list ap; + va_start(ap, format); +#ifdef _WIN32 + HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE); + + // Check if stderr is something other than a tty/console + if (stderr_handle == INVALID_HANDLE_VALUE || + stderr_handle == nullptr || + uv_guess_handle(_fileno(stderr)) != UV_TTY) { + vfprintf(stderr, format, ap); + return; + } + + // Fill in any placeholders + int n = _vscprintf(format, ap); + std::vector out(n + 1); + vsprintf(out.data(), format, ap); + + // Get required wide buffer size + n = MultiByteToWideChar(CP_UTF8, 0, out.data(), -1, nullptr, 0); + + std::vector wbuf(n); + MultiByteToWideChar(CP_UTF8, 0, out.data(), -1, wbuf.data(), n); + WriteConsoleW(stderr_handle, wbuf.data(), n, nullptr, nullptr); +#else + vfprintf(stderr, format, ap); +#endif + va_end(ap); +} + + static void CheckImmediate(uv_check_t* handle) { Environment* env = Environment::from_immediate_check_handle(handle); HandleScope scope(env->isolate()); @@ -1416,7 +1449,7 @@ void AppendExceptionLine(Environment* env, return; env->set_printed_error(true); uv_tty_reset_mode(); - fprintf(stderr, "\n%s", arrow); + PrintErrorString("\n%s", arrow); } @@ -1444,10 +1477,10 @@ static void ReportException(Environment* env, // range errors have a trace member set to undefined if (trace.length() > 0 && !trace_value->IsUndefined()) { if (arrow.IsEmpty() || !arrow->IsString()) { - fprintf(stderr, "%s\n", *trace); + PrintErrorString("%s\n", *trace); } else { node::Utf8Value arrow_string(env->isolate(), arrow); - fprintf(stderr, "%s\n%s\n", *arrow_string, *trace); + PrintErrorString("%s\n%s\n", *arrow_string, *trace); } } else { // this really only happens for RangeErrors, since they're the only @@ -1468,20 +1501,19 @@ static void ReportException(Environment* env, name->IsUndefined()) { // Not an error object. Just print as-is. node::Utf8Value message(env->isolate(), er); - fprintf(stderr, "%s\n", *message); + PrintErrorString("%s\n", *message); } else { node::Utf8Value name_string(env->isolate(), name); node::Utf8Value message_string(env->isolate(), message); if (arrow.IsEmpty() || !arrow->IsString()) { - fprintf(stderr, "%s: %s\n", *name_string, *message_string); + PrintErrorString("%s: %s\n", *name_string, *message_string); } else { node::Utf8Value arrow_string(env->isolate(), arrow); - fprintf(stderr, - "%s\n%s: %s\n", - *arrow_string, - *name_string, - *message_string); + PrintErrorString("%s\n%s: %s\n", + *arrow_string, + *name_string, + *message_string); } } } @@ -2176,9 +2208,9 @@ void DLOpen(const FunctionCallbackInfo& args) { static void OnFatalError(const char* location, const char* message) { if (location) { - fprintf(stderr, "FATAL ERROR: %s %s\n", location, message); + PrintErrorString("FATAL ERROR: %s %s\n", location, message); } else { - fprintf(stderr, "FATAL ERROR: %s\n", message); + PrintErrorString("FATAL ERROR: %s\n", message); } fflush(stderr); ABORT(); @@ -3002,7 +3034,7 @@ static void RawDebug(const FunctionCallbackInfo& args) { CHECK(args.Length() == 1 && args[0]->IsString() && "must be called with a single string"); node::Utf8Value message(args.GetIsolate(), args[0]); - fprintf(stderr, "%s\n", *message); + PrintErrorString("%s\n", *message); fflush(stderr); } From 4c411ae8f21491ed7fbd02908788b31ec26ae641 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Fri, 9 Oct 2015 15:26:55 -0700 Subject: [PATCH 42/51] test: remove util properties from common common does not need util properties anymore. Remove them. PR-URL: https://github.com/nodejs/node/pull/3304 Reviewed-By: Jeremiah Senkpiel Reviewed-By: Brendan Ashworth Reviewed-By: Sakthipriyan Vairamani --- test/common.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/common.js b/test/common.js index 750165ea8cc6c0..7fbb50b44906a1 100644 --- a/test/common.js +++ b/test/common.js @@ -5,6 +5,8 @@ var fs = require('fs'); var assert = require('assert'); var os = require('os'); var child_process = require('child_process'); +var util = require('util'); + exports.testDir = path.dirname(__filename); exports.fixturesDir = path.join(exports.testDir, 'fixtures'); @@ -171,10 +173,6 @@ exports.hasIPv6 = Object.keys(ifaces).some(function(name) { }); }); -var util = require('util'); -for (var i in util) exports[i] = util[i]; -//for (var i in exports) global[i] = exports[i]; - function protoCtrChain(o) { var result = []; for (; o; o = o.__proto__) { result.push(o.constructor); } From d4fced7cb7c12b74781f42bdba4902c232b54727 Mon Sep 17 00:00:00 2001 From: Rich Trott Date: Sun, 11 Oct 2015 22:14:01 -0700 Subject: [PATCH 43/51] test: remove util from common MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit util is loaded just for one use of util.format(). Replace it with a string template. While we're at it, delete nearby lengthy comment justifying use of fs.readFileSync(). PR-URL: https://github.com/nodejs/node/pull/3324 Reviewed-By: Michaël Zasso Reviewed-By: Johan Bergström Reviewed By: Evan Lucas --- test/common.js | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/test/common.js b/test/common.js index 7fbb50b44906a1..eb802cc8856377 100644 --- a/test/common.js +++ b/test/common.js @@ -5,7 +5,6 @@ var fs = require('fs'); var assert = require('assert'); var os = require('os'); var child_process = require('child_process'); -var util = require('util'); exports.testDir = path.dirname(__filename); @@ -410,15 +409,9 @@ exports.getServiceName = function getServiceName(port, protocol) { var serviceName = port.toString(); try { - /* - * I'm not a big fan of readFileSync, but reading /etc/services - * asynchronously here would require implementing a simple line parser, - * which seems overkill for a simple utility function that is not running - * concurrently with any other one. - */ var servicesContent = fs.readFileSync(etcServicesFileName, { encoding: 'utf8'}); - var regexp = util.format('^(\\w+)\\s+\\s%d/%s\\s', port, protocol); + var regexp = `^(\\w+)\\s+\\s${port}/${protocol}\\s`; var re = new RegExp(regexp, 'm'); var matches = re.exec(servicesContent); From b8869bdc2701a9c8bb3d3642794f1c0d4a37d781 Mon Sep 17 00:00:00 2001 From: Jonas Dohse Date: Tue, 13 Oct 2015 18:38:45 -0700 Subject: [PATCH 44/51] test: port domains regression test from v0.10 f2a45caf2e1b73fea01fa9a941bc61096a999664 contained a test for a regression that had been introduced by the original change that 77a10ed05f1e413818d29f07cbb268108b1f08fa ported. While 77a10ed05f1e413818d29f07cbb268108b1f08fa did not contain that regression, the test that f2a45caf2e1b73fea01fa9a941bc61096a999664 contained should still be in the code base to prevent any regression from happening in the future. Original message for the commit that contained the test: domains: fix stack clearing after error handled caeb67735baa8e069902e23f21d01033907c4f33 introduced a regression where the domains stack would not be cleared after an error had been handled by the top-level domain. This change clears the domains stack regardless of the position of the active domain in the stack. PR: #9364 PR-URL: https://github.com/joyent/node/pull/9364 Reviewed-By: Trevor Norris Reviewed-By: Julien Gilli PR: #3356 PR-URL: https://github.com/nodejs/node/pull/3356 Reviewed-By: Ben Noordhuis --- ...in-top-level-error-handler-clears-stack.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 test/parallel/test-domain-top-level-error-handler-clears-stack.js diff --git a/test/parallel/test-domain-top-level-error-handler-clears-stack.js b/test/parallel/test-domain-top-level-error-handler-clears-stack.js new file mode 100644 index 00000000000000..a5fec1f65ef029 --- /dev/null +++ b/test/parallel/test-domain-top-level-error-handler-clears-stack.js @@ -0,0 +1,34 @@ +'use strict'; + +const common = require('../common'); +const domain = require('domain'); + +/* + * Make sure that the domains stack is cleared after a top-level domain + * error handler exited gracefully. + */ +const d = domain.create(); + +d.on('error', common.mustCall(function() { + process.nextTick(function() { + // Scheduling a callback with process.nextTick will enter a _new_ domain, + // and the callback will be called after the domain that handled the error + // was exited. So there should be only one domain on the domains stack if + // the domains stack was cleared properly when the domain error handler + // returned. + if (domain._stack.length !== 1) { + // Do not use assert to perform this test: this callback runs in a + // different callstack as the original process._fatalException that + // handled the original error, thus throwing here would trigger another + // call to process._fatalException, and so on recursively and + // indefinitely. + console.error('domains stack length should be 1, but instead is:', + domain._stack.length); + process.exit(1); + } + }); +})); + +d.run(function() { + throw new Error('Error from domain'); +}); From 40bb8024c6999361c9838affd1cf7bce29b8fc10 Mon Sep 17 00:00:00 2001 From: Fedor Indutny Date: Fri, 16 Oct 2015 15:34:15 -0400 Subject: [PATCH 45/51] timers: reuse timer in `setTimeout().unref()` Instead of creating new timer - reuse the timer from the freelist. This won't make the freelist timer active for the duration of `uv_close()`, and will let the event-loop exit properly. Fix: #1264 PR-URL: https://github.com/nodejs/node/pull/3407 Reviewed-By: Trevor Norris Reviewed-By: Jeremiah Senkpiel --- lib/timers.js | 29 ++++++++++++++----- .../test-timers-unrefed-in-beforeexit.js | 20 +++++++++++++ 2 files changed, 42 insertions(+), 7 deletions(-) create mode 100644 test/parallel/test-timers-unrefed-in-beforeexit.js diff --git a/lib/timers.js b/lib/timers.js index 6518ae5796105e..e4b27fdc54adb6 100644 --- a/lib/timers.js +++ b/lib/timers.js @@ -119,16 +119,27 @@ function listOnTimeoutNT(list) { } -const unenroll = exports.unenroll = function(item) { +function reuse(item) { L.remove(item); var list = lists[item._idleTimeout]; - // if empty then stop the watcher - debug('unenroll'); + // if empty - reuse the watcher if (list && L.isEmpty(list)) { + debug('reuse hit'); + list.stop(); + delete lists[item._idleTimeout]; + return list; + } + + return null; +} + + +const unenroll = exports.unenroll = function(item) { + var list = reuse(item); + if (list) { debug('unenroll: list empty'); list.close(); - delete lists[item._idleTimeout]; } // if active is called later, then we want to make sure not to insert again item._idleTimeout = -1; @@ -312,12 +323,16 @@ Timeout.prototype.unref = function() { if (!this._idleStart) this._idleStart = now; var delay = this._idleStart + this._idleTimeout - now; if (delay < 0) delay = 0; - exports.unenroll(this); // Prevent running cb again when unref() is called during the same cb - if (this._called && !this._repeat) return; + if (this._called && !this._repeat) { + exports.unenroll(this); + return; + } + + var handle = reuse(this); - this._handle = new Timer(); + this._handle = handle || new Timer(); this._handle.owner = this; this._handle[kOnTimeout] = unrefdHandle; this._handle.start(delay, 0); diff --git a/test/parallel/test-timers-unrefed-in-beforeexit.js b/test/parallel/test-timers-unrefed-in-beforeexit.js new file mode 100644 index 00000000000000..0ac3311816fb4d --- /dev/null +++ b/test/parallel/test-timers-unrefed-in-beforeexit.js @@ -0,0 +1,20 @@ +'use strict'; + +require('../common'); +const assert = require('assert'); + +var once = 0; + +process.on('beforeExit', () => { + if (once > 1) + throw new RangeError('beforeExit should only have been called once!'); + + setTimeout(() => {}, 1).unref(); + once++; +}); + +process.on('exit', (code) => { + if (code !== 0) return; + + assert.strictEqual(once, 1); +}); From d9188387233c85158d6d535e6dbdb656eda7e0b7 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Sat, 24 Oct 2015 13:16:08 +0530 Subject: [PATCH 46/51] repl: handle comments properly As it is, the comments are not handled properly in REPL. So, if the comments have `'` or `"`, then they are treated as incomplete string literals and the error is thrown in REPL. This patch refactors the existing logic and groups everything in a class. Fixes: https://github.com/nodejs/node/issues/3421 PR-URL: https://github.com/nodejs/node/pull/3515 Reviewed-By: Brian White Reviewed-By: Jeremiah Senkpiel --- lib/repl.js | 172 ++++++++++++++++++++----------------- test/parallel/test-repl.js | 28 ++++++ 2 files changed, 122 insertions(+), 78 deletions(-) diff --git a/lib/repl.js b/lib/repl.js index 46288db5639c59..31e1677d0bbd1a 100644 --- a/lib/repl.js +++ b/lib/repl.js @@ -70,6 +70,88 @@ const BLOCK_SCOPED_ERROR = 'Block-scoped declarations (let, ' + 'const, function, class) not yet supported outside strict mode'; +class LineParser { + + constructor() { + this.reset(); + } + + reset() { + this._literal = null; + this.shouldFail = false; + this.blockComment = false; + } + + parseLine(line) { + var previous = null; + this.shouldFail = false; + const wasWithinStrLiteral = this._literal !== null; + + for (const current of line) { + if (previous === '\\') { + // valid escaping, skip processing. previous doesn't matter anymore + previous = null; + continue; + } + + if (!this._literal) { + if (previous === '*' && current === '/') { + if (this.blockComment) { + this.blockComment = false; + previous = null; + continue; + } else { + this.shouldFail = true; + break; + } + } + + // ignore rest of the line if `current` and `previous` are `/`s + if (previous === current && previous === '/' && !this.blockComment) { + break; + } + + if (previous === '/' && current === '*') { + this.blockComment = true; + previous = null; + } + } + + if (this.blockComment) continue; + + if (current === this._literal) { + this._literal = null; + } else if (current === '\'' || current === '"') { + this._literal = this._literal || current; + } + + previous = current; + } + + const isWithinStrLiteral = this._literal !== null; + + if (!wasWithinStrLiteral && !isWithinStrLiteral) { + // Current line has nothing to do with String literals, trim both ends + line = line.trim(); + } else if (wasWithinStrLiteral && !isWithinStrLiteral) { + // was part of a string literal, but it is over now, trim only the end + line = line.trimRight(); + } else if (isWithinStrLiteral && !wasWithinStrLiteral) { + // was not part of a string literal, but it is now, trim only the start + line = line.trimLeft(); + } + + const lastChar = line.charAt(line.length - 1); + + this.shouldFail = this.shouldFail || + ((!this._literal && lastChar === '\\') || + (this._literal && lastChar !== '\\')); + + return line; + } +} + + function REPLServer(prompt, stream, eval_, @@ -193,7 +275,7 @@ function REPLServer(prompt, debug('domain error'); const top = replMap.get(self); top.outputStream.write((e.stack || e) + '\n'); - top._currentStringLiteral = null; + top.lineParser.reset(); top.bufferedCommand = ''; top.lines.level = []; top.displayPrompt(); @@ -220,8 +302,7 @@ function REPLServer(prompt, self.outputStream = output; self.resetContext(); - // Initialize the current string literal found, to be null - self._currentStringLiteral = null; + self.lineParser = new LineParser(); self.bufferedCommand = ''; self.lines.level = []; @@ -280,87 +361,22 @@ function REPLServer(prompt, sawSIGINT = false; } - self._currentStringLiteral = null; + self.lineParser.reset(); self.bufferedCommand = ''; self.lines.level = []; self.displayPrompt(); }); - function parseLine(line, currentStringLiteral) { - var previous = null, current = null; - - for (var i = 0; i < line.length; i += 1) { - if (previous === '\\') { - // if it is a valid escaping, then skip processing and the previous - // character doesn't matter anymore. - previous = null; - continue; - } - - current = line.charAt(i); - if (current === currentStringLiteral) { - currentStringLiteral = null; - } else if (current === '\'' || - current === '"' && - currentStringLiteral === null) { - currentStringLiteral = current; - } - previous = current; - } - - return currentStringLiteral; - } - - function getFinisherFunction(cmd, defaultFn) { - if ((self._currentStringLiteral === null && - cmd.charAt(cmd.length - 1) === '\\') || - (self._currentStringLiteral !== null && - cmd.charAt(cmd.length - 1) !== '\\')) { - - // If the line continuation is used outside string literal or if the - // string continuation happens with out line continuation, then fail hard. - // Even if the error is recoverable, get the underlying error and use it. - return function(e, ret) { - var error = e instanceof Recoverable ? e.err : e; - - if (arguments.length === 2) { - // using second argument only if it is actually passed. Otherwise - // `undefined` will be printed when invalid REPL commands are used. - return defaultFn(error, ret); - } - - return defaultFn(error); - }; - } - return defaultFn; - } - self.on('line', function(cmd) { debug('line %j', cmd); sawSIGINT = false; var skipCatchall = false; - var finisherFn = finish; // leading whitespaces in template literals should not be trimmed. if (self._inTemplateLiteral) { self._inTemplateLiteral = false; } else { - const wasWithinStrLiteral = self._currentStringLiteral !== null; - self._currentStringLiteral = parseLine(cmd, self._currentStringLiteral); - const isWithinStrLiteral = self._currentStringLiteral !== null; - - if (!wasWithinStrLiteral && !isWithinStrLiteral) { - // Current line has nothing to do with String literals, trim both ends - cmd = cmd.trim(); - } else if (wasWithinStrLiteral && !isWithinStrLiteral) { - // was part of a string literal, but it is over now, trim only the end - cmd = cmd.trimRight(); - } else if (isWithinStrLiteral && !wasWithinStrLiteral) { - // was not part of a string literal, but it is now, trim only the start - cmd = cmd.trimLeft(); - } - - finisherFn = getFinisherFunction(cmd, finish); + cmd = self.lineParser.parseLine(cmd); } // Check to see if a REPL keyword was used. If it returns true, @@ -393,9 +409,9 @@ function REPLServer(prompt, } debug('eval %j', evalCmd); - self.eval(evalCmd, self.context, 'repl', finisherFn); + self.eval(evalCmd, self.context, 'repl', finish); } else { - finisherFn(null); + finish(null); } function finish(e, ret) { @@ -406,7 +422,7 @@ function REPLServer(prompt, self.outputStream.write('npm should be run outside of the ' + 'node repl, in your normal shell.\n' + '(Press Control-D to exit.)\n'); - self._currentStringLiteral = null; + self.lineParser.reset(); self.bufferedCommand = ''; self.displayPrompt(); return; @@ -414,7 +430,7 @@ function REPLServer(prompt, // If error was SyntaxError and not JSON.parse error if (e) { - if (e instanceof Recoverable) { + if (e instanceof Recoverable && !self.lineParser.shouldFail) { // Start buffering data like that: // { // ... x: 1 @@ -423,12 +439,12 @@ function REPLServer(prompt, self.displayPrompt(); return; } else { - self._domain.emit('error', e); + self._domain.emit('error', e.err || e); } } // Clear buffer if no SyntaxErrors - self._currentStringLiteral = null; + self.lineParser.reset(); self.bufferedCommand = ''; // If we got any output - print it (if no error) @@ -971,7 +987,7 @@ function defineDefaultCommands(repl) { repl.defineCommand('break', { help: 'Sometimes you get stuck, this gets you out', action: function() { - this._currentStringLiteral = null; + this.lineParser.reset(); this.bufferedCommand = ''; this.displayPrompt(); } @@ -986,7 +1002,7 @@ function defineDefaultCommands(repl) { repl.defineCommand('clear', { help: clearMessage, action: function() { - this._currentStringLiteral = null; + this.lineParser.reset(); this.bufferedCommand = ''; if (!this.useGlobal) { this.outputStream.write('Clearing context...\n'); diff --git a/test/parallel/test-repl.js b/test/parallel/test-repl.js index ac4c9012e906f6..4d65a1e9d084b4 100644 --- a/test/parallel/test-repl.js +++ b/test/parallel/test-repl.js @@ -250,6 +250,34 @@ function error_test() { { client: client_unix, send: 'function x() {\nreturn \'\\\\\';\n }', expect: prompt_multiline + prompt_multiline + 'undefined\n' + prompt_unix }, + // regression tests for https://github.com/nodejs/node/issues/3421 + { client: client_unix, send: 'function x() {\n//\'\n }', + expect: prompt_multiline + prompt_multiline + + 'undefined\n' + prompt_unix }, + { client: client_unix, send: 'function x() {\n//"\n }', + expect: prompt_multiline + prompt_multiline + + 'undefined\n' + prompt_unix }, + { client: client_unix, send: 'function x() {//\'\n }', + expect: prompt_multiline + 'undefined\n' + prompt_unix }, + { client: client_unix, send: 'function x() {//"\n }', + expect: prompt_multiline + 'undefined\n' + prompt_unix }, + { client: client_unix, send: 'function x() {\nvar i = "\'";\n }', + expect: prompt_multiline + prompt_multiline + + 'undefined\n' + prompt_unix }, + { client: client_unix, send: 'function x(/*optional*/) {}', + expect: 'undefined\n' + prompt_unix }, + { client: client_unix, send: 'function x(/* // 5 */) {}', + expect: 'undefined\n' + prompt_unix }, + { client: client_unix, send: '// /* 5 */', + expect: 'undefined\n' + prompt_unix }, + { client: client_unix, send: '"//"', + expect: '\'//\'\n' + prompt_unix }, + { client: client_unix, send: '"data /*with*/ comment"', + expect: '\'data /*with*/ comment\'\n' + prompt_unix }, + { client: client_unix, send: 'function x(/*fn\'s optional params*/) {}', + expect: 'undefined\n' + prompt_unix }, + { client: client_unix, send: '/* \'\n"\n\'"\'\n*/', + expect: 'undefined\n' + prompt_unix }, ]); } From c85736eed76bdfce049ff05550dc42afd79bcf4a Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Mon, 26 Oct 2015 14:11:03 +0100 Subject: [PATCH 47/51] src: fix race condition in debug signal on exit Before this commit, sending a SIGUSR1 at program exit could trigger a hard to reproduce race condition where `v8::Debug::DebugBreak(isolate)` got called when the isolate was in the process of being torn down. A similar race condition is in theory possible when sending signals to two threads simultaneously but I haven't been able to reproduce that myself (and I tried, oh how I tried.) This commit fixes the race condition by turning `node_isolate` into a `std::atomic` and using it as an ad hoc synchronization primitive in places where that is necessary. A bare minimum std::atomic polyfill is added for OS X because Apple wouldn't be Apple if things just worked out of the box. PR-URL: https://github.com/nodejs/node/pull/3528 Reviewed-By: Fedor Indutny Reviewed-By: James M Snell --- src/atomic-polyfill.h | 18 ++++++++++++++ src/node.cc | 55 +++++++++++++++++++++++++++++++++---------- 2 files changed, 60 insertions(+), 13 deletions(-) create mode 100644 src/atomic-polyfill.h diff --git a/src/atomic-polyfill.h b/src/atomic-polyfill.h new file mode 100644 index 00000000000000..1c5f414fa13a81 --- /dev/null +++ b/src/atomic-polyfill.h @@ -0,0 +1,18 @@ +#ifndef SRC_ATOMIC_POLYFILL_H_ +#define SRC_ATOMIC_POLYFILL_H_ + +#include "util.h" + +namespace nonstd { + +template +struct atomic { + atomic() = default; + T exchange(T value) { return __sync_lock_test_and_set(&value_, value); } + T value_ = T(); + DISALLOW_COPY_AND_ASSIGN(atomic); +}; + +} // namespace nonstd + +#endif // SRC_ATOMIC_POLYFILL_H_ diff --git a/src/node.cc b/src/node.cc index 1b49fad970ab20..46fca923ac57e8 100644 --- a/src/node.cc +++ b/src/node.cc @@ -86,6 +86,14 @@ typedef int mode_t; extern char **environ; #endif +#ifdef __APPLE__ +#include "atomic-polyfill.h" // NOLINT(build/include_order) +namespace node { template using atomic = nonstd::atomic; } +#else +#include +namespace node { template using atomic = std::atomic; } +#endif + namespace node { using v8::Array; @@ -153,7 +161,7 @@ static double prog_start_time; static bool debugger_running; static uv_async_t dispatch_debug_messages_async; -static Isolate* node_isolate = nullptr; +static node::atomic node_isolate; static v8::Platform* default_platform; @@ -3389,28 +3397,46 @@ static void EnableDebug(Environment* env) { } +// Called from an arbitrary thread. +static void TryStartDebugger() { + // Call only async signal-safe functions here! Don't retry the exchange, + // it will deadlock when the thread is interrupted inside a critical section. + if (auto isolate = node_isolate.exchange(nullptr)) { + v8::Debug::DebugBreak(isolate); + uv_async_send(&dispatch_debug_messages_async); + CHECK_EQ(nullptr, node_isolate.exchange(isolate)); + } +} + + // Called from the main thread. static void DispatchDebugMessagesAsyncCallback(uv_async_t* handle) { + // Synchronize with signal handler, see TryStartDebugger. + Isolate* isolate; + do { + isolate = node_isolate.exchange(nullptr); + } while (isolate == nullptr); + if (debugger_running == false) { fprintf(stderr, "Starting debugger agent.\n"); - HandleScope scope(node_isolate); - Environment* env = Environment::GetCurrent(node_isolate); + HandleScope scope(isolate); + Environment* env = Environment::GetCurrent(isolate); Context::Scope context_scope(env->context()); StartDebug(env, false); EnableDebug(env); } - Isolate::Scope isolate_scope(node_isolate); + + Isolate::Scope isolate_scope(isolate); v8::Debug::ProcessDebugMessages(); + CHECK_EQ(nullptr, node_isolate.exchange(isolate)); } #ifdef __POSIX__ static void EnableDebugSignalHandler(int signo) { - // Call only async signal-safe functions here! - v8::Debug::DebugBreak(*static_cast(&node_isolate)); - uv_async_send(&dispatch_debug_messages_async); + TryStartDebugger(); } @@ -3464,8 +3490,7 @@ static int RegisterDebugSignalHandler() { #ifdef _WIN32 DWORD WINAPI EnableDebugThreadProc(void* arg) { - v8::Debug::DebugBreak(*static_cast(&node_isolate)); - uv_async_send(&dispatch_debug_messages_async); + TryStartDebugger(); return 0; } @@ -3985,7 +4010,8 @@ static void StartNodeInstance(void* arg) { // Fetch a reference to the main isolate, so we have a reference to it // even when we need it to access it from another (debugger) thread. if (instance_data->is_main()) - node_isolate = isolate; + CHECK_EQ(nullptr, node_isolate.exchange(isolate)); + { Locker locker(isolate); Isolate::Scope isolate_scope(isolate); @@ -3995,7 +4021,7 @@ static void StartNodeInstance(void* arg) { array_buffer_allocator->set_env(env); Context::Scope context_scope(context); - node_isolate->SetAbortOnUncaughtExceptionCallback( + isolate->SetAbortOnUncaughtExceptionCallback( ShouldAbortOnUncaughtException); // Start debug agent when argv has --debug @@ -4049,12 +4075,15 @@ static void StartNodeInstance(void* arg) { env = nullptr; } + if (instance_data->is_main()) { + // Synchronize with signal handler, see TryStartDebugger. + while (isolate != node_isolate.exchange(nullptr)); // NOLINT + } + CHECK_NE(isolate, nullptr); isolate->Dispose(); isolate = nullptr; delete array_buffer_allocator; - if (instance_data->is_main()) - node_isolate = nullptr; } int Start(int argc, char** argv) { From 77511eb10817807fb31e40c9aef9c9f66a6ffb52 Mon Sep 17 00:00:00 2001 From: Ali Ijaz Sheikh Date: Sun, 25 Oct 2015 17:19:58 -0700 Subject: [PATCH 48/51] deps: backport 010897c from V8 upstream This is a reland of https://github.com/nodejs/node/pull/3165. The patch abates the truncation of script filenames in the perf-event output produced by V8. V8 commits: Original: https://github.com/v8/v8/commit/03ef3cd004c2fd31ae7e48772f106df67b8c2feb Reland: https://github.com/v8/v8/commit/010897c16adb46d3fe403eab525502a63e174b0c Original commit message: improve perf_basic_prof filename reporting The buffer used for appending filenames to the string printed to the perf_basic_prof log was unnecessarily too small. Bump it up to be at least kUtf8BufferSize. Truncation of filenames makes it really hard to work with profiles gathered on Node.js. Because of the way Node.js works, you can have node module dependencies in deeply nested directories. The last thing you want when investigating a performance problem is to have script names be truncated. This patch is a stop-gap. Ideally, I want no truncation of the filename at all and use a dynamically growing buffer. That would be a larger change, and I wanted to have a quick fix available that can be back-ported to Node.js LTS release. R=yangguo@chromium.org,yurys@chromium.org BUG= Review URL: https://codereview.chromium.org/1388543002 PR-URL: https://github.com/nodejs/node/pull/3520 Reviewed-By: bnoordhuis - Ben Noordhuis --- deps/v8/src/log.cc | 12 ++++--- deps/v8/test/cctest/test-log.cc | 55 +++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/deps/v8/src/log.cc b/deps/v8/src/log.cc index 8f47e81f0eccce..d5842597cf9404 100644 --- a/deps/v8/src/log.cc +++ b/deps/v8/src/log.cc @@ -125,8 +125,9 @@ class CodeEventLogger::NameBuffer { } void AppendInt(int n) { - Vector buffer(utf8_buffer_ + utf8_pos_, - kUtf8BufferSize - utf8_pos_); + int space = kUtf8BufferSize - utf8_pos_; + if (space <= 0) return; + Vector buffer(utf8_buffer_ + utf8_pos_, space); int size = SNPrintF(buffer, "%d", n); if (size > 0 && utf8_pos_ + size <= kUtf8BufferSize) { utf8_pos_ += size; @@ -134,8 +135,9 @@ class CodeEventLogger::NameBuffer { } void AppendHex(uint32_t n) { - Vector buffer(utf8_buffer_ + utf8_pos_, - kUtf8BufferSize - utf8_pos_); + int space = kUtf8BufferSize - utf8_pos_; + if (space <= 0) return; + Vector buffer(utf8_buffer_ + utf8_pos_, space); int size = SNPrintF(buffer, "%x", n); if (size > 0 && utf8_pos_ + size <= kUtf8BufferSize) { utf8_pos_ += size; @@ -147,7 +149,7 @@ class CodeEventLogger::NameBuffer { private: static const int kUtf8BufferSize = 512; - static const int kUtf16BufferSize = 128; + static const int kUtf16BufferSize = kUtf8BufferSize; int utf8_pos_; char utf8_buffer_[kUtf8BufferSize]; diff --git a/deps/v8/test/cctest/test-log.cc b/deps/v8/test/cctest/test-log.cc index 0938a9ede21053..daf2e688b6541d 100644 --- a/deps/v8/test/cctest/test-log.cc +++ b/deps/v8/test/cctest/test-log.cc @@ -531,3 +531,58 @@ TEST(LogVersion) { } isolate->Dispose(); } + + +// https://crbug.com/539892 +// CodeCreateEvents with really large names should not crash. +TEST(Issue539892) { + class : public i::CodeEventLogger { + public: + virtual void CodeMoveEvent(Address from, Address to) {} + virtual void CodeDeleteEvent(Address from) {} + virtual void CodeDisableOptEvent(i::Code* code, + i::SharedFunctionInfo* shared) {} + + private: + virtual void LogRecordedBuffer(i::Code* code, i::SharedFunctionInfo* shared, + const char* name, int length) {} + } code_event_logger; + SETUP_FLAGS(); + v8::Isolate::CreateParams create_params; + create_params.array_buffer_allocator = CcTest::array_buffer_allocator(); + v8::Isolate* isolate = v8::Isolate::New(create_params); + + { + ScopedLoggerInitializer initialize_logger(saved_log, saved_prof, isolate); + Logger* logger = initialize_logger.logger(); + logger->addCodeEventListener(&code_event_logger); + + // Function with a really large name. + const char* source_text = + "(function " + "baaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaac" + "(){})();"; + + CompileRun(source_text); + + // Must not crash. + logger->LogCompiledFunctions(); + } + isolate->Dispose(); +} From 282065f48b0d9a549a5ed0c5664212bd5fda9f61 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Sun, 20 Sep 2015 12:29:24 +0530 Subject: [PATCH 49/51] doc: mention the behaviour if URL is invalid If the URL passed to `http.request` is not properly parsable by `url.parse`, we fall back to use `localhost` and port 80. This creates confusing error messages like in this question http://stackoverflow.com/q/32675907/1903116. PR-URL: https://github.com/nodejs/node/pull/2966 Reviewed-By: James M Snell --- doc/api/http.markdown | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/doc/api/http.markdown b/doc/api/http.markdown index 8c9e6babb64a04..1506ca10a34fbc 100644 --- a/doc/api/http.markdown +++ b/doc/api/http.markdown @@ -464,7 +464,12 @@ Node.js maintains several connections per server to make HTTP requests. This function allows one to transparently issue requests. `options` can be an object or a string. If `options` is a string, it is -automatically parsed with [url.parse()][]. +automatically parsed with [url.parse()][] and it must be a valid complete URL, +including protocol and complete domain name or IP address. + +**Note**: If the passed string is not in the valid URL format, then the + connection will be established to the default domain name, localhost, and on + the default port, 80. *This will be fixed soon.* Options: @@ -564,7 +569,7 @@ There are a few special headers that should be noted. ## http.get(options[, callback]) Since most requests are GET requests without bodies, Node.js provides this -convenience method. The only difference between this method and `http.request()` +convenience method. The only difference between this method and [http.request][] is that it sets the method to GET and calls `req.end()` automatically. Example: From 1f6f12301223e656cc21663a441e95e8844e7542 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 29 Oct 2015 13:33:03 +0100 Subject: [PATCH 50/51] Revert "src: fix stuck debugger process" This reverts commit ff877e93e16971ab8514772bd8d112e240b74803. Reverted for breaking `node --debug-brk -e 0`. It should immediately quit but instead it hangs now. PR-URL: https://github.com/nodejs/node/pull/3585 Reviewed-By: Evan Lucas Reviewed-By: James M Snell --- src/node.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/node.cc b/src/node.cc index 46fca923ac57e8..18b92d382522fc 100644 --- a/src/node.cc +++ b/src/node.cc @@ -3726,6 +3726,7 @@ void Init(int* argc, uv_async_init(uv_default_loop(), &dispatch_debug_messages_async, DispatchDebugMessagesAsyncCallback); + uv_unref(reinterpret_cast(&dispatch_debug_messages_async)); #if defined(NODE_V8_OPTIONS) // Should come before the call to V8::SetFlagsFromCommandLine() @@ -4033,11 +4034,8 @@ static void StartNodeInstance(void* arg) { env->set_trace_sync_io(trace_sync_io); // Enable debugger - if (instance_data->use_debug_agent()) { + if (instance_data->use_debug_agent()) EnableDebug(env); - } else { - uv_unref(reinterpret_cast(&dispatch_debug_messages_async)); - } { SealHandleScope seal(isolate); From 37aec6a97a8a45d320898c1d31681a5ad3bfff8f Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 29 Oct 2015 13:39:56 +0100 Subject: [PATCH 51/51] test: add regression test for --debug-brk -e 0 Check that `node --debug-brk -e 0` immediately quits. PR-URL: https://github.com/nodejs/node/pull/3585 Reviewed-By: Evan Lucas Reviewed-By: James M Snell --- test/parallel/test-debug-brk.js | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 test/parallel/test-debug-brk.js diff --git a/test/parallel/test-debug-brk.js b/test/parallel/test-debug-brk.js new file mode 100644 index 00000000000000..49b19898e030b9 --- /dev/null +++ b/test/parallel/test-debug-brk.js @@ -0,0 +1,9 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const spawnSync = require('child_process').spawnSync; + +const args = [`--debug-brk=${common.PORT}`, `-e`, `0`]; +const proc = spawnSync(process.execPath, args, {encoding: 'utf8'}); +assert(/Debugger listening on/.test(proc.stderr));