From 94f3eed22927cbd23de0bf077e567abf65b29338 Mon Sep 17 00:00:00 2001
From: Lucas Holmquist <lholmqui@redhat.com>
Date: Thu, 16 Jan 2020 09:46:28 -0500
Subject: [PATCH] fs: make fs.read params optional

This makes all the parameters of the `fs.read` function, except
for `fd` and the callback(when not using as a promise) optional.

They will default to sensible defaults.

Fixes: https://github.com/nodejs/node/issues/31237

PR-URL: https://github.com/nodejs/node/pull/31402
Reviewed-By: Robert Nagy <ronagy@icloud.com>
---
 doc/api/fs.md                                 | 34 ++++++++++++++++++
 lib/fs.js                                     | 26 ++++++++++++++
 test/parallel/test-fs-read-optional-params.js | 36 +++++++++++++++++++
 .../test-fs-read-promises-optional-params.js  | 19 ++++++++++
 4 files changed, 115 insertions(+)
 create mode 100644 test/parallel/test-fs-read-optional-params.js
 create mode 100644 test/parallel/test-fs-read-promises-optional-params.js

diff --git a/doc/api/fs.md b/doc/api/fs.md
index c561715d1afbef..6cdd5bfdcb42c0 100644
--- a/doc/api/fs.md
+++ b/doc/api/fs.md
@@ -2775,6 +2775,29 @@ The callback is given the three arguments, `(err, bytesRead, buffer)`.
 If this method is invoked as its [`util.promisify()`][]ed version, it returns
 a `Promise` for an `Object` with `bytesRead` and `buffer` properties.
 
+## `fs.read(fd, [options,] callback)`
+<!-- YAML
+added: REPLACEME
+changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/31402
+    description: Options object can be passed in
+                 to make Buffer, offset, length and position optional
+-->
+* `fd` {integer}
+* `options` {Object}
+  * `buffer` {Buffer|TypedArray|DataView} **Default:** `Buffer.alloc(16384)`
+  * `offset` {integer} **Default:** `0`
+  * `length` {integer} **Default:** `buffer.length`
+  * `position` {integer} **Default:** `null`
+* `callback` {Function}
+  * `err` {Error}
+  * `bytesRead` {integer}
+  * `buffer` {Buffer}
+
+Similar to the above `fs.read` function, this version takes an optional `options` object.
+If no `options` object is specified, it will default with the above values.
+
 ## `fs.readdir(path[, options], callback)`
 <!-- YAML
 added: v0.1.8
@@ -4362,6 +4385,17 @@ Following successful read, the `Promise` is resolved with an object with a
 `bytesRead` property specifying the number of bytes read, and a `buffer`
 property that is a reference to the passed in `buffer` argument.
 
+#### `filehandle.read(options)`
+<!-- YAML
+added: REPLACEME
+-->
+* `options` {Object}
+  * `buffer` {Buffer|Uint8Array} **Default:** `Buffer.alloc(16384)`
+  * `offset` {integer} **Default:** `0`
+  * `length` {integer} **Default:** `buffer.length`
+  * `position` {integer} **Default:** `null`
+* Returns: {Promise}
+
 #### `filehandle.readFile(options)`
 <!-- YAML
 added: v10.0.0
diff --git a/lib/fs.js b/lib/fs.js
index c3db4e69aadce2..033c25cda948d6 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -463,8 +463,34 @@ function openSync(path, flags, mode) {
   return result;
 }
 
+// usage:
+// fs.read(fd, buffer, offset, length, position, callback);
+// OR
+// fs.read(fd, {}, callback)
 function read(fd, buffer, offset, length, position, callback) {
   validateInt32(fd, 'fd', 0);
+
+  if (arguments.length <= 3) {
+    // Assume fs.read(fd, options, callback)
+    let options = {};
+    if (arguments.length < 3) {
+      // This is fs.read(fd, callback)
+      // buffer will be the callback
+      callback = buffer;
+    } else {
+      // This is fs.read(fd, {}, callback)
+      // buffer will be the options object
+      // offset is the callback
+      options = buffer;
+      callback = offset;
+    }
+
+    buffer = options.buffer || Buffer.alloc(16384);
+    offset = options.offset || 0;
+    length = options.length || buffer.length;
+    position = options.position;
+  }
+
   validateBuffer(buffer);
   callback = maybeCallback(callback);
 
diff --git a/test/parallel/test-fs-read-optional-params.js b/test/parallel/test-fs-read-optional-params.js
new file mode 100644
index 00000000000000..9b915d8ce4838c
--- /dev/null
+++ b/test/parallel/test-fs-read-optional-params.js
@@ -0,0 +1,36 @@
+'use strict';
+
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+const fs = require('fs');
+const assert = require('assert');
+const filepath = fixtures.path('x.txt');
+const fd = fs.openSync(filepath, 'r');
+
+const expected = Buffer.from('xyz\n');
+const defaultBufferAsync = Buffer.alloc(16384);
+const bufferAsOption = Buffer.allocUnsafe(expected.length);
+
+// Test passing in an empty options object
+fs.read(fd, { position: 0 }, common.mustCall((err, bytesRead, buffer) => {
+  assert.strictEqual(bytesRead, expected.length);
+  assert.deepStrictEqual(defaultBufferAsync.length, buffer.length);
+}));
+
+// Test not passing in any options object
+fs.read(fd, common.mustCall((err, bytesRead, buffer) => {
+  assert.strictEqual(bytesRead, expected.length);
+  assert.deepStrictEqual(defaultBufferAsync.length, buffer.length);
+}));
+
+// Test passing in options
+fs.read(fd, {
+  buffer: bufferAsOption,
+  offset: 0,
+  lenght: bufferAsOption.length,
+  position: 0
+},
+        common.mustCall((err, bytesRead, buffer) => {
+          assert.strictEqual(bytesRead, expected.length);
+          assert.deepStrictEqual(bufferAsOption.length, buffer.length);
+        }));
diff --git a/test/parallel/test-fs-read-promises-optional-params.js b/test/parallel/test-fs-read-promises-optional-params.js
new file mode 100644
index 00000000000000..9d19eaa41151a8
--- /dev/null
+++ b/test/parallel/test-fs-read-promises-optional-params.js
@@ -0,0 +1,19 @@
+'use strict';
+
+const common = require('../common');
+const fixtures = require('../common/fixtures');
+const fs = require('fs');
+const read = require('util').promisify(fs.read);
+const assert = require('assert');
+const filepath = fixtures.path('x.txt');
+const fd = fs.openSync(filepath, 'r');
+
+const expected = Buffer.from('xyz\n');
+const defaultBufferAsync = Buffer.alloc(16384);
+
+read(fd, {})
+  .then(function({ bytesRead, buffer }) {
+    assert.strictEqual(bytesRead, expected.length);
+    assert.deepStrictEqual(defaultBufferAsync.length, buffer.length);
+  })
+  .then(common.mustCall());