From 1bfca0b61a3cdc243f299c43ece309ca0f315aeb Mon Sep 17 00:00:00 2001 From: LiviaMedeiros Date: Fri, 21 Jan 2022 00:00:17 +0800 Subject: [PATCH] fs: make promises/write and writeSync params optional This change allows passing objects as "named parameters" and prohibits passing objects with own toString property as strings. Fixes: https://github.com/nodejs/node/issues/41666 --- doc/api/fs.md | 72 +++++++++++++++++++++++++++---------- lib/fs.js | 22 +++++++++--- lib/internal/fs/promises.js | 16 +++++++-- lib/internal/fs/utils.js | 11 ++++++ 4 files changed, 95 insertions(+), 26 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 2f9705752556d9..6bf9a0fa82df47 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -586,7 +586,7 @@ changes: buffers anymore. --> -* `buffer` {Buffer|TypedArray|DataView|string|Object} +* `buffer` {Buffer|TypedArray|DataView} * `offset` {integer} The start position from within `buffer` where the data to write begins. **Default:** `0` * `length` {integer} The number of bytes from `buffer` to write. **Default:** @@ -599,13 +599,10 @@ changes: Write `buffer` to the file. -If `buffer` is a plain object, it must have an own (not inherited) `toString` -function property. - The promise is resolved with an object containing two properties: * `bytesWritten` {integer} the number of bytes written -* `buffer` {Buffer|TypedArray|DataView|string|Object} a reference to the +* `buffer` {Buffer|TypedArray|DataView} a reference to the `buffer` written. It is unsafe to use `filehandle.write()` multiple times on the same file @@ -616,11 +613,34 @@ On Linux, positional writes do not work when the file is opened in append mode. The kernel ignores the position argument and always appends the data to the end of the file. +#### `filehandle.write(buffer, options)` + + + +* `buffer` {Buffer|TypedArray|DataView} +* `options` {Object} + * `offset` {integer} **Default:** `0` + * `length` {integer} **Default:** `buffer.byteLength` + * `position` {integer} **Default:** `null` +* Returns: {Promise} + +Write `buffer` to the file. + +Similar to the above `filehandle.write` function, this version takes an +optional `options` object. If no `options` object is specified, it will +default with the above values. + #### `filehandle.write(string[, position[, encoding]])` -* `string` {string|Object} +* `string` {string} * `position` {integer} The offset from the beginning of the file where the data from `string` should be written. If `position` is not a `number` the data will be written at the current position. See the POSIX pwrite(2) @@ -639,13 +659,13 @@ changes: * `encoding` {string} The expected string encoding. **Default:** `'utf8'` * Returns: {Promise} -Write `string` to the file. If `string` is not a string, or an object with an -own `toString` function property, the promise is rejected with an error. +Write `string` to the file. If `string` is not a string, the promise is +rejected with an error. The promise is resolved with an object containing two properties: * `bytesWritten` {integer} the number of bytes written -* `buffer` {string|Object} a reference to the `string` written. +* `buffer` {string} a reference to the `string` written. It is unsafe to use `filehandle.write()` multiple times on the same file without waiting for the promise to be resolved (or rejected). For this @@ -4375,7 +4395,7 @@ changes: --> * `fd` {integer} -* `buffer` {Buffer|TypedArray|DataView|string|Object} +* `buffer` {Buffer|TypedArray|DataView} * `offset` {integer} * `length` {integer} * `position` {integer} @@ -4384,8 +4404,7 @@ changes: * `bytesWritten` {integer} * `buffer` {Buffer|TypedArray|DataView} -Write `buffer` to the file specified by `fd`. If `buffer` is a normal object, it -must have an own `toString` function property. +Write `buffer` to the file specified by `fd`. `offset` determines the part of the buffer to be written, and `length` is an integer specifying the number of bytes to write. @@ -5776,14 +5795,28 @@ changes: --> * `fd` {integer} -* `buffer` {Buffer|TypedArray|DataView|string|Object} +* `buffer` {Buffer|TypedArray|DataView} * `offset` {integer} * `length` {integer} * `position` {integer} * Returns: {number} The number of bytes written. -If `buffer` is a plain object, it must have an own (not inherited) `toString` -function property. +For detailed information, see the documentation of the asynchronous version of +this API: [`fs.write(fd, buffer...)`][]. + +### `fs.writeSync(fd, buffer, options)` + + + +* `fd` {integer} +* `buffer` {Buffer|TypedArray|DataView} +* `options` {Object} + * `offset` {integer} **Default:** `0` + * `length` {integer} **Default:** `buffer.byteLength` + * `position` {integer} **Default:** `null` +* Returns: {number} The number of bytes written. For detailed information, see the documentation of the asynchronous version of this API: [`fs.write(fd, buffer...)`][]. @@ -5793,6 +5826,10 @@ this API: [`fs.write(fd, buffer...)`][]. * `fd` {integer} -* `string` {string|Object} +* `string` {string} * `position` {integer} * `encoding` {string} * Returns: {number} The number of bytes written. -If `string` is a plain object, it must have an own (not inherited) `toString` -function property. - For detailed information, see the documentation of the asynchronous version of this API: [`fs.write(fd, string...)`][]. diff --git a/lib/fs.js b/lib/fs.js index 2895c11d8ecf1d..a0608e32771077 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -116,6 +116,7 @@ const { validateRmOptionsSync, validateRmdirOptions, validateStringAfterArrayBufferView, + validatePrimitiveStringAfterArrayBufferView, warnOnNonPortableTemplate } = require('internal/fs/utils'); const { @@ -852,16 +853,27 @@ ObjectDefineProperty(write, internalUtil.customPromisifyArgs, * Synchronously writes `buffer` to the * specified `fd` (file descriptor). * @param {number} fd - * @param {Buffer | TypedArray | DataView | string | object} buffer - * @param {number} [offset] - * @param {number} [length] - * @param {number} [position] + * @param {Buffer | TypedArray | DataView | string} buffer + * @param {{ + * offset?: number; + * length?: number; + * position?: number; + * }} [offset] * @returns {number} */ function writeSync(fd, buffer, offset, length, position) { fd = getValidatedFd(fd); const ctx = {}; let result; + + if (typeof offset === 'object' && offset !== null) { + ({ + offset = 0, + length = buffer.byteLength, + position = null + } = offset); + } + if (isArrayBufferView(buffer)) { if (position === undefined) position = null; @@ -876,7 +888,7 @@ function writeSync(fd, buffer, offset, length, position) { result = binding.writeBuffer(fd, buffer, offset, length, position, undefined, ctx); } else { - validateStringAfterArrayBufferView(buffer, 'buffer'); + validatePrimitiveStringAfterArrayBufferView(buffer, 'buffer'); validateEncoding(buffer, length); if (offset === undefined) diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js index 868c7df2f1ed79..50c69f3cf8b836 100644 --- a/lib/internal/fs/promises.js +++ b/lib/internal/fs/promises.js @@ -66,6 +66,7 @@ const { validateRmOptions, validateRmdirOptions, validateStringAfterArrayBufferView, + validatePrimitiveStringAfterArrayBufferView, warnOnNonPortableTemplate, } = require('internal/fs/utils'); const { opendir } = require('internal/fs/dir'); @@ -560,7 +561,18 @@ async function readv(handle, buffers, position) { return { bytesRead, buffers }; } -async function write(handle, buffer, offset, length, position) { +async function write(handle, bufferOrOptions, offset, length, position) { + let buffer = bufferOrOptions; + if (!isArrayBufferView(buffer) && typeof buffer !== 'string') { + validateBuffer(bufferOrOptions?.buffer); + ({ + buffer, + offset = 0, + length = buffer.byteLength, + position = null + } = bufferOrOptions); + } + if (buffer?.byteLength === 0) return { bytesWritten: 0, buffer }; @@ -581,7 +593,7 @@ async function write(handle, buffer, offset, length, position) { return { bytesWritten, buffer }; } - validateStringAfterArrayBufferView(buffer, 'buffer'); + validatePrimitiveStringAfterArrayBufferView(buffer, 'buffer'); validateEncoding(buffer, length); const bytesWritten = (await binding.writeString(handle.fd, buffer, offset, length, kUsePromises)) || 0; diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 481b5292b1d726..61670011372bb7 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -889,6 +889,16 @@ const validateStringAfterArrayBufferView = hideStackFrames((buffer, name) => { ); }); +const validatePrimitiveStringAfterArrayBufferView = hideStackFrames((buffer, name) => { + if (typeof buffer !== 'string') { + throw new ERR_INVALID_ARG_TYPE( + name, + ['string', 'Buffer', 'TypedArray', 'DataView'], + buffer + ); + } +}); + const validatePosition = hideStackFrames((position, name) => { if (typeof position === 'number') { validateInteger(position, 'position'); @@ -943,5 +953,6 @@ module.exports = { validateRmOptionsSync, validateRmdirOptions, validateStringAfterArrayBufferView, + validatePrimitiveStringAfterArrayBufferView, warnOnNonPortableTemplate };