From beb75bd0317c48de1d88a8e54625a80e59b3959f Mon Sep 17 00:00:00 2001 From: Jordan Harband Date: Sun, 30 Aug 2020 21:20:48 -0700 Subject: [PATCH] fs: loosen validation to allow objects with an own toString function PR-URL: https://github.com/nodejs/node/pull/34993 Reviewed-By: Joyee Cheung Reviewed-By: Bradley Farias Reviewed-By: Matteo Collina Reviewed-By: Benjamin Gruenbaum Reviewed-By: Derek Lewis --- doc/api/fs.md | 80 ++++++++++++++++++------ lib/fs.js | 7 ++- lib/internal/fs/utils.js | 24 +++++-- test/parallel/test-fs-write-file-sync.js | 14 +++++ test/parallel/test-fs-write.js | 16 +++++ 5 files changed, 113 insertions(+), 28 deletions(-) diff --git a/doc/api/fs.md b/doc/api/fs.md index 0f67e31a52fefa..7932426b2983fc 100644 --- a/doc/api/fs.md +++ b/doc/api/fs.md @@ -4143,6 +4143,10 @@ This happens when: * `fd` {integer} -* `buffer` {Buffer|TypedArray|DataView} +* `buffer` {Buffer|TypedArray|DataView|string|Object} * `offset` {integer} * `length` {integer} * `position` {integer} @@ -4177,7 +4181,8 @@ changes: * `bytesWritten` {integer} * `buffer` {Buffer|TypedArray|DataView} -Write `buffer` to the file specified by `fd`. +Write `buffer` to the file specified by `fd`. If `buffer` is a normal object, it +must have an own `toString` function property. `offset` determines the part of the buffer to be written, and `length` is an integer specifying the number of bytes to write. @@ -4204,6 +4209,10 @@ the end of the file. * `fd` {integer} -* `string` {string} +* `string` {string|Object} * `position` {integer} * `encoding` {string} **Default:** `'utf8'` * `callback` {Function} @@ -4230,8 +4239,8 @@ changes: * `written` {integer} * `string` {string} -Write `string` to the file specified by `fd`. If `string` is not a string, then -an exception will be thrown. +Write `string` to the file specified by `fd`. If `string` is not a string, or an +object with an own `toString` function property, then an exception is thrown. `position` refers to the offset from the beginning of the file where this data should be written. If `typeof position !== 'number'` the data will be written at @@ -4263,6 +4272,10 @@ details. * `file` {string|Buffer|URL|integer} filename or file descriptor -* `data` {string|Buffer|TypedArray|DataView} +* `data` {string|Buffer|TypedArray|DataView|Object} * `options` {Object|string} * `encoding` {string|null} **Default:** `'utf8'` * `mode` {integer} **Default:** `0o666` @@ -4304,6 +4317,7 @@ When `file` is a file descriptor, the behavior is similar to calling a file descriptor. The `encoding` option is ignored if `data` is a buffer. +If `data` is a normal object, it must have an own `toString` function property. ```js const data = new Uint8Array(Buffer.from('Hello Node.js')); @@ -4353,6 +4367,10 @@ to contain only `', World'`. * `file` {string|Buffer|URL|integer} filename or file descriptor -* `data` {string|Buffer|TypedArray|DataView} +* `data` {string|Buffer|TypedArray|DataView|Object} * `options` {Object|string} * `encoding` {string|null} **Default:** `'utf8'` * `mode` {integer} **Default:** `0o666` @@ -4385,6 +4403,10 @@ this API: [`fs.writeFile()`][]. * `fd` {integer} -* `buffer` {Buffer|TypedArray|DataView} +* `buffer` {Buffer|TypedArray|DataView|string|Object} * `offset` {integer} * `length` {integer} * `position` {integer} @@ -4415,6 +4437,10 @@ this API: [`fs.write(fd, buffer...)`][]. * `fd` {integer} -* `string` {string} +* `string` {string|Object} * `position` {integer} * `encoding` {string} * Returns: {number} The number of bytes written. @@ -4788,13 +4814,17 @@ This function does not work on AIX versions before 7.1, it will resolve the -* `buffer` {Buffer|Uint8Array} +* `buffer` {Buffer|Uint8Array|string|Object} * `offset` {integer} * `length` {integer} * `position` {integer} @@ -4825,19 +4855,23 @@ the end of the file. -* `string` {string} +* `string` {string|Object} * `position` {integer} * `encoding` {string} **Default:** `'utf8'` * Returns: {Promise} -Write `string` to the file. If `string` is not a string, then -an exception will be thrown. +Write `string` to the file. If `string` is not a string, or an +object with an own `toString` function property, then an exception is thrown. The `Promise` is resolved with an object containing a `bytesWritten` property identifying the number of bytes written, and a `buffer` property containing @@ -4861,20 +4895,24 @@ the end of the file. -* `data` {string|Buffer|Uint8Array} +* `data` {string|Buffer|Uint8Array|Object} * `options` {Object|string} * `encoding` {string|null} **Default:** `'utf8'` * Returns: {Promise} Asynchronously writes data to a file, replacing the file if it already exists. -`data` can be a string or a buffer. The `Promise` will be resolved with no -arguments upon success. +`data` can be a string, a buffer, or an object with an own `toString` function +property. The `Promise` is resolved with no arguments upon success. The `encoding` option is ignored if `data` is a buffer. @@ -5492,6 +5530,10 @@ The `atime` and `mtime` arguments follow these rules: * `file` {string|Buffer|URL|FileHandle} filename or `FileHandle` -* `data` {string|Buffer|Uint8Array} +* `data` {string|Buffer|Uint8Array|Object} * `options` {Object|string} * `encoding` {string|null} **Default:** `'utf8'` * `mode` {integer} **Default:** `0o666` @@ -5507,8 +5549,8 @@ changes: * Returns: {Promise} Asynchronously writes data to a file, replacing the file if it already exists. -`data` can be a string or a buffer. The `Promise` will be resolved with no -arguments upon success. +`data` can be a string, a buffer, or an object with an own `toString` function +property. The `Promise` is resolved with no arguments upon success. The `encoding` option is ignored if `data` is a buffer. diff --git a/lib/fs.js b/lib/fs.js index 229167fc51eac6..67a050b4e8c480 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -37,6 +37,7 @@ const { ObjectDefineProperties, ObjectDefineProperty, Promise, + String, } = primordials; const { fs: constants } = internalBinding('constants'); @@ -663,7 +664,7 @@ function write(fd, buffer, offset, length, position, callback) { const req = new FSReqCallback(); req.oncomplete = wrapper; - return binding.writeString(fd, buffer, offset, length, req); + return binding.writeString(fd, String(buffer), offset, length, req); } ObjectDefineProperty(write, internalUtil.customPromisifyArgs, @@ -1383,7 +1384,7 @@ function writeFile(path, data, options, callback) { if (!isArrayBufferView(data)) { validateStringAfterArrayBufferView(data, 'data'); - data = Buffer.from(data, options.encoding || 'utf8'); + data = Buffer.from(String(data), options.encoding || 'utf8'); } if (isFd(path)) { @@ -1407,7 +1408,7 @@ function writeFileSync(path, data, options) { if (!isArrayBufferView(data)) { validateStringAfterArrayBufferView(data, 'data'); - data = Buffer.from(data, options.encoding || 'utf8'); + data = Buffer.from(String(data), options.encoding || 'utf8'); } const flag = options.flag || 'w'; diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 712bd87d85f6e6..ed89c616f26802 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -5,6 +5,7 @@ const { BigInt, DateNow, Error, + ObjectPrototypeHasOwnProperty, Number, NumberIsFinite, MathMin, @@ -697,13 +698,24 @@ const getValidMode = hideStackFrames((mode, type) => { }); const validateStringAfterArrayBufferView = hideStackFrames((buffer, name) => { - if (typeof buffer !== 'string') { - throw new ERR_INVALID_ARG_TYPE( - name, - ['string', 'Buffer', 'TypedArray', 'DataView'], - buffer - ); + if (typeof buffer === 'string') { + return; + } + + if ( + typeof buffer === 'object' && + buffer !== null && + typeof buffer.toString === 'function' && + ObjectPrototypeHasOwnProperty(buffer, 'toString') + ) { + return; } + + throw new ERR_INVALID_ARG_TYPE( + name, + ['string', 'Buffer', 'TypedArray', 'DataView'], + buffer + ); }); module.exports = { diff --git a/test/parallel/test-fs-write-file-sync.js b/test/parallel/test-fs-write-file-sync.js index 013322ed15ba46..2ef7d14365b781 100644 --- a/test/parallel/test-fs-write-file-sync.js +++ b/test/parallel/test-fs-write-file-sync.js @@ -101,3 +101,17 @@ tmpdir.refresh(); const content = fs.readFileSync(file, { encoding: 'utf8' }); assert.strictEqual(content, 'hello world!'); } + +// Test writeFileSync with an object with an own toString function +{ + const file = path.join(tmpdir.path, 'testWriteFileSyncStringify.txt'); + const data = { + toString() { + return 'hello world!'; + } + }; + + fs.writeFileSync(file, data, { encoding: 'utf8', flag: 'a' }); + const content = fs.readFileSync(file, { encoding: 'utf8' }); + assert.strictEqual(content, String(data)); +} diff --git a/test/parallel/test-fs-write.js b/test/parallel/test-fs-write.js index bdbbad9cb834d4..a6724eac300d16 100644 --- a/test/parallel/test-fs-write.js +++ b/test/parallel/test-fs-write.js @@ -32,6 +32,7 @@ tmpdir.refresh(); const fn = path.join(tmpdir.path, 'write.txt'); const fn2 = path.join(tmpdir.path, 'write2.txt'); const fn3 = path.join(tmpdir.path, 'write3.txt'); +const fn4 = path.join(tmpdir.path, 'write4.txt'); const expected = 'ümlaut.'; const constants = fs.constants; @@ -134,6 +135,21 @@ fs.open(fn3, 'w', 0o644, common.mustCall((err, fd) => { fs.write(fd, expected, done); })); +fs.open(fn4, 'w', 0o644, common.mustCall((err, fd) => { + assert.ifError(err); + + const done = common.mustCall((err, written) => { + assert.ifError(err); + assert.strictEqual(written, Buffer.byteLength(expected)); + fs.closeSync(fd); + }); + + const data = { + toString() { return expected; } + }; + fs.write(fd, data, done); +})); + [false, 'test', {}, [], null, undefined].forEach((i) => { assert.throws( () => fs.write(i, common.mustNotCall()),