From 96f28536c4fab39eba9bcd35a5b3b512b358fa9e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 19 Feb 2018 17:36:59 +0800 Subject: [PATCH 1/2] test: introduce common.runWithInvalidFD() This provides a more reliable way to get a fd that can be used to tirgger EBADF. --- test/common/README.md | 8 ++++++++ test/common/index.js | 13 +++++++++++++ 2 files changed, 21 insertions(+) diff --git a/test/common/README.md b/test/common/README.md index e0a66e9da0c2c9..cdccec06d36604 100644 --- a/test/common/README.md +++ b/test/common/README.md @@ -139,6 +139,14 @@ consisting of all `ArrayBufferView` and an `ArrayBuffer`. Returns the file name and line number for the provided Function. +### runWithInvalidFD(func) +* `func` [<Function>] + +Runs `func` with an invalid file descriptor that is an unsigned integer and +can be used to trigger `EBADF` as the first argument. If no such file +descriptor could be generated, a skip message will be printed and the `func` +will not be run. + ### globalCheck * [<Boolean>] diff --git a/test/common/index.js b/test/common/index.js index b24d2158e7d089..123db241ef8199 100644 --- a/test/common/index.js +++ b/test/common/index.js @@ -799,6 +799,19 @@ function restoreWritable(name) { delete process[name].writeTimes; } +exports.runWithInvalidFD = function(func) { + let fd = 1 << 30; + // Get first known bad file descriptor. 1 << 30 is usually unlikely to + // be an valid one. + try { + while (fs.fstatSync(fd--) && fd > 0); + } catch (e) { + return func(fd); + } + + exports.printSkipMessage('Could not generate an invalid fd'); +}; + exports.hijackStdout = hijackStdWritable.bind(null, 'stdout'); exports.hijackStderr = hijackStdWritable.bind(null, 'stderr'); exports.restoreStdout = restoreWritable.bind(null, 'stdout'); From 4202c5ea3da34c08bec08935ed5fb2909f78742d Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Mon, 19 Feb 2018 17:38:11 +0800 Subject: [PATCH 2/2] test: use runWithInvalidFD() in tests expecting EBADF --- test/parallel/test-fs-close-errors.js | 40 ------------ test/parallel/test-fs-error-messages.js | 83 ++++++++++++++---------- test/parallel/test-ttywrap-invalid-fd.js | 19 ++---- 3 files changed, 56 insertions(+), 86 deletions(-) diff --git a/test/parallel/test-fs-close-errors.js b/test/parallel/test-fs-close-errors.js index 4f0d31369a2651..f81d96c0569990 100644 --- a/test/parallel/test-fs-close-errors.js +++ b/test/parallel/test-fs-close-errors.js @@ -4,9 +4,7 @@ // include the desired properties const common = require('../common'); -const assert = require('assert'); const fs = require('fs'); -const uv = process.binding('uv'); ['', false, null, undefined, {}, []].forEach((i) => { common.expectsError( @@ -26,41 +24,3 @@ const uv = process.binding('uv'); } ); }); - -{ - assert.throws( - () => { - const fd = fs.openSync(__filename, 'r'); - fs.closeSync(fd); - fs.closeSync(fd); - }, - (err) => { - assert.strictEqual(err.code, 'EBADF'); - assert.strictEqual( - err.message, - 'EBADF: bad file descriptor, close' - ); - assert.strictEqual(err.constructor, Error); - assert.strictEqual(err.syscall, 'close'); - assert.strictEqual(err.errno, uv.UV_EBADF); - return true; - } - ); -} - -{ - const fd = fs.openSync(__filename, 'r'); - fs.close(fd, common.mustCall((err) => { - assert.ifError(err); - fs.close(fd, common.mustCall((err) => { - assert.strictEqual(err.code, 'EBADF'); - assert.strictEqual( - err.message, - 'EBADF: bad file descriptor, close' - ); - assert.strictEqual(err.constructor, Error); - assert.strictEqual(err.syscall, 'close'); - assert.strictEqual(err.errno, uv.UV_EBADF); - })); - })); -} diff --git a/test/parallel/test-fs-error-messages.js b/test/parallel/test-fs-error-messages.js index ba44c28d432983..28461de9bee9b2 100644 --- a/test/parallel/test-fs-error-messages.js +++ b/test/parallel/test-fs-error-messages.js @@ -94,15 +94,14 @@ function re(literals, ...values) { return true; }; - const fd = fs.openSync(existingFile, 'r'); - fs.closeSync(fd); - - fs.fstat(fd, common.mustCall(validateError)); - - assert.throws( - () => fs.fstatSync(fd), - validateError - ); + common.runWithInvalidFD((fd) => { + fs.fstat(fd, common.mustCall(validateError)); + + assert.throws( + () => fs.fstatSync(fd), + validateError + ); + }); } // realpath @@ -414,6 +413,27 @@ function re(literals, ...values) { ); } + +// close +{ + const validateError = (err) => { + assert.strictEqual(err.message, 'EBADF: bad file descriptor, close'); + assert.strictEqual(err.errno, uv.UV_EBADF); + assert.strictEqual(err.code, 'EBADF'); + assert.strictEqual(err.syscall, 'close'); + return true; + }; + + common.runWithInvalidFD((fd) => { + fs.close(fd, common.mustCall(validateError)); + + assert.throws( + () => fs.closeSync(fd), + validateError + ); + }); +} + // readFile { const validateError = (err) => { @@ -472,15 +492,14 @@ function re(literals, ...values) { return true; }; - const fd = fs.openSync(existingFile, 'r'); - fs.closeSync(fd); + common.runWithInvalidFD((fd) => { + fs.ftruncate(fd, 4, common.mustCall(validateError)); - fs.ftruncate(fd, 4, common.mustCall(validateError)); - - assert.throws( - () => fs.ftruncateSync(fd, 4), - validateError - ); + assert.throws( + () => fs.ftruncateSync(fd, 4), + validateError + ); + }); } // fdatasync @@ -493,15 +512,14 @@ function re(literals, ...values) { return true; }; - const fd = fs.openSync(existingFile, 'r'); - fs.closeSync(fd); - - fs.fdatasync(fd, common.mustCall(validateError)); + common.runWithInvalidFD((fd) => { + fs.fdatasync(fd, common.mustCall(validateError)); - assert.throws( - () => fs.fdatasyncSync(fd), - validateError - ); + assert.throws( + () => fs.fdatasyncSync(fd), + validateError + ); + }); } // fsync @@ -514,13 +532,12 @@ function re(literals, ...values) { return true; }; - const fd = fs.openSync(existingFile, 'r'); - fs.closeSync(fd); - - fs.fsync(fd, common.mustCall(validateError)); + common.runWithInvalidFD((fd) => { + fs.fsync(fd, common.mustCall(validateError)); - assert.throws( - () => fs.fsyncSync(fd), - validateError - ); + assert.throws( + () => fs.fsyncSync(fd), + validateError + ); + }); } diff --git a/test/parallel/test-ttywrap-invalid-fd.js b/test/parallel/test-ttywrap-invalid-fd.js index 6564d47fd938a6..adf88cbde659ce 100644 --- a/test/parallel/test-ttywrap-invalid-fd.js +++ b/test/parallel/test-ttywrap-invalid-fd.js @@ -2,7 +2,6 @@ // Flags: --expose-internals const common = require('../common'); -const fs = require('fs'); const tty = require('tty'); const { SystemError } = require('internal/errors'); @@ -22,12 +21,9 @@ common.expectsError( common.expectsError( () => { - let fd = 2; - // Get first known bad file descriptor. - try { - while (fs.fstatSync(++fd)); - } catch (e) { } - new tty.WriteStream(fd); + common.runWithInvalidFD((fd) => { + new tty.WriteStream(fd); + }); }, { code: 'ERR_SYSTEM_ERROR', type: SystemError, @@ -37,12 +33,9 @@ common.expectsError( common.expectsError( () => { - let fd = 2; - // Get first known bad file descriptor. - try { - while (fs.fstatSync(++fd)); - } catch (e) { } - new tty.ReadStream(fd); + common.runWithInvalidFD((fd) => { + new tty.ReadStream(fd); + }); }, { code: 'ERR_SYSTEM_ERROR', type: SystemError,