fs: make promises/write and writeSync params optional

This change allows passing objects as "named parameters" and
prohibits passing objects with own toString property as strings.

Fixes: nodejs#41666

doc/api/fs.md

@@ -586,7 +586,7 @@ changes:
                  buffers anymore.
 -->
 
-* `buffer` {Buffer|TypedArray|DataView|string|Object}
+* `buffer` {Buffer|TypedArray|DataView}
 * `offset` {integer} The start position from within `buffer` where the data
   to write begins. **Default:** `0`
 * `length` {integer} The number of bytes from `buffer` to write. **Default:**
@@ -599,13 +599,10 @@ changes:
 
 Write `buffer` to the file.
 
-If `buffer` is a plain object, it must have an own (not inherited) `toString`
-function property.
-
 The promise is resolved with an object containing two properties:
 
 * `bytesWritten` {integer} the number of bytes written
-* `buffer` {Buffer|TypedArray|DataView|string|Object} a reference to the
+* `buffer` {Buffer|TypedArray|DataView} a reference to the
   `buffer` written.
 
 It is unsafe to use `filehandle.write()` multiple times on the same file
@@ -616,11 +613,34 @@ On Linux, positional writes do not work when the file is opened in append mode.
 The kernel ignores the position argument and always appends the data to
 the end of the file.
 
+#### `filehandle.write(buffer, options)`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* `buffer` {Buffer|TypedArray|DataView}
+* `options` {Object}
+  * `offset` {integer} **Default:** `0`
+  * `length` {integer} **Default:** `buffer.byteLength`
+  * `position` {integer} **Default:** `null`
+* Returns: {Promise}
+
+Write `buffer` to the file.
+
+Similar to the above `filehandle.write` function, this version takes an
+optional `options` object. If no `options` object is specified, it will
+default with the above values.
+
 #### `filehandle.write(string[, position[, encoding]])`
 
 <!-- YAML
 added: v10.0.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/41677
+    description: The `string` parameter no longer accepts objects with an
+                 own `toString` function property.
   - version: v14.12.0
     pr-url: https://github.com/nodejs/node/pull/34993
     description: The `string` parameter will stringify an object with an
@@ -631,21 +651,21 @@ changes:
                  strings anymore.
 -->
 
-* `string` {string|Object}
+* `string` {string}
 * `position` {integer} The offset from the beginning of the file where the
   data from `string` should be written. If `position` is not a `number` the
   data will be written at the current position. See the POSIX pwrite(2)
   documentation for more detail.
 * `encoding` {string} The expected string encoding. **Default:** `'utf8'`
 * Returns: {Promise}
 
-Write `string` to the file. If `string` is not a string, or an object with an
-own `toString` function property, the promise is rejected with an error.
+Write `string` to the file. If `string` is not a string, the promise is
+rejected with an error.
 
 The promise is resolved with an object containing two properties:
 
 * `bytesWritten` {integer} the number of bytes written
-* `buffer` {string|Object} a reference to the `string` written.
+* `buffer` {string} a reference to the `string` written.
 
 It is unsafe to use `filehandle.write()` multiple times on the same file
 without waiting for the promise to be resolved (or rejected). For this
@@ -4375,7 +4395,7 @@ changes:
 -->
 
 * `fd` {integer}
-* `buffer` {Buffer|TypedArray|DataView|string|Object}
+* `buffer` {Buffer|TypedArray|DataView}
 * `offset` {integer}
 * `length` {integer}
 * `position` {integer}
@@ -4384,8 +4404,7 @@ changes:
   * `bytesWritten` {integer}
   * `buffer` {Buffer|TypedArray|DataView}
 
-Write `buffer` to the file specified by `fd`. If `buffer` is a normal object, it
-must have an own `toString` function property.
+Write `buffer` to the file specified by `fd`.
 
 `offset` determines the part of the buffer to be written, and `length` is
 an integer specifying the number of bytes to write.
@@ -5776,14 +5795,28 @@ changes:
 -->
 
 * `fd` {integer}
-* `buffer` {Buffer|TypedArray|DataView|string|Object}
+* `buffer` {Buffer|TypedArray|DataView}
 * `offset` {integer}
 * `length` {integer}
 * `position` {integer}
 * Returns: {number} The number of bytes written.
 
-If `buffer` is a plain object, it must have an own (not inherited) `toString`
-function property.
+For detailed information, see the documentation of the asynchronous version of
+this API: [`fs.write(fd, buffer...)`][].
+
+### `fs.writeSync(fd, buffer, options)`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* `fd` {integer}
+* `buffer` {Buffer|TypedArray|DataView}
+* `options` {Object}
+  * `offset` {integer} **Default:** `0`
+  * `length` {integer} **Default:** `buffer.byteLength`
+  * `position` {integer} **Default:** `null`
+* Returns: {number} The number of bytes written.
 
 For detailed information, see the documentation of the asynchronous version of
 this API: [`fs.write(fd, buffer...)`][].
@@ -5793,6 +5826,10 @@ this API: [`fs.write(fd, buffer...)`][].
 <!-- YAML
 added: v0.11.5
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/41677
+    description: The `string` parameter no longer accepts objects with an
+                 own `toString` function property.
   - version: v14.12.0
     pr-url: https://github.com/nodejs/node/pull/34993
     description: The `string` parameter will stringify an object with an
@@ -5807,14 +5844,11 @@ changes:
 -->
 
 * `fd` {integer}
-* `string` {string|Object}
+* `string` {string}
 * `position` {integer}
 * `encoding` {string}
 * Returns: {number} The number of bytes written.
 
-If `string` is a plain object, it must have an own (not inherited) `toString`
-function property.
-
 For detailed information, see the documentation of the asynchronous version of
 this API: [`fs.write(fd, string...)`][].
 

lib/fs.js

@@ -116,6 +116,7 @@ const {
   validateRmOptionsSync,
   validateRmdirOptions,
   validateStringAfterArrayBufferView,
+  validatePrimitiveStringAfterArrayBufferView,
   warnOnNonPortableTemplate
 } = require('internal/fs/utils');
 const {
@@ -852,16 +853,27 @@ ObjectDefineProperty(write, internalUtil.customPromisifyArgs,
  * Synchronously writes `buffer` to the
  * specified `fd` (file descriptor).
  * @param {number} fd
- * @param {Buffer | TypedArray | DataView | string | object} buffer
- * @param {number} [offset]
- * @param {number} [length]
- * @param {number} [position]
+ * @param {Buffer | TypedArray | DataView | string} buffer
+ * @param {{
+ *   offset?: number;
+ *   length?: number;
+ *   position?: number;
+ *   }} [offset]
  * @returns {number}
  */
 function writeSync(fd, buffer, offset, length, position) {
   fd = getValidatedFd(fd);
   const ctx = {};
   let result;
+
+  if (typeof offset === 'object' && offset !== null) {
+    ({
+      offset = 0,
+      length = buffer.byteLength,
+      position = null
+    } = offset);
+  }
+
   if (isArrayBufferView(buffer)) {
     if (position === undefined)
       position = null;
@@ -876,7 +888,7 @@ function writeSync(fd, buffer, offset, length, position) {
     result = binding.writeBuffer(fd, buffer, offset, length, position,
                                  undefined, ctx);
   } else {
-    validateStringAfterArrayBufferView(buffer, 'buffer');
+    validatePrimitiveStringAfterArrayBufferView(buffer, 'buffer');
     validateEncoding(buffer, length);
 
     if (offset === undefined)

lib/internal/fs/promises.js

@@ -66,6 +66,7 @@ const {
   validateRmOptions,
   validateRmdirOptions,
   validateStringAfterArrayBufferView,
+  validatePrimitiveStringAfterArrayBufferView,
   warnOnNonPortableTemplate,
 } = require('internal/fs/utils');
 const { opendir } = require('internal/fs/dir');
@@ -560,7 +561,18 @@ async function readv(handle, buffers, position) {
   return { bytesRead, buffers };
 }
 
-async function write(handle, buffer, offset, length, position) {
+async function write(handle, bufferOrOptions, offset, length, position) {
+  let buffer = bufferOrOptions;
+  if (!isArrayBufferView(buffer) && typeof buffer !== 'string') {
+    validateBuffer(bufferOrOptions?.buffer);
+    ({
+      buffer,
+      offset = 0,
+      length = buffer.byteLength,
+      position = null
+    } = bufferOrOptions);
+  }
+
   if (buffer?.byteLength === 0)
     return { bytesWritten: 0, buffer };
 
@@ -581,7 +593,7 @@ async function write(handle, buffer, offset, length, position) {
     return { bytesWritten, buffer };
   }
 
-  validateStringAfterArrayBufferView(buffer, 'buffer');
+  validatePrimitiveStringAfterArrayBufferView(buffer, 'buffer');
   validateEncoding(buffer, length);
   const bytesWritten = (await binding.writeString(handle.fd, buffer, offset,
                                                   length, kUsePromises)) || 0;

lib/internal/fs/utils.js

@@ -889,6 +889,16 @@ const validateStringAfterArrayBufferView = hideStackFrames((buffer, name) => {
   );
 });
 
+const validatePrimitiveStringAfterArrayBufferView = hideStackFrames((buffer, name) => {
+  if (typeof buffer !== 'string') {
+    throw new ERR_INVALID_ARG_TYPE(
+      name,
+      ['string', 'Buffer', 'TypedArray', 'DataView'],
+      buffer
+    );
+  }
+});
+
 const validatePosition = hideStackFrames((position, name) => {
   if (typeof position === 'number') {
     validateInteger(position, 'position');
@@ -943,5 +953,6 @@ module.exports = {
   validateRmOptionsSync,
   validateRmdirOptions,
   validateStringAfterArrayBufferView,
+  validatePrimitiveStringAfterArrayBufferView,
   warnOnNonPortableTemplate
 };