diff options
author | LiviaMedeiros <livia@cirno.name> | 2022-04-04 14:07:50 +0300 |
---|---|---|
committer | Bryan English <bryan@bryanenglish.com> | 2022-05-30 19:26:50 +0300 |
commit | 3e89b7336dbdfb7bec601e52b49d63fa3a0105f4 (patch) | |
tree | f0bb0730071dd579374fa5427f88eadcbfa1eeca /test | |
parent | 72a767b7cab21c9e5c57994767e318c6f2041a9c (diff) |
fs: make params in writing methods optional
This change allows passing objects as "named parameters":
- `fs.write(fd, buffer[, options], callback)`
- `fs.writeSync(fd, buffer[, options])`
- `filehandle.write(buffer[, options])`
Fixes: https://github.com/nodejs/node/issues/41666
PR-URL: https://github.com/nodejs/node/pull/42601
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Minwoo Jung <nodecorelab@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Diffstat (limited to 'test')
-rw-r--r-- | test/parallel/test-fs-promises-write-optional-params.js | 95 | ||||
-rw-r--r-- | test/parallel/test-fs-write-optional-params.js | 102 | ||||
-rw-r--r-- | test/parallel/test-fs-write-sync-optional-params.js | 89 |
3 files changed, 286 insertions, 0 deletions
diff --git a/test/parallel/test-fs-promises-write-optional-params.js b/test/parallel/test-fs-promises-write-optional-params.js new file mode 100644 index 00000000000..b130c491541 --- /dev/null +++ b/test/parallel/test-fs-promises-write-optional-params.js @@ -0,0 +1,95 @@ +'use strict'; + +const common = require('../common'); + +// This test ensures that filehandle.write accepts "named parameters" object +// and doesn't interpret objects as strings + +const assert = require('assert'); +const fsPromises = require('fs').promises; +const path = require('path'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +const dest = path.resolve(tmpdir.path, 'tmp.txt'); +const buffer = Buffer.from('zyx'); + +async function testInvalid(dest, expectedCode, ...params) { + let fh; + try { + fh = await fsPromises.open(dest, 'w+'); + await assert.rejects( + fh.write(...params), + { code: expectedCode }); + } finally { + await fh?.close(); + } +} + +async function testValid(dest, buffer, options) { + let fh; + try { + fh = await fsPromises.open(dest, 'w+'); + const writeResult = await fh.write(buffer, options); + const writeBufCopy = Uint8Array.prototype.slice.call(writeResult.buffer); + + const readResult = await fh.read(buffer, options); + const readBufCopy = Uint8Array.prototype.slice.call(readResult.buffer); + + assert.ok(writeResult.bytesWritten >= readResult.bytesRead); + if (options.length !== undefined && options.length !== null) { + assert.strictEqual(writeResult.bytesWritten, options.length); + } + if (options.offset === undefined || options.offset === 0) { + assert.deepStrictEqual(writeBufCopy, readBufCopy); + } + assert.deepStrictEqual(writeResult.buffer, readResult.buffer); + } finally { + await fh?.close(); + } +} + +(async () => { + // Test if first argument is not wrongly interpreted as ArrayBufferView|string + for (const badBuffer of [ + undefined, null, true, 42, 42n, Symbol('42'), NaN, [], () => {}, + Promise.resolve(new Uint8Array(1)), + {}, + { buffer: 'amNotParam' }, + { string: 'amNotParam' }, + { buffer: new Uint8Array(1).buffer }, + new Date(), + new String('notPrimitive'), + { toString() { return 'amObject'; } }, + { [Symbol.toPrimitive]: (hint) => 'amObject' }, + ]) { + await testInvalid(dest, 'ERR_INVALID_ARG_TYPE', badBuffer, {}); + } + + // First argument (buffer or string) is mandatory + await testInvalid(dest, 'ERR_INVALID_ARG_TYPE'); + + // Various invalid options + await testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: 5 }); + await testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { offset: 5 }); + await testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: 1, offset: 3 }); + await testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: -1 }); + await testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { offset: -1 }); + await testInvalid(dest, 'ERR_INVALID_ARG_TYPE', buffer, { offset: false }); + await testInvalid(dest, 'ERR_INVALID_ARG_TYPE', buffer, { offset: true }); + + // Test compatibility with filehandle.read counterpart + for (const options of [ + {}, + { length: 1 }, + { position: 5 }, + { length: 1, position: 5 }, + { length: 1, position: -1, offset: 2 }, + { length: null }, + { position: null }, + { offset: 1 }, + ]) { + await testValid(dest, buffer, options); + } +})().then(common.mustCall()); diff --git a/test/parallel/test-fs-write-optional-params.js b/test/parallel/test-fs-write-optional-params.js new file mode 100644 index 00000000000..21732114153 --- /dev/null +++ b/test/parallel/test-fs-write-optional-params.js @@ -0,0 +1,102 @@ +'use strict'; + +const common = require('../common'); + +// This test ensures that fs.write accepts "named parameters" object +// and doesn't interpret objects as strings + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const tmpdir = require('../common/tmpdir'); +const util = require('util'); + +tmpdir.refresh(); + +const destInvalid = path.resolve(tmpdir.path, 'rwopt_invalid'); +const buffer = Buffer.from('zyx'); + +function testInvalidCb(fd, expectedCode, buffer, options, callback) { + assert.throws( + () => fs.write(fd, buffer, options, common.mustNotCall()), + { code: expectedCode } + ); + callback(0); +} + +function testValidCb(buffer, options, index, callback) { + const dest = path.resolve(tmpdir.path, `rwopt_valid_${index}`); + fs.open(dest, 'w+', common.mustSucceed((fd) => { + fs.write(fd, buffer, options, common.mustSucceed((bytesWritten, bufferWritten) => { + const writeBufCopy = Uint8Array.prototype.slice.call(bufferWritten); + + fs.read(fd, buffer, options, common.mustSucceed((bytesRead, bufferRead) => { + const readBufCopy = Uint8Array.prototype.slice.call(bufferRead); + + assert.ok(bytesWritten >= bytesRead); + if (options.length !== undefined && options.length !== null) { + assert.strictEqual(bytesWritten, options.length); + } + if (options.offset === undefined || options.offset === 0) { + assert.deepStrictEqual(writeBufCopy, readBufCopy); + } + assert.deepStrictEqual(bufferWritten, bufferRead); + fs.close(fd, common.mustSucceed(callback)); + })); + })); + })); +} + +// Promisify to reduce flakiness +const testInvalid = util.promisify(testInvalidCb); +const testValid = util.promisify(testValidCb); + +async function runTests(fd) { + // Test if first argument is not wrongly interpreted as ArrayBufferView|string + for (const badBuffer of [ + undefined, null, true, 42, 42n, Symbol('42'), NaN, [], () => {}, + Promise.resolve(new Uint8Array(1)), + {}, + { buffer: 'amNotParam' }, + { string: 'amNotParam' }, + { buffer: new Uint8Array(1).buffer }, + new Date(), + new String('notPrimitive'), + { [Symbol.toPrimitive]: (hint) => 'amObject' }, + + // TODO(LiviaMedeiros): add the following after DEP0162 EOL + // { toString() { return 'amObject'; } }, + ]) { + await testInvalid(fd, 'ERR_INVALID_ARG_TYPE', badBuffer, {}); + } + + // First argument (buffer or string) is mandatory + await testInvalid(fd, 'ERR_INVALID_ARG_TYPE', undefined, undefined); + + // Various invalid options + await testInvalid(fd, 'ERR_OUT_OF_RANGE', buffer, { length: 5 }); + await testInvalid(fd, 'ERR_OUT_OF_RANGE', buffer, { offset: 5 }); + await testInvalid(fd, 'ERR_OUT_OF_RANGE', buffer, { length: 1, offset: 3 }); + await testInvalid(fd, 'ERR_OUT_OF_RANGE', buffer, { length: -1 }); + await testInvalid(fd, 'ERR_OUT_OF_RANGE', buffer, { offset: -1 }); + await testInvalid(fd, 'ERR_INVALID_ARG_TYPE', buffer, { offset: false }); + await testInvalid(fd, 'ERR_INVALID_ARG_TYPE', buffer, { offset: true }); + + // Test compatibility with fs.read counterpart + for (const [ index, options ] of [ + {}, + { length: 1 }, + { position: 5 }, + { length: 1, position: 5 }, + { length: 1, position: -1, offset: 2 }, + { length: null }, + { position: null }, + { offset: 1 }, + ].entries()) { + await testValid(buffer, options, index); + } +} + +fs.open(destInvalid, 'w+', common.mustSucceed(async (fd) => { + runTests(fd).then(common.mustCall(() => fs.close(fd, common.mustSucceed()))); +})); diff --git a/test/parallel/test-fs-write-sync-optional-params.js b/test/parallel/test-fs-write-sync-optional-params.js new file mode 100644 index 00000000000..80dfa51e0ba --- /dev/null +++ b/test/parallel/test-fs-write-sync-optional-params.js @@ -0,0 +1,89 @@ +'use strict'; + +require('../common'); + +// This test ensures that fs.writeSync accepts "named parameters" object +// and doesn't interpret objects as strings + +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const tmpdir = require('../common/tmpdir'); + +tmpdir.refresh(); + +const dest = path.resolve(tmpdir.path, 'tmp.txt'); +const buffer = Buffer.from('zyx'); + +function testInvalid(dest, expectedCode, ...bufferAndOptions) { + let fd; + try { + fd = fs.openSync(dest, 'w+'); + assert.throws( + () => fs.writeSync(fd, ...bufferAndOptions), + { code: expectedCode }); + } finally { + if (fd != null) fs.closeSync(fd); + } +} + +function testValid(dest, buffer, options) { + let fd; + try { + fd = fs.openSync(dest, 'w+'); + const bytesWritten = fs.writeSync(fd, buffer, options); + const bytesRead = fs.readSync(fd, buffer, options); + + assert.ok(bytesWritten >= bytesRead); + if (options.length !== undefined && options.length !== null) { + assert.strictEqual(bytesWritten, options.length); + } + } finally { + if (fd != null) fs.closeSync(fd); + } +} + +{ + // Test if second argument is not wrongly interpreted as string or options + for (const badBuffer of [ + undefined, null, true, 42, 42n, Symbol('42'), NaN, [], () => {}, + {}, + { buffer: 'amNotParam' }, + { string: 'amNotParam' }, + { buffer: new Uint8Array(1) }, + { buffer: new Uint8Array(1).buffer }, + Promise.resolve(new Uint8Array(1)), + new Date(), + new String('notPrimitive'), + { toString() { return 'amObject'; } }, + { [Symbol.toPrimitive]: (hint) => 'amObject' }, + ]) { + testInvalid(dest, 'ERR_INVALID_ARG_TYPE', badBuffer); + } + + // First argument (buffer or string) is mandatory + testInvalid(dest, 'ERR_INVALID_ARG_TYPE'); + + // Various invalid options + testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: 5 }); + testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { offset: 5 }); + testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: 1, offset: 3 }); + testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { length: -1 }); + testInvalid(dest, 'ERR_OUT_OF_RANGE', buffer, { offset: -1 }); + testInvalid(dest, 'ERR_INVALID_ARG_TYPE', buffer, { offset: false }); + testInvalid(dest, 'ERR_INVALID_ARG_TYPE', buffer, { offset: true }); + + // Test compatibility with fs.readSync counterpart with reused options + for (const options of [ + {}, + { length: 1 }, + { position: 5 }, + { length: 1, position: 5 }, + { length: 1, position: -1, offset: 2 }, + { length: null }, + { position: null }, + { offset: 1 }, + ]) { + testValid(dest, buffer, options); + } +} |