fs: loosen validation to allow objects with an own toString function

PR-URL: #34993 Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
nodejs · Sep 15, 2020 · 9cf9e4a · 9cf9e4a
1 parent 0577127
commit 9cf9e4a
Show file tree

Hide file tree

Showing 5 changed files with 113 additions and 28 deletions.
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -4163,6 +4163,10 @@ This happens when:
 <!-- YAML
 added: v0.0.2
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34993
+    description: The `buffer` parameter will stringify an object with an
+                 explicit `toString` function.
   - version: v14.0.0
     pr-url: https://github.com/nodejs/node/pull/31030
     description: The `buffer` parameter won't coerce unsupported input to
@@ -4188,7 +4192,7 @@ changes:
 -->
 
 * `fd` {integer}
-* `buffer` {Buffer|TypedArray|DataView}
+* `buffer` {Buffer|TypedArray|DataView|string|Object}
 * `offset` {integer}
 * `length` {integer}
 * `position` {integer}
@@ -4197,7 +4201,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.
@@ -4224,6 +4229,10 @@ the end of the file.
 <!-- YAML
 added: v0.11.5
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34993
+    description: The `string` parameter will stringify an object with an
+                 explicit `toString` function.
   - version: v14.0.0
     pr-url: https://github.com/nodejs/node/pull/31030
     description: The `string` parameter won't coerce unsupported input to
@@ -4242,16 +4251,16 @@ changes:
 -->
 
 * `fd` {integer}
-* `string` {string}
+* `string` {string|Object}
 * `position` {integer}
 * `encoding` {string} **Default:** `'utf8'`
 * `callback` {Function}
   * `err` {Error}
   * `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
@@ -4283,6 +4292,10 @@ details.
 <!-- YAML
 added: v0.1.29
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34993
+    description: The `data` parameter will stringify an object with an
+                 explicit `toString` function.
   - version: v14.0.0
     pr-url: https://github.com/nodejs/node/pull/31030
     description: The `data` parameter won't coerce unsupported input to
@@ -4308,7 +4321,7 @@ changes:
 -->
 
 * `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`
@@ -4324,6 +4337,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'));
@@ -4373,6 +4387,10 @@ to contain only `', World'`.
 <!-- YAML
 added: v0.1.29
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34993
+    description: The `data` parameter will stringify an object with an
+                 explicit `toString` function.
   - version: v14.0.0
     pr-url: https://github.com/nodejs/node/pull/31030
     description: The `data` parameter won't coerce unsupported input to
@@ -4390,7 +4408,7 @@ changes:
 -->
 
 * `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`
@@ -4405,6 +4423,10 @@ this API: [`fs.writeFile()`][].
 <!-- YAML
 added: v0.1.21
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34993
+    description: The `buffer` parameter will stringify an object with an
+                 explicit `toString` function.
   - version: v14.0.0
     pr-url: https://github.com/nodejs/node/pull/31030
     description: The `buffer` parameter won't coerce unsupported input to
@@ -4422,7 +4444,7 @@ changes:
 -->
 
 * `fd` {integer}
-* `buffer` {Buffer|TypedArray|DataView}
+* `buffer` {Buffer|TypedArray|DataView|string|Object}
 * `offset` {integer}
 * `length` {integer}
 * `position` {integer}
@@ -4435,6 +4457,10 @@ this API: [`fs.write(fd, buffer...)`][].
 <!-- YAML
 added: v0.11.5
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34993
+    description: The `string` parameter will stringify an object with an
+                 explicit `toString` function.
   - version: v14.0.0
     pr-url: https://github.com/nodejs/node/pull/31030
     description: The `string` parameter won't coerce unsupported input to
@@ -4445,7 +4471,7 @@ changes:
 -->
 
 * `fd` {integer}
-* `string` {string}
+* `string` {string|Object}
 * `position` {integer}
 * `encoding` {string}
 * Returns: {number} The number of bytes written.
@@ -4812,13 +4838,17 @@ This function does not work on AIX versions before 7.1, it will resolve the
 <!-- YAML
 added: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34993
+    description: The `buffer` parameter will stringify an object with an
+                 explicit `toString` function.
   - version: v14.0.0
     pr-url: https://github.com/nodejs/node/pull/31030
     description: The `buffer` parameter won't coerce unsupported input to
                  buffers anymore.
 -->
 
-* `buffer` {Buffer|Uint8Array}
+* `buffer` {Buffer|Uint8Array|string|Object}
 * `offset` {integer}
 * `length` {integer}
 * `position` {integer}
@@ -4849,19 +4879,23 @@ the end of the file.
 <!-- YAML
 added: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34993
+    description: The `string` parameter will stringify an object with an
+                 explicit `toString` function.
   - version: v14.0.0
     pr-url: https://github.com/nodejs/node/pull/31030
     description: The `string` parameter won't coerce unsupported input to
                  strings anymore.
 -->
 
-* `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
@@ -4885,20 +4919,24 @@ the end of the file.
 <!-- YAML
 added: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34993
+    description: The `data` parameter will stringify an object with an
+                 explicit `toString` function.
   - version: v14.0.0
     pr-url: https://github.com/nodejs/node/pull/31030
     description: The `data` parameter won't coerce unsupported input to
                  strings anymore.
 -->
 
-* `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.
 
@@ -5516,23 +5554,27 @@ The `atime` and `mtime` arguments follow these rules:
 <!-- YAML
 added: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/34993
+    description: The `data` parameter will stringify an object with an
+                 explicit `toString` function.
   - version: v14.0.0
     pr-url: https://github.com/nodejs/node/pull/31030
     description: The `data` parameter won't coerce unsupported input to
                  strings anymore.
 -->
 
 * `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`
   * `flag` {string} See [support of file system `flags`][]. **Default:** `'w'`.
 * 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
@@ -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
@@ -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
@@ -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
@@ -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()),