fixup! fixup! fixup! fs: change default value of position in read and…

… readSync
nodejs · Jan 29, 2021 · 89f3abf · 89f3abf
1 parent a0cb2fb
commit 89f3abf
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 3 deletions.
diff --git a/doc/api/fs.md b/doc/api/fs.md
@@ -2921,6 +2921,9 @@ this API: [`fs.open()`][].
 <!-- YAML
 added: v0.0.2
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37101
+    description: Runtime description
   - version: v10.10.0
     pr-url: https://github.com/nodejs/node/pull/22150
     description: The `buffer` parameter can now be any `TypedArray`, or a
@@ -2970,6 +2973,9 @@ added:
  - v13.11.0
  - v12.17.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37101
+    description: Runtime description
   - version:
      - v13.11.0
      - v12.17.0
@@ -3288,6 +3294,9 @@ the link path returned will be passed as a `Buffer` object.
 <!-- YAML
 added: v0.1.21
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37101
+    description: Runtime description
   - version: v10.10.0
     pr-url: https://github.com/nodejs/node/pull/22150
     description: The `buffer` parameter can now be any `TypedArray` or a
@@ -3315,6 +3324,9 @@ added:
  - v13.13.0
  - v12.17.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37101
+    description: Runtime description
   - version:
      - v13.13.0
      - v12.17.0

diff --git a/lib/fs.js b/lib/fs.js
@@ -547,6 +547,14 @@ function read(fd, buffer, offset, length, position = -1, callback) {
 
   validateOffsetLengthRead(offset, length, buffer.byteLength);
 
+  if (position === null) {
+    process.emitWarning(
+      `The provided ${position} is not a valid position, and is supported ` +
+      'in the fs module solely for compatibility.',
+      'DeprecationWarning', 'DEP01149');
+    position = -1;
+  }
+
   validatePosition(position, 'position');
 
   function wrapper(err, bytesRead) {
@@ -602,6 +610,14 @@ function readSync(fd, buffer, offset, length, position = -1) {
 
   validateOffsetLengthRead(offset, length, buffer.byteLength);
 
+  if (position === null) {
+    process.emitWarning(
+      `The provided ${position} is not a valid position, and is supported ` +
+      'in the fs module solely for compatibility.',
+      'DeprecationWarning', 'DEP0149');
+    position = -1;
+  }
+
   validatePosition(position, 'position');
 
   const ctx = {};

diff --git a/test/parallel/test-fs-read-type.js b/test/parallel/test-fs-read-type.js
@@ -8,7 +8,6 @@ const filepath = fixtures.path('x.txt');
 const fd = fs.openSync(filepath, 'r');
 const expected = 'xyz\n';
 
-
 // Error must be thrown with string
 assert.throws(
   () => fs.read(fd, expected.length, 0, 'utf-8', common.mustNotCall()),
@@ -76,7 +75,7 @@ assert.throws(() => {
            'It must be >= 0. Received -1'
 });
 
-[true, () => {}, {}, '', null].forEach((value) => {
+[true, () => {}, {}, ''].forEach((value) => {
   assert.throws(() => {
     fs.read(fd,
             Buffer.allocUnsafe(expected.length),
@@ -90,6 +89,21 @@ assert.throws(() => {
   });
 });
 
+{
+  fs.read(fd,
+          Buffer.allocUnsafe(expected.length),
+          0,
+          expected.length,
+          null,
+          common.mustCall());
+  common.expectWarning(
+    'DeprecationWarning',
+    'The provided null is not a valid position, and is supported ' +
+    'in the fs module solely for compatibility.',
+    'DEP01149',
+  );
+}
+
 [0.5, 2 ** 53, 2n ** 63n].forEach((value) => {
   assert.throws(() => {
     fs.read(fd,
@@ -197,7 +211,7 @@ assert.throws(() => {
            'It must be <= 4. Received 5'
 });
 
-[true, () => {}, {}, '', null].forEach((value) => {
+[true, () => {}, {}, ''].forEach((value) => {
   assert.throws(() => {
     fs.readSync(fd,
                 Buffer.allocUnsafe(expected.length),
@@ -210,6 +224,20 @@ assert.throws(() => {
   });
 });
 
+{
+  fs.readSync(fd,
+              Buffer.allocUnsafe(expected.length),
+              0,
+              expected.length,
+              null);
+  common.expectWarning(
+    'DeprecationWarning',
+    'The provided null is not a valid position, and is supported ' +
+    'in the fs module solely for compatibility.',
+    'DEP01149',
+  );
+}
+
 [0.5, 2 ** 53, 2n ** 63n].forEach((value) => {
   assert.throws(() => {
     fs.readSync(fd,