Skip to content

Commit

Permalink
fs: make promises/write and writeSync params optional
Browse files Browse the repository at this point in the history
This change allows passing objects as "named parameters" and
prohibits passing objects with own toString property as strings.

Fixes: nodejs#41666
  • Loading branch information
LiviaMedeiros committed Feb 27, 2022
1 parent 0b5e8e0 commit 8e017ae
Show file tree
Hide file tree
Showing 4 changed files with 95 additions and 26 deletions.
72 changes: 53 additions & 19 deletions doc/api/fs.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:**
Expand All @@ -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
Expand All @@ -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)`
<!-- YAML
added: REPLACEME
-->
* `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]])`
<!-- YAML
added: v10.0.0
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/41677
description: The `string` parameter no longer accepts objects with an
own `toString` function property.
- version: v14.12.0
pr-url: https://github.com/nodejs/node/pull/34993
description: The `string` parameter will stringify an object with an
Expand All @@ -631,21 +651,21 @@ changes:
strings anymore.
-->
* `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)
documentation for more detail.
* `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
Expand Down Expand Up @@ -4375,7 +4395,7 @@ changes:
-->
* `fd` {integer}
* `buffer` {Buffer|TypedArray|DataView|string|Object}
* `buffer` {Buffer|TypedArray|DataView}
* `offset` {integer}
* `length` {integer}
* `position` {integer}
Expand All @@ -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.
Expand Down Expand Up @@ -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)`
<!-- YAML
added: REPLACEME
-->
* `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...)`][].
Expand All @@ -5793,6 +5826,10 @@ this API: [`fs.write(fd, buffer...)`][].
<!-- YAML
added: v0.11.5
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/41677
description: The `string` parameter no longer accepts objects with an
own `toString` function property.
- version: v14.12.0
pr-url: https://github.com/nodejs/node/pull/34993
description: The `string` parameter will stringify an object with an
Expand All @@ -5807,14 +5844,11 @@ changes:
-->
* `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...)`][].
Expand Down
22 changes: 17 additions & 5 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ const {
validateRmOptionsSync,
validateRmdirOptions,
validateStringAfterArrayBufferView,
validatePrimitiveStringAfterArrayBufferView,
warnOnNonPortableTemplate
} = require('internal/fs/utils');
const {
Expand Down Expand Up @@ -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;
Expand All @@ -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)
Expand Down
16 changes: 14 additions & 2 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const {
validateRmOptions,
validateRmdirOptions,
validateStringAfterArrayBufferView,
validatePrimitiveStringAfterArrayBufferView,
warnOnNonPortableTemplate,
} = require('internal/fs/utils');
const { opendir } = require('internal/fs/dir');
Expand Down Expand Up @@ -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 };

Expand All @@ -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;
Expand Down
11 changes: 11 additions & 0 deletions lib/internal/fs/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -943,5 +953,6 @@ module.exports = {
validateRmOptionsSync,
validateRmdirOptions,
validateStringAfterArrayBufferView,
validatePrimitiveStringAfterArrayBufferView,
warnOnNonPortableTemplate
};

0 comments on commit 8e017ae

Please sign in to comment.