From fce175bb1f5961710be72511cf4b85b1670b2742 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 20 Jan 2018 15:11:03 +0100 Subject: [PATCH 1/4] src: don't abort when package.json is a directory PR-URL: https://github.com/nodejs/node/pull/18270 Fixes: https://github.com/nodejs/node/issues/8307 Reviewed-By: Richard Lau Reviewed-By: Bradley Farias Reviewed-By: Anna Henningsen Reviewed-By: Guy Bedford Reviewed-By: Matteo Collina --- src/node_file.cc | 14 +++++++++----- .../packages/is-dir/package.json/.placeholder | 0 test/parallel/test-module-loading-error.js | 7 +++++++ 3 files changed, 16 insertions(+), 5 deletions(-) create mode 100644 test/fixtures/packages/is-dir/package.json/.placeholder diff --git a/src/node_file.cc b/src/node_file.cc index 9df13be5bd2dff..2790f274100a60 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -486,6 +486,12 @@ static void InternalModuleReadFile(const FunctionCallbackInfo& args) { return; } + std::shared_ptr defer_close(nullptr, [fd, loop] (...) { + uv_fs_t close_req; + CHECK_EQ(0, uv_fs_close(loop, &close_req, fd, nullptr)); + uv_fs_req_cleanup(&close_req); + }); + const size_t kBlockSize = 32 << 10; std::vector chars; int64_t offset = 0; @@ -502,14 +508,12 @@ static void InternalModuleReadFile(const FunctionCallbackInfo& args) { numchars = uv_fs_read(loop, &read_req, fd, &buf, 1, offset, nullptr); uv_fs_req_cleanup(&read_req); - CHECK_GE(numchars, 0); + if (numchars < 0) + return; + offset += numchars; } while (static_cast(numchars) == kBlockSize); - uv_fs_t close_req; - CHECK_EQ(0, uv_fs_close(loop, &close_req, fd, nullptr)); - uv_fs_req_cleanup(&close_req); - size_t start = 0; if (offset >= 3 && 0 == memcmp(&chars[0], "\xEF\xBB\xBF", 3)) { start = 3; // Skip UTF-8 BOM. diff --git a/test/fixtures/packages/is-dir/package.json/.placeholder b/test/fixtures/packages/is-dir/package.json/.placeholder new file mode 100644 index 00000000000000..e69de29bb2d1d6 diff --git a/test/parallel/test-module-loading-error.js b/test/parallel/test-module-loading-error.js index f1c457af2b2c9e..11af0225830257 100644 --- a/test/parallel/test-module-loading-error.js +++ b/test/parallel/test-module-loading-error.js @@ -72,3 +72,10 @@ common.expectsError( code: 'ERR_ASSERTION', message: /^path must be a string$/ }); + +common.expectsError( + () => { require('../fixtures/packages/is-dir'); }, + { + code: 'MODULE_NOT_FOUND', + message: 'Cannot find module \'../fixtures/packages/is-dir\'' + }); From 2d45c3140d25d90b7cda9608a45d2c50ee0edd3a Mon Sep 17 00:00:00 2001 From: Ruben Bridgewater Date: Tue, 13 Feb 2018 06:09:31 +0100 Subject: [PATCH 2/4] test,benchmark,doc: enable dot-notation rule This enables the eslint dot-notation rule for all code instead of only in /lib. PR-URL: https://github.com/nodejs/node/pull/18749 Reviewed-By: Luigi Pinca Reviewed-By: James M Snell Reviewed-By: Matheus Marchini --- shas | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 shas diff --git a/shas b/shas new file mode 100644 index 00000000000000..c8b2c66deb6736 --- /dev/null +++ b/shas @@ -0,0 +1,10 @@ +de0ed84f04 +469036add4 +2caa1f5458 +e83adf87f5 +808c05858a +1e57a8d117 +0089860757 +92bf2492cd +7514eb3cff +6934792eb3 From 4f1b7459a239a9980539e2a8fc3656439a7058d9 Mon Sep 17 00:00:00 2001 From: Bartosz Sosnowski Date: Wed, 7 Feb 2018 10:33:51 +0100 Subject: [PATCH 3/4] test: stdio pipe behavior tests Add two regression tests for stdio over pipes. test-stdio-pipe-access tests if accessing stdio pipe that is being read by another process does not deadlocks Node.js. This was reported in https://github.com/nodejs/node/issues/10836 and was fixed in v8.3.0. The deadlock would happen intermittently, so we run the test 5 times. test-stdio-pipe-redirect tests if redirecting one child process stdin to another process stdout does not crash Node as reported in https://github.com/nodejs/node/issues/17493. It was fixed in https://github.com/nodejs/node/pull/18019. PR-URL: https://github.com/nodejs/node/pull/18614 Reviewed-By: James M Snell Reviewed-By: Ruben Bridgewater --- test/parallel/test-stdio-pipe-access.js | 35 +++++++++++++++++++++ test/parallel/test-stdio-pipe-redirect.js | 38 +++++++++++++++++++++++ 2 files changed, 73 insertions(+) create mode 100644 test/parallel/test-stdio-pipe-access.js create mode 100644 test/parallel/test-stdio-pipe-redirect.js diff --git a/test/parallel/test-stdio-pipe-access.js b/test/parallel/test-stdio-pipe-access.js new file mode 100644 index 00000000000000..ef84bb83803b26 --- /dev/null +++ b/test/parallel/test-stdio-pipe-access.js @@ -0,0 +1,35 @@ +'use strict'; +require('../common'); + +// Test if Node handles acessing process.stdin if it is a redirected +// pipe without deadlocking +const { spawn, spawnSync } = require('child_process'); + +const numTries = 5; +const who = process.argv.length <= 2 ? 'runner' : process.argv[2]; + +switch (who) { + case 'runner': + for (let num = 0; num < numTries; ++num) { + spawnSync(process.argv0, + [process.argv[1], 'parent'], + { 'stdio': 'inherit' }); + } + break; + case 'parent': + const middle = spawn(process.argv0, + [process.argv[1], 'middle'], + { 'stdio': 'pipe' }); + middle.stdout.on('data', () => {}); + break; + case 'middle': + spawn(process.argv0, + [process.argv[1], 'bottom'], + { 'stdio': [ process.stdin, + process.stdout, + process.stderr ] }); + break; + case 'bottom': + process.stdin; + break; +} diff --git a/test/parallel/test-stdio-pipe-redirect.js b/test/parallel/test-stdio-pipe-redirect.js new file mode 100644 index 00000000000000..b47f5b9cf44af5 --- /dev/null +++ b/test/parallel/test-stdio-pipe-redirect.js @@ -0,0 +1,38 @@ +'use strict'; +require('../common'); + +// Test if Node handles redirecting one child process stdout to another +// process stdin without crashing. +const spawn = require('child_process').spawn; + +const writeSize = 100; +const totalDots = 10000; + +const who = process.argv.length <= 2 ? 'parent' : process.argv[2]; + +switch (who) { + case 'parent': + const consumer = spawn(process.argv0, [process.argv[1], 'consumer'], { + stdio: ['pipe', 'ignore', 'inherit'], + }); + const producer = spawn(process.argv0, [process.argv[1], 'producer'], { + stdio: ['pipe', consumer.stdin, 'inherit'], + }); + process.stdin.on('data', () => {}); + producer.on('exit', process.exit); + break; + case 'producer': + const buffer = Buffer.alloc(writeSize, '.'); + let written = 0; + const write = () => { + if (written < totalDots) { + written += writeSize; + process.stdout.write(buffer, write); + } + }; + write(); + break; + case 'consumer': + process.stdin.on('data', () => {}); + break; +} From 4aef7dfb1e6b110f189b98badaf709adec770d16 Mon Sep 17 00:00:00 2001 From: Yihong Wang Date: Wed, 17 Jan 2018 13:16:14 -0800 Subject: [PATCH 4/4] build: include the libuv and zlib into node Add libuv and zlib into node executable and shared lib. Also fix an issue that openssl is not fully included in node executable for macOS. Signed-off-by: Yihong Wang Fixes: https://github.com/nodejs/node/issues/17444 PR-URL: https://github.com/nodejs/node/pull/18383 Reviewed-By: Anna Henningsen Reviewed-By: Gireesh Punathil Reviewed-By: Ben Noordhuis Reviewed-By: Michael Dawson Reviewed-By: James M Snell --- node.gyp | 2 +- node.gypi | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/node.gyp b/node.gyp index 0e9db9a16eea9d..1da1f5e70cfe89 100644 --- a/node.gyp +++ b/node.gyp @@ -227,7 +227,7 @@ }, }, 'conditions': [ - ['OS in "linux freebsd openbsd solaris android"', { + ['OS!="aix"', { 'ldflags': [ '-Wl,--whole-archive,<(OBJ_DIR)/<(STATIC_LIB_PREFIX)' '<(node_core_target_name)<(STATIC_LIB_SUFFIX)', diff --git a/node.gypi b/node.gypi index 386601906fbe4a..c304ac7f74bc6f 100644 --- a/node.gypi +++ b/node.gypi @@ -107,6 +107,32 @@ }], [ 'node_shared_zlib=="false"', { 'dependencies': [ 'deps/zlib/zlib.gyp:zlib' ], + 'conditions': [ + [ 'force_load=="true"', { + 'xcode_settings': { + 'OTHER_LDFLAGS': [ + '-Wl,-force_load,<(PRODUCT_DIR)/<(STATIC_LIB_PREFIX)' + 'zlib<(STATIC_LIB_SUFFIX)', + ], + }, + 'msvs_settings': { + 'VCLinkerTool': { + 'AdditionalOptions': [ + '/WHOLEARCHIVE:<(PRODUCT_DIR)\\lib\\zlib<(STATIC_LIB_SUFFIX)', + ], + }, + }, + 'conditions': [ + ['OS!="aix" and node_shared=="false"', { + 'ldflags': [ + '-Wl,--whole-archive,<(OBJ_DIR)/deps/zlib/<(STATIC_LIB_PREFIX)' + 'zlib<(STATIC_LIB_SUFFIX)', + '-Wl,--no-whole-archive', + ], + }], + ], + }], + ], }], [ 'node_shared_http_parser=="false"', { @@ -119,6 +145,32 @@ [ 'node_shared_libuv=="false"', { 'dependencies': [ 'deps/uv/uv.gyp:libuv' ], + 'conditions': [ + [ 'force_load=="true"', { + 'xcode_settings': { + 'OTHER_LDFLAGS': [ + '-Wl,-force_load,<(PRODUCT_DIR)/<(STATIC_LIB_PREFIX)' + 'uv<(STATIC_LIB_SUFFIX)', + ], + }, + 'msvs_settings': { + 'VCLinkerTool': { + 'AdditionalOptions': [ + '/WHOLEARCHIVE:<(PRODUCT_DIR)\\lib\\libuv<(STATIC_LIB_SUFFIX)', + ], + }, + }, + 'conditions': [ + ['OS!="aix" and node_shared=="false"', { + 'ldflags': [ + '-Wl,--whole-archive,<(OBJ_DIR)/deps/uv/<(STATIC_LIB_PREFIX)' + 'uv<(STATIC_LIB_SUFFIX)', + '-Wl,--no-whole-archive', + ], + }], + ], + }], + ], }], [ 'node_shared_nghttp2=="false"', { @@ -240,12 +292,18 @@ '-Wl,-force_load,<(PRODUCT_DIR)/<(OPENSSL_PRODUCT)', ], }, + 'msvs_settings': { + 'VCLinkerTool': { + 'AdditionalOptions': [ + '/WHOLEARCHIVE:<(PRODUCT_DIR)\\lib\\<(OPENSSL_PRODUCT)', + ], + }, + }, 'conditions': [ ['OS in "linux freebsd" and node_shared=="false"', { 'ldflags': [ '-Wl,--whole-archive,' - '<(OBJ_DIR)/deps/openssl/' - '<(OPENSSL_PRODUCT)', + '<(OBJ_DIR)/deps/openssl/<(OPENSSL_PRODUCT)', '-Wl,--no-whole-archive', ], }],