Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 75302d3

Browse filesBrowse files
LiviaMedeirosjuanarbol
LiviaMedeiros
authored and
juanarbol
committed
fs: fix write methods param validation and docs
The FS docs wrongfully indicated support for passing object with an own `toString` function property to `FileHandle.prototype.appendFile`, `FileHandle.prototype.writeFile`, `FileHandle.prototype.write`, `fsPromises.writeFile`, and `fs.writeSync`. This commit fixes that, and adds some test to ensure the actual behavior is aligned with the docs. It also fixes a bug that makes the process crash if a non-buffer object was passed to `FileHandle.prototype.write`. Refs: #34993 PR-URL: #41677 Refs: #41666 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Backport-PR-URL: #42603
1 parent 77ba012 commit 75302d3
Copy full SHA for 75302d3

File tree

Expand file treeCollapse file tree

6 files changed

+46
-71
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+46
-71
lines changed
Open diff view settings
Collapse file

‎doc/api/fs.md‎

Copy file name to clipboardExpand all lines: doc/api/fs.md
+17-64Lines changed: 17 additions & 64 deletions
  • Display the source diff
  • Display the rich diff
Original file line numberDiff line numberDiff line change
@@ -171,18 +171,13 @@ changes:
171171
- version: v14.18.0
172172
pr-url: https://github.com/nodejs/node/pull/37490
173173
description: The `data` argument supports `AsyncIterable`, `Iterable` and `Stream`.
174-
- version: v14.12.0
175-
pr-url: https://github.com/nodejs/node/pull/34993
176-
description: The `data` parameter will stringify an object with an
177-
explicit `toString` function.
178174
- version: v14.0.0
179175
pr-url: https://github.com/nodejs/node/pull/31030
180176
description: The `data` parameter won't coerce unsupported input to
181177
strings anymore.
182178
-->
183179

184-
* `data` {string|Buffer|TypedArray|DataView|Object|AsyncIterable|Iterable
185-
|Stream}
180+
* `data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream}
186181
* `options` {Object|string}
187182
* `encoding` {string|null} **Default:** `'utf8'`
188183
* Returns: {Promise} Fulfills with `undefined` upon success.
@@ -425,21 +420,17 @@ promise with an error using code `UV_ENOSYS`.
425420
<!-- YAML
426421
added: v10.0.0
427422
changes:
428-
- version: v14.12.0
429-
pr-url: https://github.com/nodejs/node/pull/34993
430-
description: The `buffer` parameter will stringify an object with an
431-
explicit `toString` function.
432423
- version: v14.0.0
433424
pr-url: https://github.com/nodejs/node/pull/31030
434425
description: The `buffer` parameter won't coerce unsupported input to
435426
buffers anymore.
436427
-->
437428
438-
* `buffer` {Buffer|TypedArray|DataView|string|Object}
429+
* `buffer` {Buffer|TypedArray|DataView}
439430
* `offset` {integer} The start position from within `buffer` where the data
440431
to write begins. **Default:** `0`
441432
* `length` {integer} The number of bytes from `buffer` to write. **Default:**
442-
`buffer.byteLength`
433+
`buffer.byteLength - offset`
443434
* `position` {integer} The offset from the beginning of the file where the
444435
data from `buffer` should be written. If `position` is not a `number`,
445436
the data will be written at the current position. See the POSIX pwrite(2)
@@ -448,13 +439,10 @@ changes:
448439
449440
Write `buffer` to the file.
450441
451-
If `buffer` is a plain object, it must have an own (not inherited) `toString`
452-
function property.
453-
454442
The promise is resolved with an object containing two properties:
455443
456444
* `bytesWritten` {integer} the number of bytes written
457-
* `buffer` {Buffer|TypedArray|DataView|string|Object} a reference to the
445+
* `buffer` {Buffer|TypedArray|DataView} a reference to the
458446
`buffer` written.
459447
460448
It is unsafe to use `filehandle.write()` multiple times on the same file
@@ -469,31 +457,27 @@ the end of the file.
469457
<!-- YAML
470458
added: v10.0.0
471459
changes:
472-
- version: v14.12.0
473-
pr-url: https://github.com/nodejs/node/pull/34993
474-
description: The `string` parameter will stringify an object with an
475-
explicit `toString` function.
476460
- version: v14.0.0
477461
pr-url: https://github.com/nodejs/node/pull/31030
478462
description: The `string` parameter won't coerce unsupported input to
479463
strings anymore.
480464
-->
481465
482-
* `string` {string|Object}
466+
* `string` {string}
483467
* `position` {integer} The offset from the beginning of the file where the
484468
data from `string` should be written. If `position` is not a `number` the
485469
data will be written at the current position. See the POSIX pwrite(2)
486470
documentation for more detail.
487471
* `encoding` {string} The expected string encoding. **Default:** `'utf8'`
488472
* Returns: {Promise}
489473
490-
Write `string` to the file. If `string` is not a string, or an object with an
491-
own `toString` function property, the promise is rejected with an error.
474+
Write `string` to the file. If `string` is not a string, the promise is
475+
rejected with an error.
492476
493477
The promise is resolved with an object containing two properties:
494478
495479
* `bytesWritten` {integer} the number of bytes written
496-
* `buffer` {string|Object} a reference to the `string` written.
480+
* `buffer` {string} a reference to the `string` written.
497481
498482
It is unsafe to use `filehandle.write()` multiple times on the same file
499483
without waiting for the promise to be resolved (or rejected). For this
@@ -510,27 +494,21 @@ changes:
510494
- version: v14.18.0
511495
pr-url: https://github.com/nodejs/node/pull/37490
512496
description: The `data` argument supports `AsyncIterable`, `Iterable` and `Stream`.
513-
- version: v14.12.0
514-
pr-url: https://github.com/nodejs/node/pull/34993
515-
description: The `data` parameter will stringify an object with an
516-
explicit `toString` function.
517497
- version: v14.0.0
518498
pr-url: https://github.com/nodejs/node/pull/31030
519499
description: The `data` parameter won't coerce unsupported input to
520500
strings anymore.
521501
-->
522502
523-
* `data` {string|Buffer|TypedArray|DataView|Object|AsyncIterable|Iterable
524-
|Stream}
503+
* `data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream}
525504
* `options` {Object|string}
526505
* `encoding` {string|null} The expected character encoding when `data` is a
527506
string. **Default:** `'utf8'`
528507
* Returns: {Promise}
529508
530509
Asynchronously writes data to a file, replacing the file if it already exists.
531-
`data` can be a string, a buffer, an {AsyncIterable} or {Iterable} object, or an
532-
object with an own `toString` function
533-
property. The promise is resolved with no arguments upon success.
510+
`data` can be a string, a buffer, an {AsyncIterable} or {Iterable} object.
511+
The promise is resolved with no arguments upon success.
534512
535513
If `options` is a string, then it specifies the `encoding`.
536514
@@ -1274,19 +1252,14 @@ changes:
12741252
pr-url: https://github.com/nodejs/node/pull/35993
12751253
description: The options argument may include an AbortSignal to abort an
12761254
ongoing writeFile request.
1277-
- version: v14.12.0
1278-
pr-url: https://github.com/nodejs/node/pull/34993
1279-
description: The `data` parameter will stringify an object with an
1280-
explicit `toString` function.
12811255
- version: v14.0.0
12821256
pr-url: https://github.com/nodejs/node/pull/31030
12831257
description: The `data` parameter won't coerce unsupported input to
12841258
strings anymore.
12851259
-->
12861260
12871261
* `file` {string|Buffer|URL|FileHandle} filename or `FileHandle`
1288-
* `data` {string|Buffer|TypedArray|DataView|Object|AsyncIterable|Iterable
1289-
|Stream}
1262+
* `data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream}
12901263
* `options` {Object|string}
12911264
* `encoding` {string|null} **Default:** `'utf8'`
12921265
* `mode` {integer} **Default:** `0o666`
@@ -1295,8 +1268,7 @@ changes:
12951268
* Returns: {Promise} Fulfills with `undefined` upon success.
12961269
12971270
Asynchronously writes data to a file, replacing the file if it already exists.
1298-
`data` can be a string, a {Buffer}, or, an object with an own (not inherited)
1299-
`toString` function property.
1271+
`data` can be a string, a buffer, an {AsyncIterable} or {Iterable} object.
13001272
13011273
The `encoding` option is ignored if `data` is a buffer.
13021274
@@ -3763,10 +3735,6 @@ This happens when:
37633735
<!-- YAML
37643736
added: v0.0.2
37653737
changes:
3766-
- version: v14.12.0
3767-
pr-url: https://github.com/nodejs/node/pull/34993
3768-
description: The `buffer` parameter will stringify an object with an
3769-
explicit `toString` function.
37703738
- version: v14.0.0
37713739
pr-url: https://github.com/nodejs/node/pull/31030
37723740
description: The `buffer` parameter won't coerce unsupported input to
@@ -3792,7 +3760,7 @@ changes:
37923760
-->
37933761
37943762
* `fd` {integer}
3795-
* `buffer` {Buffer|TypedArray|DataView|string|Object}
3763+
* `buffer` {Buffer|TypedArray|DataView}
37963764
* `offset` {integer}
37973765
* `length` {integer}
37983766
* `position` {integer}
@@ -3801,8 +3769,7 @@ changes:
38013769
* `bytesWritten` {integer}
38023770
* `buffer` {Buffer|TypedArray|DataView}
38033771
3804-
Write `buffer` to the file specified by `fd`. If `buffer` is a normal object, it
3805-
must have an own `toString` function property.
3772+
Write `buffer` to the file specified by `fd`.
38063773
38073774
`offset` determines the part of the buffer to be written, and `length` is
38083775
an integer specifying the number of bytes to write.
@@ -5046,10 +5013,6 @@ this API: [`fs.writeFile()`][].
50465013
<!-- YAML
50475014
added: v0.1.21
50485015
changes:
5049-
- version: v14.12.0
5050-
pr-url: https://github.com/nodejs/node/pull/34993
5051-
description: The `buffer` parameter will stringify an object with an
5052-
explicit `toString` function.
50535016
- version: v14.0.0
50545017
pr-url: https://github.com/nodejs/node/pull/31030
50555018
description: The `buffer` parameter won't coerce unsupported input to
@@ -5067,26 +5030,19 @@ changes:
50675030
-->
50685031
50695032
* `fd` {integer}
5070-
* `buffer` {Buffer|TypedArray|DataView|string|Object}
5033+
* `buffer` {Buffer|TypedArray|DataView}
50715034
* `offset` {integer}
50725035
* `length` {integer}
50735036
* `position` {integer}
50745037
* Returns: {number} The number of bytes written.
50755038
5076-
If `buffer` is a plain object, it must have an own (not inherited) `toString`
5077-
function property.
5078-
50795039
For detailed information, see the documentation of the asynchronous version of
50805040
this API: [`fs.write(fd, buffer...)`][].
50815041
50825042
### `fs.writeSync(fd, string[, position[, encoding]])`
50835043
<!-- YAML
50845044
added: v0.11.5
50855045
changes:
5086-
- version: v14.12.0
5087-
pr-url: https://github.com/nodejs/node/pull/34993
5088-
description: The `string` parameter will stringify an object with an
5089-
explicit `toString` function.
50905046
- version: v14.0.0
50915047
pr-url: https://github.com/nodejs/node/pull/31030
50925048
description: The `string` parameter won't coerce unsupported input to
@@ -5097,14 +5053,11 @@ changes:
50975053
-->
50985054
50995055
* `fd` {integer}
5100-
* `string` {string|Object}
5056+
* `string` {string}
51015057
* `position` {integer}
51025058
* `encoding` {string}
51035059
* Returns: {number} The number of bytes written.
51045060
5105-
If `string` is a plain object, it must have an own (not inherited) `toString`
5106-
function property.
5107-
51085061
For detailed information, see the documentation of the asynchronous version of
51095062
this API: [`fs.write(fd, string...)`][].
51105063
Collapse file

‎lib/fs.js‎

Copy file name to clipboardExpand all lines: lib/fs.js
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ const {
103103
validateRmOptionsSync,
104104
validateRmdirOptions,
105105
validateStringAfterArrayBufferView,
106+
validatePrimitiveStringAfterArrayBufferView,
106107
warnOnNonPortableTemplate
107108
} = require('internal/fs/utils');
108109
const {
@@ -726,7 +727,7 @@ function writeSync(fd, buffer, offset, length, position) {
726727
result = binding.writeBuffer(fd, buffer, offset, length, position,
727728
undefined, ctx);
728729
} else {
729-
validateStringAfterArrayBufferView(buffer, 'buffer');
730+
validatePrimitiveStringAfterArrayBufferView(buffer, 'buffer');
730731

731732
if (offset === undefined)
732733
offset = null;
Collapse file

‎lib/internal/fs/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/promises.js
+4-4Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ const {
5656
validateOffsetLengthWrite,
5757
validateRmOptions,
5858
validateRmdirOptions,
59-
validateStringAfterArrayBufferView,
60-
warnOnNonPortableTemplate
59+
validatePrimitiveStringAfterArrayBufferView,
60+
warnOnNonPortableTemplate,
6161
} = require('internal/fs/utils');
6262
const { opendir } = require('internal/fs/dir');
6363
const {
@@ -461,7 +461,7 @@ async function write(handle, buffer, offset, length, position) {
461461
return { bytesWritten, buffer };
462462
}
463463

464-
validateStringAfterArrayBufferView(buffer, 'buffer');
464+
validatePrimitiveStringAfterArrayBufferView(buffer, 'buffer');
465465
const bytesWritten = (await binding.writeString(handle.fd, buffer, offset,
466466
length, kUsePromises)) || 0;
467467
return { bytesWritten, buffer };
@@ -683,7 +683,7 @@ async function writeFile(path, data, options) {
683683
const flag = options.flag || 'w';
684684

685685
if (!isArrayBufferView(data) && !isCustomIterable(data)) {
686-
validateStringAfterArrayBufferView(data, 'data');
686+
validatePrimitiveStringAfterArrayBufferView(data, 'data');
687687
data = Buffer.from(data, options.encoding || 'utf8');
688688
}
689689

Collapse file

‎lib/internal/fs/utils.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/utils.js
+12Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -815,6 +815,17 @@ const validateStringAfterArrayBufferView = hideStackFrames((buffer, name) => {
815815
);
816816
});
817817

818+
const validatePrimitiveStringAfterArrayBufferView =
819+
hideStackFrames((buffer, name) => {
820+
if (typeof buffer !== 'string') {
821+
throw new ERR_INVALID_ARG_TYPE(
822+
name,
823+
['string', 'Buffer', 'TypedArray', 'DataView'],
824+
buffer
825+
);
826+
}
827+
});
828+
818829
const validatePosition = hideStackFrames((position, name) => {
819830
if (typeof position === 'number') {
820831
validateInteger(position, 'position');
@@ -866,5 +877,6 @@ module.exports = {
866877
validateRmOptionsSync,
867878
validateRmdirOptions,
868879
validateStringAfterArrayBufferView,
880+
validatePrimitiveStringAfterArrayBufferView,
869881
warnOnNonPortableTemplate
870882
};
Collapse file

‎test/parallel/test-fs-promises-file-handle-write.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-fs-promises-file-handle-write.js
+6-1Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,12 @@ async function validateNonUint8ArrayWrite() {
5353
async function validateNonStringValuesWrite() {
5454
const filePathForHandle = path.resolve(tmpDir, 'tmp-non-string-write.txt');
5555
const fileHandle = await open(filePathForHandle, 'w+');
56-
const nonStringValues = [123, {}, new Map()];
56+
const nonStringValues = [
57+
123, {}, new Map(), null, undefined, 0n, () => {}, Symbol(), true,
58+
new String('notPrimitive'),
59+
{ toString() { return 'amObject'; } },
60+
{ [Symbol.toPrimitive]: (hint) => 'amObject' },
61+
];
5762
for (const nonStringValue of nonStringValues) {
5863
await assert.rejects(
5964
fileHandle.write(nonStringValue),
Collapse file

‎test/parallel/test-fs-write.js‎

Copy file name to clipboardExpand all lines: test/parallel/test-fs-write.js
+5-1Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,11 @@ fs.open(fn4, 'w', 0o644, common.mustSucceed((fd) => {
154154
);
155155
});
156156

157-
[false, 5, {}, [], null, undefined].forEach((data) => {
157+
[
158+
false, 5, {}, [], null, undefined,
159+
new String('notPrimitive'),
160+
{ [Symbol.toPrimitive]: (hint) => 'amObject' },
161+
].forEach((data) => {
158162
assert.throws(
159163
() => fs.write(1, data, common.mustNotCall()),
160164
{

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.