From 37c97b1cb7f6aa462b79b33c7fd23472fa2f2148 Mon Sep 17 00:00:00 2001 From: Omar Crisostomo Date: Tue, 26 Dec 2017 22:16:45 -0600 Subject: [PATCH 01/23] test: add test-fs-read-constructor-errors test test added to extend the code coverage testing of the project --- .../test-fs-read-constructor-errors.js | 107 ++++++++++++++++++ 1 file changed, 107 insertions(+) create mode 100644 test/parallel/test-fs-read-constructor-errors.js diff --git a/test/parallel/test-fs-read-constructor-errors.js b/test/parallel/test-fs-read-constructor-errors.js new file mode 100644 index 00000000000000..8441aa7e658448 --- /dev/null +++ b/test/parallel/test-fs-read-constructor-errors.js @@ -0,0 +1,107 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const fs = require('fs'); +const filepath = fixtures.path('x.txt'); +const fd = fs.openSync(filepath, 'r'); + +const expected = Buffer.from('xyz\n'); + +//fs.read expects incorrect fd parameter +function testFd(bufferAsync, offset, bufferSync, expected) { + common.expectsError( + fs.read.bind(fd, + bufferAsync, + offset, + expected.length, + 0, + common.mustNotCall()), + { code: 'ERR_INVALID_ARG_TYPE', type: TypeError } + ); +} + +//fs.read expects fd parameter out of range +function testFdRangeOut(bufferAsync, offset, bufferSync, expected) { + common.expectsError( + fs.read.bind(undefined, + -1, + bufferAsync, + offset, + expected.length, + 0, + common.mustNotCall()), + { code: 'ERR_OUT_OF_RANGE', type: RangeError } + ); +} + +//fs.read expects offset parameter out of range +function testOffSet(bufferAsync, offset, length, expected) { + common.expectsError( + fs.read.bind(undefined, + fd, + bufferAsync, + offset, + expected.length, + 0, + common.mustNotCall()), + { code: 'ERR_OUT_OF_RANGE', type: RangeError } + ); +} + +//fs.read expects length parameter out of range +function testLenght(bufferAsync, offset, length) { + common.expectsError( + fs.read.bind(undefined, + fd, + bufferAsync, + offset, + length, + 0, + common.mustNotCall()), + { code: 'ERR_OUT_OF_RANGE', type: RangeError } + ); +} + +//test fd parameter +testFd(Buffer.allocUnsafe(expected.length), + 0, + Buffer.allocUnsafe(expected.length), + expected); + +//test fd parameter out of range +testFdRangeOut(Buffer.allocUnsafe(expected.length), + 0, + Buffer.allocUnsafe(expected.length), + expected); + +//test offset parameter out of range +testOffSet(Buffer.allocUnsafe(expected.length), + -1, + Buffer.allocUnsafe(expected.length), + expected); + +//test length parameter out of range +testLenght(Buffer.allocUnsafe(expected.length), + 0, + -1); From d7326b245437deb004bda8ac673bcaf78df5e2cc Mon Sep 17 00:00:00 2001 From: Omar Crisostomo Date: Tue, 26 Dec 2017 22:17:17 -0600 Subject: [PATCH 02/23] test: add test-fs-readSync-constructor-errors test test added to extend the code coverage testing of the project --- .../test-fs-readSyc-constructor-errors.js | 103 ++++++++++++++++++ 1 file changed, 103 insertions(+) create mode 100644 test/parallel/test-fs-readSyc-constructor-errors.js diff --git a/test/parallel/test-fs-readSyc-constructor-errors.js b/test/parallel/test-fs-readSyc-constructor-errors.js new file mode 100644 index 00000000000000..45722ae0ad11bd --- /dev/null +++ b/test/parallel/test-fs-readSyc-constructor-errors.js @@ -0,0 +1,103 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const fs = require('fs'); +const filepath = fixtures.path('x.txt'); +const fd = fs.openSync(filepath, 'r'); + +const expected = Buffer.from('xyz\n'); + +//fs.readSync expects incorrect fd parameter +function testFd(bufferAsync, offset, bufferSync, expected) { + common.expectsError( + fs.readSync.bind(fd, + bufferAsync, + offset, + expected.length, + 0), + { code: 'ERR_INVALID_ARG_TYPE', type: TypeError } + ); +} + +//fs.readSync expects fd parameter out of range +function testFdRangeOut(bufferAsync, offset, bufferSync, expected) { + common.expectsError( + fs.readSync.bind(undefined, + -1, + bufferAsync, + offset, + expected.length, + 0), + { code: 'ERR_OUT_OF_RANGE', type: RangeError } + ); +} + +//fs.readSync expects offset parameter out of range +function testOffSet(bufferAsync, offset, length, expected) { + common.expectsError( + fs.readSync.bind(undefined, + fd, + bufferAsync, + offset, + expected.length, + 0), + { code: 'ERR_OUT_OF_RANGE', type: RangeError } + ); +} + +//fs.readSync expects length parameter out of range +function testLenght(bufferAsync, offset, length) { + common.expectsError( + fs.readSync.bind(undefined, + fd, + bufferAsync, + offset, + length, + 0), + { code: 'ERR_OUT_OF_RANGE', type: RangeError } + ); +} + +//test fd parameter +testFd(Buffer.allocUnsafe(expected.length), + 0, + Buffer.allocUnsafe(expected.length), + expected); + +//test fd parameter out of range +testFdRangeOut(Buffer.allocUnsafe(expected.length), + 0, + Buffer.allocUnsafe(expected.length), + expected); + +//test offset parameter out of range +testOffSet(Buffer.allocUnsafe(expected.length), + -1, + Buffer.allocUnsafe(expected.length), + expected); + +//test length parameter out of range +testLenght(Buffer.allocUnsafe(expected.length), + 0, + -1); From bd3aa3e0489be72dc3cd3ea945d43900e792bb35 Mon Sep 17 00:00:00 2001 From: Omar Crisostomo Date: Wed, 27 Dec 2017 03:09:07 -0600 Subject: [PATCH 03/23] test: add test-fs-read-constructor-errors test test added to extend the code coverage testing of the project --- .../test-fs-read-constructor-errors.js | 99 +++++-------------- 1 file changed, 25 insertions(+), 74 deletions(-) diff --git a/test/parallel/test-fs-read-constructor-errors.js b/test/parallel/test-fs-read-constructor-errors.js index 8441aa7e658448..5c47063d74c396 100644 --- a/test/parallel/test-fs-read-constructor-errors.js +++ b/test/parallel/test-fs-read-constructor-errors.js @@ -28,80 +28,31 @@ const fd = fs.openSync(filepath, 'r'); const expected = Buffer.from('xyz\n'); -//fs.read expects incorrect fd parameter -function testFd(bufferAsync, offset, bufferSync, expected) { - common.expectsError( - fs.read.bind(fd, - bufferAsync, - offset, - expected.length, - 0, - common.mustNotCall()), - { code: 'ERR_INVALID_ARG_TYPE', type: TypeError } - ); -} - -//fs.read expects fd parameter out of range -function testFdRangeOut(bufferAsync, offset, bufferSync, expected) { - common.expectsError( - fs.read.bind(undefined, - -1, - bufferAsync, - offset, - expected.length, - 0, - common.mustNotCall()), - { code: 'ERR_OUT_OF_RANGE', type: RangeError } - ); -} - -//fs.read expects offset parameter out of range -function testOffSet(bufferAsync, offset, length, expected) { - common.expectsError( - fs.read.bind(undefined, - fd, - bufferAsync, - offset, - expected.length, - 0, - common.mustNotCall()), - { code: 'ERR_OUT_OF_RANGE', type: RangeError } - ); -} - -//fs.read expects length parameter out of range -function testLenght(bufferAsync, offset, length) { - common.expectsError( - fs.read.bind(undefined, - fd, - bufferAsync, - offset, - length, - 0, - common.mustNotCall()), - { code: 'ERR_OUT_OF_RANGE', type: RangeError } - ); -} - -//test fd parameter -testFd(Buffer.allocUnsafe(expected.length), - 0, - Buffer.allocUnsafe(expected.length), - expected); - -//test fd parameter out of range -testFdRangeOut(Buffer.allocUnsafe(expected.length), - 0, +common.expectsError( + fs.read.bind(fd, Buffer.allocUnsafe(expected.length), - expected); + 0, + expected.length, + 0, + common.mustNotCall()), + { code: 'ERR_INVALID_ARG_TYPE', type: TypeError }); -//test offset parameter out of range -testOffSet(Buffer.allocUnsafe(expected.length), - -1, - Buffer.allocUnsafe(expected.length), - expected); +common.expectsError( + fs.read.bind(undefined, + fd, + Buffer.allocUnsafe(expected.length), + -1, + expected.length, + 0, + common.mustNotCall()), + { code: 'ERR_OUT_OF_RANGE', type: RangeError }); -//test length parameter out of range -testLenght(Buffer.allocUnsafe(expected.length), - 0, - -1); +common.expectsError( + fs.read.bind(undefined, + fd, + Buffer.allocUnsafe(expected.length), + 0, + -1, + 0, + common.mustNotCall()), + { code: 'ERR_OUT_OF_RANGE', type: RangeError }); From 5e59b4fab76ef108acec8e882c5c09fb16d65e6c Mon Sep 17 00:00:00 2001 From: Omar Crisostomo Date: Wed, 27 Dec 2017 03:09:42 -0600 Subject: [PATCH 04/23] test: add test-fs-readSync-constructor-errors test test added to extend the code coverage testing of the project --- .../test-fs-readSyc-constructor-errors.js | 98 +++++-------------- 1 file changed, 25 insertions(+), 73 deletions(-) diff --git a/test/parallel/test-fs-readSyc-constructor-errors.js b/test/parallel/test-fs-readSyc-constructor-errors.js index 45722ae0ad11bd..17ddc5454eb7cc 100644 --- a/test/parallel/test-fs-readSyc-constructor-errors.js +++ b/test/parallel/test-fs-readSyc-constructor-errors.js @@ -28,76 +28,28 @@ const fd = fs.openSync(filepath, 'r'); const expected = Buffer.from('xyz\n'); -//fs.readSync expects incorrect fd parameter -function testFd(bufferAsync, offset, bufferSync, expected) { - common.expectsError( - fs.readSync.bind(fd, - bufferAsync, - offset, - expected.length, - 0), - { code: 'ERR_INVALID_ARG_TYPE', type: TypeError } - ); -} - -//fs.readSync expects fd parameter out of range -function testFdRangeOut(bufferAsync, offset, bufferSync, expected) { - common.expectsError( - fs.readSync.bind(undefined, - -1, - bufferAsync, - offset, - expected.length, - 0), - { code: 'ERR_OUT_OF_RANGE', type: RangeError } - ); -} - -//fs.readSync expects offset parameter out of range -function testOffSet(bufferAsync, offset, length, expected) { - common.expectsError( - fs.readSync.bind(undefined, - fd, - bufferAsync, - offset, - expected.length, - 0), - { code: 'ERR_OUT_OF_RANGE', type: RangeError } - ); -} - -//fs.readSync expects length parameter out of range -function testLenght(bufferAsync, offset, length) { - common.expectsError( - fs.readSync.bind(undefined, - fd, - bufferAsync, - offset, - length, - 0), - { code: 'ERR_OUT_OF_RANGE', type: RangeError } - ); -} - -//test fd parameter -testFd(Buffer.allocUnsafe(expected.length), - 0, - Buffer.allocUnsafe(expected.length), - expected); - -//test fd parameter out of range -testFdRangeOut(Buffer.allocUnsafe(expected.length), - 0, - Buffer.allocUnsafe(expected.length), - expected); - -//test offset parameter out of range -testOffSet(Buffer.allocUnsafe(expected.length), - -1, - Buffer.allocUnsafe(expected.length), - expected); - -//test length parameter out of range -testLenght(Buffer.allocUnsafe(expected.length), - 0, - -1); +common.expectsError( + fs.readSync.bind(fd, + Buffer.allocUnsafe(expected.length), + 0, + expected.length, + 0), + { code: 'ERR_INVALID_ARG_TYPE', type: TypeError }); + +common.expectsError( + fs.readSync.bind(undefined, + fd, + Buffer.allocUnsafe(expected.length), + -1, + expected.length, + 0), + { code: 'ERR_OUT_OF_RANGE', type: RangeError }); + +common.expectsError( + fs.readSync.bind(undefined, + 1, + Buffer.allocUnsafe(expected.length), + 0, + -1, + 0), + { code: 'ERR_OUT_OF_RANGE', type: RangeError }); From 9ec700b073894ad54432db4c4b540e2b61b45325 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 25 Dec 2017 05:00:24 +0800 Subject: [PATCH 05/23] fs: validate path in fs.readFile PR-URL: https://github.com/nodejs/node/pull/17852 Refs: https://github.com/nodejs/node/pull/17667 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Matteo Collina --- lib/fs.js | 3 +++ test/parallel/test-fs-readfile-error.js | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/lib/fs.js b/lib/fs.js index 7741e5838bd216..e41d0db03390b0 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -389,6 +389,9 @@ fs.readFile = function(path, options, callback) { req.oncomplete(null, path); }); return; + } else if (typeof path !== 'string' && !(path instanceof Buffer)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'path', + ['string', 'Buffer', 'URL']); } binding.open(pathModule.toNamespacedPath(path), diff --git a/test/parallel/test-fs-readfile-error.js b/test/parallel/test-fs-readfile-error.js index 616760b06695a1..9b1a06183d7530 100644 --- a/test/parallel/test-fs-readfile-error.js +++ b/test/parallel/test-fs-readfile-error.js @@ -21,6 +21,7 @@ 'use strict'; const common = require('../common'); +const fs = require('fs'); // Test that fs.readFile fails correctly on a non-existent file. @@ -54,3 +55,12 @@ test({ NODE_DEBUG: 'fs' }, common.mustCall((data) => { assert(/EISDIR/.test(data)); assert(/test-fs-readfile-error/.test(data)); })); + +common.expectsError( + () => { fs.readFile(() => {}); }, + { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "path" argument must be one of type string, Buffer, or URL', + type: TypeError + } +); From 71396a200d75b11a71af7caae18d12f3dc461d07 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 25 Dec 2017 05:01:11 +0800 Subject: [PATCH 06/23] fs: validate path in fs.exists{Sync} PR-URL: https://github.com/nodejs/node/pull/17852 Refs: https://github.com/nodejs/node/pull/17667 Reviewed-By: James M Snell Reviewed-By: Colin Ihrig Reviewed-By: Matteo Collina --- lib/fs.js | 7 +++++++ test/parallel/test-fs-exists.js | 11 +++++++++++ 2 files changed, 18 insertions(+) diff --git a/lib/fs.js b/lib/fs.js index e41d0db03390b0..93be37533405da 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -340,6 +340,10 @@ fs.accessSync = function(path, mode) { fs.exists = function(path, callback) { if (handleError((path = getPathFromURL(path)), cb)) return; + if (typeof path !== 'string' && !(path instanceof Buffer)) { + throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'path', + ['string', 'Buffer', 'URL']); + } if (!nullCheck(path, cb)) return; var req = new FSReqWrap(); req.oncomplete = cb; @@ -361,6 +365,9 @@ Object.defineProperty(fs.exists, internalUtil.promisify.custom, { fs.existsSync = function(path) { try { handleError((path = getPathFromURL(path))); + if (typeof path !== 'string' && !(path instanceof Buffer)) { + return false; + } nullCheck(path); binding.stat(pathModule.toNamespacedPath(path)); return true; diff --git a/test/parallel/test-fs-exists.js b/test/parallel/test-fs-exists.js index 331c8c04e38e78..a526373c6a876d 100644 --- a/test/parallel/test-fs-exists.js +++ b/test/parallel/test-fs-exists.js @@ -42,3 +42,14 @@ fs.exists(new URL('https://foo'), common.mustCall(function(y) { assert(fs.existsSync(f)); assert(!fs.existsSync(`${f}-NO`)); + +common.expectsError( + () => { fs.exists(() => {}); }, + { + code: 'ERR_INVALID_ARG_TYPE', + message: 'The "path" argument must be one of type string, Buffer, or URL', + type: TypeError + } +); + +assert(!fs.existsSync()); From c0e9cded1ec9daeec178d536b39acf944f80809c Mon Sep 17 00:00:00 2001 From: Eugene Ostroukhov Date: Wed, 13 Dec 2017 09:24:35 -0800 Subject: [PATCH 07/23] inspector: make Coverity happy PR-URL: https://github.com/nodejs/node/pull/17656 Reviewed-By: Colin Ihrig Reviewed-By: Timothy Gu Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- src/inspector_socket.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/src/inspector_socket.cc b/src/inspector_socket.cc index 386b4c3af07a2d..ad8b502da307c0 100644 --- a/src/inspector_socket.cc +++ b/src/inspector_socket.cc @@ -116,6 +116,7 @@ class WriteRequest { WriteRequest(ProtocolHandler* handler, const std::vector& buffer) : handler(handler) , storage(buffer) + , req(uv_write_t()) , buf(uv_buf_init(storage.data(), storage.size())) {} static WriteRequest* from_write_req(uv_write_t* req) { From b0dd43cd6379a77a4e437f6ba1fdda7c0b4b744a Mon Sep 17 00:00:00 2001 From: Dmitriy Kasyanov Date: Wed, 13 Dec 2017 23:21:24 +0200 Subject: [PATCH 08/23] tools: fix AttributeError: __exit__ on Python 2.6 Error occurs while dealing with Tar archives PR-URL: https://github.com/nodejs/node/pull/17663 Reviewed-By: Anna Henningsen Reviewed-By: James M Snell Reviewed-By: Steven R Loomis Reviewed-By: Colin Ihrig --- tools/configure.d/nodedownload.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/configure.d/nodedownload.py b/tools/configure.d/nodedownload.py index 5cf8e0dbd690e1..3f4bc090f71bdd 100644 --- a/tools/configure.d/nodedownload.py +++ b/tools/configure.d/nodedownload.py @@ -60,7 +60,7 @@ def unpack(packedfile, parent_path): icuzip.extractall(parent_path) return parent_path elif tarfile.is_tarfile(packedfile): - with tarfile.TarFile.open(packedfile, 'r') as icuzip: + with contextlib.closing(tarfile.TarFile.open(packedfile, 'r')) as icuzip: print ' Extracting tarfile: %s' % packedfile icuzip.extractall(parent_path) return parent_path From 6c0da349056578d36254d1aed973ddbaa252ce89 Mon Sep 17 00:00:00 2001 From: XadillaX Date: Thu, 14 Dec 2017 12:40:32 +0800 Subject: [PATCH 09/23] http: add rawPacket in err of `clientError` event The `rawPacket` is the current buffer that just parsed. Adding this buffer to the error object of `clientError` event is to make it possible that developers can log the broken packet. PR-URL: https://github.com/nodejs/node/pull/17672 Reviewed-By: Anna Henningsen Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell --- doc/api/http.md | 11 +++++++++++ lib/_http_server.js | 1 + test/parallel/test-http-server-client-error.js | 6 ++++++ 3 files changed, 18 insertions(+) diff --git a/doc/api/http.md b/doc/api/http.md index 9f06296e6e2584..928ed0d972b472 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -734,6 +734,11 @@ changes: description: The default action of calling `.destroy()` on the `socket` will no longer take place if there are listeners attached for `clientError`. + - version: REPLACEME + pr-url: https://github.com/nodejs/node/pull/17672 + description: The rawPacket is the current buffer that just parsed. Adding + this buffer to the error object of clientError event is to make + it possible that developers can log the broken packet. --> * `exception` {Error} @@ -765,6 +770,12 @@ object, so any HTTP response sent, including response headers and payload, *must* be written directly to the `socket` object. Care must be taken to ensure the response is a properly formatted HTTP response message. +`err` is an instance of `Error` with two extra columns: + ++ `bytesParsed`: the bytes count of request packet that Node.js may have parsed + correctly; ++ `rawPacket`: the raw packet of current request. + ### Event: 'close'