FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fs: fix write methods param validation and docs · nodejs/node@a45aabe · GitHub

/ node Public

Commit a45aabe

Browse files
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>
1 parent a2e858f commit a45aabe

6 files changed

Lines changed: 45 additions & 71 deletions

File tree

‎doc/api/fs.md‎

Lines changed: 17 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -185,18 +185,13 @@ changes:
185185
- v14.18.0
186186
pr-url: https://github.com/nodejs/node/pull/37490
187187
description: The `data` argument supports `AsyncIterable`, `Iterable` and `Stream`.
188-
- version: v14.12.0
189-
pr-url: https://github.com/nodejs/node/pull/34993
190-
description: The `data` parameter will stringify an object with an
191-
explicit `toString` function.
192188
- version: v14.0.0
193189
pr-url: https://github.com/nodejs/node/pull/31030
194190
description: The `data` parameter won't coerce unsupported input to
195191
strings anymore.
196192
-->
197193

198-
* `data` {string|Buffer|TypedArray|DataView|Object|AsyncIterable|Iterable
199-
|Stream}
194+
* `data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream}
200195
* `options` {Object|string}
201196
* `encoding` {string|null} **Default:** `'utf8'`
202197
* Returns: {Promise} Fulfills with `undefined` upon success.
@@ -544,21 +539,17 @@ then resolves the promise with no arguments upon success.
544539
<!-- YAML
545540
added: v10.0.0
546541
changes:
547-
- version: v14.12.0
548-
pr-url: https://github.com/nodejs/node/pull/34993
549-
description: The `buffer` parameter will stringify an object with an
550-
explicit `toString` function.
551542
- version: v14.0.0
552543
pr-url: https://github.com/nodejs/node/pull/31030
553544
description: The `buffer` parameter won't coerce unsupported input to
554545
buffers anymore.
555546
-->
556547
557-
* `buffer` {Buffer|TypedArray|DataView|string|Object}
548+
* `buffer` {Buffer|TypedArray|DataView}
558549
* `offset` {integer} The start position from within `buffer` where the data
559550
to write begins. **Default:** `0`
560551
* `length` {integer} The number of bytes from `buffer` to write. **Default:**
561-
`buffer.byteLength`
552+
`buffer.byteLength - offset`
562553
* `position` {integer} The offset from the beginning of the file where the
563554
data from `buffer` should be written. If `position` is not a `number`,
564555
the data will be written at the current position. See the POSIX pwrite(2)
@@ -567,13 +558,10 @@ changes:
567558
568559
Write `buffer` to the file.
569560
570-
If `buffer` is a plain object, it must have an own (not inherited) `toString`
571-
function property.
572-
573561
The promise is resolved with an object containing two properties:
574562
575563
* `bytesWritten` {integer} the number of bytes written
576-
* `buffer` {Buffer|TypedArray|DataView|string|Object} a reference to the
564+
* `buffer` {Buffer|TypedArray|DataView} a reference to the
577565
`buffer` written.
578566
579567
It is unsafe to use `filehandle.write()` multiple times on the same file
@@ -589,31 +577,27 @@ the end of the file.
589577
<!-- YAML
590578
added: v10.0.0
591579
changes:
592-
- version: v14.12.0
593-
pr-url: https://github.com/nodejs/node/pull/34993
594-
description: The `string` parameter will stringify an object with an
595-
explicit `toString` function.
596580
- version: v14.0.0
597581
pr-url: https://github.com/nodejs/node/pull/31030
598582
description: The `string` parameter won't coerce unsupported input to
599583
strings anymore.
600584
-->
601585
602-
* `string` {string|Object}
586+
* `string` {string}
603587
* `position` {integer} The offset from the beginning of the file where the
604588
data from `string` should be written. If `position` is not a `number` the
605589
data will be written at the current position. See the POSIX pwrite(2)
606590
documentation for more detail.
607591
* `encoding` {string} The expected string encoding. **Default:** `'utf8'`
608592
* Returns: {Promise}
609593
610-
Write `string` to the file. If `string` is not a string, or an object with an
611-
own `toString` function property, the promise is rejected with an error.
594+
Write `string` to the file. If `string` is not a string, the promise is
595+
rejected with an error.
612596
613597
The promise is resolved with an object containing two properties:
614598
615599
* `bytesWritten` {integer} the number of bytes written
616-
* `buffer` {string|Object} a reference to the `string` written.
600+
* `buffer` {string} a reference to the `string` written.
617601
618602
It is unsafe to use `filehandle.write()` multiple times on the same file
619603
without waiting for the promise to be resolved (or rejected). For this
@@ -631,27 +615,21 @@ changes:
631615
- version: v15.14.0
632616
pr-url: https://github.com/nodejs/node/pull/37490
633617
description: The `data` argument supports `AsyncIterable`, `Iterable` and `Stream`.
634-
- version: v14.12.0
635-
pr-url: https://github.com/nodejs/node/pull/34993
636-
description: The `data` parameter will stringify an object with an
637-
explicit `toString` function.
638618
- version: v14.0.0
639619
pr-url: https://github.com/nodejs/node/pull/31030
640620
description: The `data` parameter won't coerce unsupported input to
641621
strings anymore.
642622
-->
643623
644-
* `data` {string|Buffer|TypedArray|DataView|Object|AsyncIterable|Iterable
645-
|Stream}
624+
* `data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream}
646625
* `options` {Object|string}
647626
* `encoding` {string|null} The expected character encoding when `data` is a
648627
string. **Default:** `'utf8'`
649628
* Returns: {Promise}
650629
651630
Asynchronously writes data to a file, replacing the file if it already exists.
652-
`data` can be a string, a buffer, an {AsyncIterable} or {Iterable} object, or an
653-
object with an own `toString` function
654-
property. The promise is resolved with no arguments upon success.
631+
`data` can be a string, a buffer, an {AsyncIterable} or {Iterable} object.
632+
The promise is resolved with no arguments upon success.
655633
656634
If `options` is a string, then it specifies the `encoding`.
657635
@@ -1481,19 +1459,14 @@ changes:
14811459
pr-url: https://github.com/nodejs/node/pull/35993
14821460
description: The options argument may include an AbortSignal to abort an
14831461
ongoing writeFile request.
1484-
- version: v14.12.0
1485-
pr-url: https://github.com/nodejs/node/pull/34993
1486-
description: The `data` parameter will stringify an object with an
1487-
explicit `toString` function.
14881462
- version: v14.0.0
14891463
pr-url: https://github.com/nodejs/node/pull/31030
14901464
description: The `data` parameter won't coerce unsupported input to
14911465
strings anymore.
14921466
-->
14931467
14941468
* `file` {string|Buffer|URL|FileHandle} filename or `FileHandle`
1495-
* `data` {string|Buffer|TypedArray|DataView|Object|AsyncIterable|Iterable
1496-
|Stream}
1469+
* `data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream}
14971470
* `options` {Object|string}
14981471
* `encoding` {string|null} **Default:** `'utf8'`
14991472
* `mode` {integer} **Default:** `0o666`
@@ -1502,8 +1475,7 @@ changes:
15021475
* Returns: {Promise} Fulfills with `undefined` upon success.
15031476
15041477
Asynchronously writes data to a file, replacing the file if it already exists.
1505-
`data` can be a string, a {Buffer}, or, an object with an own (not inherited)
1506-
`toString` function property.
1478+
`data` can be a string, a buffer, an {AsyncIterable} or {Iterable} object.
15071479
15081480
The `encoding` option is ignored if `data` is a buffer.
15091481
@@ -4098,10 +4070,6 @@ This happens when:
40984070
<!-- YAML
40994071
added: v0.0.2
41004072
changes:
4101-
- version: v14.12.0
4102-
pr-url: https://github.com/nodejs/node/pull/34993
4103-
description: The `buffer` parameter will stringify an object with an
4104-
explicit `toString` function.
41054073
- version: v14.0.0
41064074
pr-url: https://github.com/nodejs/node/pull/31030
41074075
description: The `buffer` parameter won't coerce unsupported input to
@@ -4127,7 +4095,7 @@ changes:
41274095
-->
41284096
41294097
* `fd` {integer}
4130-
* `buffer` {Buffer|TypedArray|DataView|string|Object}
4098+
* `buffer` {Buffer|TypedArray|DataView}
41314099
* `offset` {integer}
41324100
* `length` {integer}
41334101
* `position` {integer}
@@ -4136,8 +4104,7 @@ changes:
41364104
* `bytesWritten` {integer}
41374105
* `buffer` {Buffer|TypedArray|DataView}
41384106
4139-
Write `buffer` to the file specified by `fd`. If `buffer` is a normal object, it
4140-
must have an own `toString` function property.
4107+
Write `buffer` to the file specified by `fd`.
41414108
41424109
`offset` determines the part of the buffer to be written, and `length` is
41434110
an integer specifying the number of bytes to write.
@@ -5490,10 +5457,6 @@ this API: [`fs.writeFile()`][].
54905457
<!-- YAML
54915458
added: v0.1.21
54925459
changes:
5493-
- version: v14.12.0
5494-
pr-url: https://github.com/nodejs/node/pull/34993
5495-
description: The `buffer` parameter will stringify an object with an
5496-
explicit `toString` function.
54975460
- version: v14.0.0
54985461
pr-url: https://github.com/nodejs/node/pull/31030
54995462
description: The `buffer` parameter won't coerce unsupported input to
@@ -5511,15 +5474,12 @@ changes:
55115474
-->
55125475
55135476
* `fd` {integer}
5514-
* `buffer` {Buffer|TypedArray|DataView|string|Object}
5477+
* `buffer` {Buffer|TypedArray|DataView}
55155478
* `offset` {integer}
55165479
* `length` {integer}
55175480
* `position` {integer}
55185481
* Returns: {number} The number of bytes written.
55195482
5520-
If `buffer` is a plain object, it must have an own (not inherited) `toString`
5521-
function property.
5522-
55235483
For detailed information, see the documentation of the asynchronous version of
55245484
this API: [`fs.write(fd, buffer...)`][].
55255485
@@ -5528,10 +5488,6 @@ this API: [`fs.write(fd, buffer...)`][].
55285488
<!-- YAML
55295489
added: v0.11.5
55305490
changes:
5531-
- version: v14.12.0
5532-
pr-url: https://github.com/nodejs/node/pull/34993
5533-
description: The `string` parameter will stringify an object with an
5534-
explicit `toString` function.
55355491
- version: v14.0.0
55365492
pr-url: https://github.com/nodejs/node/pull/31030
55375493
description: The `string` parameter won't coerce unsupported input to
@@ -5542,14 +5498,11 @@ changes:
55425498
-->
55435499
55445500
* `fd` {integer}
5545-
* `string` {string|Object}
5501+
* `string` {string}
55465502
* `position` {integer}
55475503
* `encoding` {string}
55485504
* Returns: {number} The number of bytes written.
55495505
5550-
If `string` is a plain object, it must have an own (not inherited) `toString`
5551-
function property.
5552-
55535506
For detailed information, see the documentation of the asynchronous version of
55545507
this API: [`fs.write(fd, string...)`][].
55555508

‎lib/fs.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ const {
116116
validateRmOptionsSync,
117117
validateRmdirOptions,
118118
validateStringAfterArrayBufferView,
119+
validatePrimitiveStringAfterArrayBufferView,
119120
warnOnNonPortableTemplate
120121
} = require('internal/fs/utils');
121122
const {
@@ -853,7 +854,7 @@ ObjectDefineProperty(write, internalUtil.customPromisifyArgs,
853854
* Synchronously writes `buffer` to the
854855
* specified `fd` (file descriptor).
855856
* @param {number} fd
856-
* @param {Buffer | TypedArray | DataView | string | object} buffer
857+
* @param {Buffer | TypedArray | DataView | string} buffer
857858
* @param {number} [offset]
858859
* @param {number} [length]
859860
* @param {number} [position]
@@ -877,7 +878,7 @@ function writeSync(fd, buffer, offset, length, position) {
877878
result = binding.writeBuffer(fd, buffer, offset, length, position,
878879
undefined, ctx);
879880
} else {
880-
validateStringAfterArrayBufferView(buffer, 'buffer');
881+
validatePrimitiveStringAfterArrayBufferView(buffer, 'buffer');
881882
validateEncoding(buffer, length);
882883

883884
if (offset === undefined)

‎lib/internal/fs/promises.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ const {
6262
validateOffsetLengthWrite,
6363
validateRmOptions,
6464
validateRmdirOptions,
65-
validateStringAfterArrayBufferView,
65+
validatePrimitiveStringAfterArrayBufferView,
6666
warnOnNonPortableTemplate,
6767
} = require('internal/fs/utils');
6868
const { opendir } = require('internal/fs/dir');
@@ -529,7 +529,7 @@ async function write(handle, buffer, offset, length, position) {
529529
return { bytesWritten, buffer };
530530
}
531531

532-
validateStringAfterArrayBufferView(buffer, 'buffer');
532+
validatePrimitiveStringAfterArrayBufferView(buffer, 'buffer');
533533
validateEncoding(buffer, length);
534534
const bytesWritten = (await binding.writeString(handle.fd, buffer, offset,
535535
length, kUsePromises)) || 0;
@@ -759,7 +759,7 @@ async function writeFile(path, data, options) {
759759
const flag = options.flag || 'w';
760760

761761
if (!isArrayBufferView(data) && !isCustomIterable(data)) {
762-
validateStringAfterArrayBufferView(data, 'data');
762+
validatePrimitiveStringAfterArrayBufferView(data, 'data');
763763
data = Buffer.from(data, options.encoding || 'utf8');
764764
}
765765

‎lib/internal/fs/utils.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -889,6 +889,16 @@ const validateStringAfterArrayBufferView = hideStackFrames((buffer, name) => {
889889
);
890890
});
891891

892+
const validatePrimitiveStringAfterArrayBufferView = hideStackFrames((buffer, name) => {
893+
if (typeof buffer !== 'string') {
894+
throw new ERR_INVALID_ARG_TYPE(
895+
name,
896+
['string', 'Buffer', 'TypedArray', 'DataView'],
897+
buffer
898+
);
899+
}
900+
});
901+
892902
const validatePosition = hideStackFrames((position, name) => {
893903
if (typeof position === 'number') {
894904
validateInteger(position, 'position');
@@ -943,5 +953,6 @@ module.exports = {
943953
validateRmOptionsSync,
944954
validateRmdirOptions,
945955
validateStringAfterArrayBufferView,
956+
validatePrimitiveStringAfterArrayBufferView,
946957
warnOnNonPortableTemplate
947958
};

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

Lines 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(), null, undefined, 0n, () => {}, Symbol(), true];
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),

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,11 @@ fs.open(fn4, 'w', 0o644, common.mustSucceed((fd) => {
155155
);
156156
});
157157

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

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL