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

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

/ node Public

Commit 8009cb0

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 Backport-PR-URL: #42631 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 6e56924 commit 8009cb0

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.
@@ -591,21 +586,17 @@ then resolves the promise with no arguments upon success.
591586
<!-- YAML
592587
added: v10.0.0
593588
changes:
594-
- version: v14.12.0
595-
pr-url: https://github.com/nodejs/node/pull/34993
596-
description: The `buffer` parameter will stringify an object with an
597-
explicit `toString` function.
598589
- version: v14.0.0
599590
pr-url: https://github.com/nodejs/node/pull/31030
600591
description: The `buffer` parameter won't coerce unsupported input to
601592
buffers anymore.
602593
-->
603594
604-
* `buffer` {Buffer|TypedArray|DataView|string|Object}
595+
* `buffer` {Buffer|TypedArray|DataView}
605596
* `offset` {integer} The start position from within `buffer` where the data
606597
to write begins. **Default:** `0`
607598
* `length` {integer} The number of bytes from `buffer` to write. **Default:**
608-
`buffer.byteLength`
599+
`buffer.byteLength - offset`
609600
* `position` {integer} The offset from the beginning of the file where the
610601
data from `buffer` should be written. If `position` is not a `number`,
611602
the data will be written at the current position. See the POSIX pwrite(2)
@@ -614,13 +605,10 @@ changes:
614605
615606
Write `buffer` to the file.
616607
617-
If `buffer` is a plain object, it must have an own (not inherited) `toString`
618-
function property.
619-
620608
The promise is resolved with an object containing two properties:
621609
622610
* `bytesWritten` {integer} the number of bytes written
623-
* `buffer` {Buffer|TypedArray|DataView|string|Object} a reference to the
611+
* `buffer` {Buffer|TypedArray|DataView} a reference to the
624612
`buffer` written.
625613
626614
It is unsafe to use `filehandle.write()` multiple times on the same file
@@ -636,31 +624,27 @@ the end of the file.
636624
<!-- YAML
637625
added: v10.0.0
638626
changes:
639-
- version: v14.12.0
640-
pr-url: https://github.com/nodejs/node/pull/34993
641-
description: The `string` parameter will stringify an object with an
642-
explicit `toString` function.
643627
- version: v14.0.0
644628
pr-url: https://github.com/nodejs/node/pull/31030
645629
description: The `string` parameter won't coerce unsupported input to
646630
strings anymore.
647631
-->
648632
649-
* `string` {string|Object}
633+
* `string` {string}
650634
* `position` {integer} The offset from the beginning of the file where the
651635
data from `string` should be written. If `position` is not a `number` the
652636
data will be written at the current position. See the POSIX pwrite(2)
653637
documentation for more detail.
654638
* `encoding` {string} The expected string encoding. **Default:** `'utf8'`
655639
* Returns: {Promise}
656640
657-
Write `string` to the file. If `string` is not a string, or an object with an
658-
own `toString` function property, the promise is rejected with an error.
641+
Write `string` to the file. If `string` is not a string, the promise is
642+
rejected with an error.
659643
660644
The promise is resolved with an object containing two properties:
661645
662646
* `bytesWritten` {integer} the number of bytes written
663-
* `buffer` {string|Object} a reference to the `string` written.
647+
* `buffer` {string} a reference to the `string` written.
664648
665649
It is unsafe to use `filehandle.write()` multiple times on the same file
666650
without waiting for the promise to be resolved (or rejected). For this
@@ -680,27 +664,21 @@ changes:
680664
- v14.18.0
681665
pr-url: https://github.com/nodejs/node/pull/37490
682666
description: The `data` argument supports `AsyncIterable`, `Iterable` and `Stream`.
683-
- version: v14.12.0
684-
pr-url: https://github.com/nodejs/node/pull/34993
685-
description: The `data` parameter will stringify an object with an
686-
explicit `toString` function.
687667
- version: v14.0.0
688668
pr-url: https://github.com/nodejs/node/pull/31030
689669
description: The `data` parameter won't coerce unsupported input to
690670
strings anymore.
691671
-->
692672
693-
* `data` {string|Buffer|TypedArray|DataView|Object|AsyncIterable|Iterable
694-
|Stream}
673+
* `data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream}
695674
* `options` {Object|string}
696675
* `encoding` {string|null} The expected character encoding when `data` is a
697676
string. **Default:** `'utf8'`
698677
* Returns: {Promise}
699678
700679
Asynchronously writes data to a file, replacing the file if it already exists.
701-
`data` can be a string, a buffer, an {AsyncIterable} or {Iterable} object, or an
702-
object with an own `toString` function
703-
property. The promise is resolved with no arguments upon success.
680+
`data` can be a string, a buffer, an {AsyncIterable} or {Iterable} object.
681+
The promise is resolved with no arguments upon success.
704682
705683
If `options` is a string, then it specifies the `encoding`.
706684
@@ -1536,19 +1514,14 @@ changes:
15361514
pr-url: https://github.com/nodejs/node/pull/35993
15371515
description: The options argument may include an AbortSignal to abort an
15381516
ongoing writeFile request.
1539-
- version: v14.12.0
1540-
pr-url: https://github.com/nodejs/node/pull/34993
1541-
description: The `data` parameter will stringify an object with an
1542-
explicit `toString` function.
15431517
- version: v14.0.0
15441518
pr-url: https://github.com/nodejs/node/pull/31030
15451519
description: The `data` parameter won't coerce unsupported input to
15461520
strings anymore.
15471521
-->
15481522
15491523
* `file` {string|Buffer|URL|FileHandle} filename or `FileHandle`
1550-
* `data` {string|Buffer|TypedArray|DataView|Object|AsyncIterable|Iterable
1551-
|Stream}
1524+
* `data` {string|Buffer|TypedArray|DataView|AsyncIterable|Iterable|Stream}
15521525
* `options` {Object|string}
15531526
* `encoding` {string|null} **Default:** `'utf8'`
15541527
* `mode` {integer} **Default:** `0o666`
@@ -1557,8 +1530,7 @@ changes:
15571530
* Returns: {Promise} Fulfills with `undefined` upon success.
15581531
15591532
Asynchronously writes data to a file, replacing the file if it already exists.
1560-
`data` can be a string, a {Buffer}, or, an object with an own (not inherited)
1561-
`toString` function property.
1533+
`data` can be a string, a buffer, an {AsyncIterable} or {Iterable} object.
15621534
15631535
The `encoding` option is ignored if `data` is a buffer.
15641536
@@ -4161,10 +4133,6 @@ This happens when:
41614133
<!-- YAML
41624134
added: v0.0.2
41634135
changes:
4164-
- version: v14.12.0
4165-
pr-url: https://github.com/nodejs/node/pull/34993
4166-
description: The `buffer` parameter will stringify an object with an
4167-
explicit `toString` function.
41684136
- version: v14.0.0
41694137
pr-url: https://github.com/nodejs/node/pull/31030
41704138
description: The `buffer` parameter won't coerce unsupported input to
@@ -4190,7 +4158,7 @@ changes:
41904158
-->
41914159
41924160
* `fd` {integer}
4193-
* `buffer` {Buffer|TypedArray|DataView|string|Object}
4161+
* `buffer` {Buffer|TypedArray|DataView}
41944162
* `offset` {integer}
41954163
* `length` {integer}
41964164
* `position` {integer}
@@ -4199,8 +4167,7 @@ changes:
41994167
* `bytesWritten` {integer}
42004168
* `buffer` {Buffer|TypedArray|DataView}
42014169
4202-
Write `buffer` to the file specified by `fd`. If `buffer` is a normal object, it
4203-
must have an own `toString` function property.
4170+
Write `buffer` to the file specified by `fd`.
42044171
42054172
`offset` determines the part of the buffer to be written, and `length` is
42064173
an integer specifying the number of bytes to write.
@@ -5557,10 +5524,6 @@ this API: [`fs.writeFile()`][].
55575524
<!-- YAML
55585525
added: v0.1.21
55595526
changes:
5560-
- version: v14.12.0
5561-
pr-url: https://github.com/nodejs/node/pull/34993
5562-
description: The `buffer` parameter will stringify an object with an
5563-
explicit `toString` function.
55645527
- version: v14.0.0
55655528
pr-url: https://github.com/nodejs/node/pull/31030
55665529
description: The `buffer` parameter won't coerce unsupported input to
@@ -5578,15 +5541,12 @@ changes:
55785541
-->
55795542
55805543
* `fd` {integer}
5581-
* `buffer` {Buffer|TypedArray|DataView|string|Object}
5544+
* `buffer` {Buffer|TypedArray|DataView}
55825545
* `offset` {integer}
55835546
* `length` {integer}
55845547
* `position` {integer}
55855548
* Returns: {number} The number of bytes written.
55865549
5587-
If `buffer` is a plain object, it must have an own (not inherited) `toString`
5588-
function property.
5589-
55905550
For detailed information, see the documentation of the asynchronous version of
55915551
this API: [`fs.write(fd, buffer...)`][].
55925552
@@ -5595,10 +5555,6 @@ this API: [`fs.write(fd, buffer...)`][].
55955555
<!-- YAML
55965556
added: v0.11.5
55975557
changes:
5598-
- version: v14.12.0
5599-
pr-url: https://github.com/nodejs/node/pull/34993
5600-
description: The `string` parameter will stringify an object with an
5601-
explicit `toString` function.
56025558
- version: v14.0.0
56035559
pr-url: https://github.com/nodejs/node/pull/31030
56045560
description: The `string` parameter won't coerce unsupported input to
@@ -5609,14 +5565,11 @@ changes:
56095565
-->
56105566
56115567
* `fd` {integer}
5612-
* `string` {string|Object}
5568+
* `string` {string}
56135569
* `position` {integer}
56145570
* `encoding` {string}
56155571
* Returns: {number} The number of bytes written.
56165572
5617-
If `string` is a plain object, it must have an own (not inherited) `toString`
5618-
function property.
5619-
56205573
For detailed information, see the documentation of the asynchronous version of
56215574
this API: [`fs.write(fd, string...)`][].
56225575

‎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
@@ -65,7 +65,7 @@ const {
6565
validateOffsetLengthWrite,
6666
validateRmOptions,
6767
validateRmdirOptions,
68-
validateStringAfterArrayBufferView,
68+
validatePrimitiveStringAfterArrayBufferView,
6969
warnOnNonPortableTemplate,
7070
} = require('internal/fs/utils');
7171
const { opendir } = require('internal/fs/dir');
@@ -581,7 +581,7 @@ async function write(handle, buffer, offset, length, position) {
581581
return { bytesWritten, buffer };
582582
}
583583

584-
validateStringAfterArrayBufferView(buffer, 'buffer');
584+
validatePrimitiveStringAfterArrayBufferView(buffer, 'buffer');
585585
validateEncoding(buffer, length);
586586
const bytesWritten = (await binding.writeString(handle.fd, buffer, offset,
587587
length, kUsePromises)) || 0;
@@ -811,7 +811,7 @@ async function writeFile(path, data, options) {
811811
const flag = options.flag || 'w';
812812

813813
if (!isArrayBufferView(data) && !isCustomIterable(data)) {
814-
validateStringAfterArrayBufferView(data, 'data');
814+
validatePrimitiveStringAfterArrayBufferView(data, 'data');
815815
data = Buffer.from(data, options.encoding || 'utf8');
816816
}
817817

‎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