fs: validate position argument before length === 0 early return

PR-URL: #62674
Reviewed-By: Aviv Keller <me@aviv.sh>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Author: geeksilva97

lib/fs.js

@@ -641,6 +641,12 @@ function read(fd, buffer, offsetOrOptions, length, position, callback) {
 
   length |= 0;
 
+  if (position == null) {
+    position = -1;
+  } else {
+    validatePosition(position, 'position', length);
+  }
+
   if (length === 0) {
     return process.nextTick(function tick() {
       callback(null, 0, buffer);
@@ -654,12 +660,6 @@ function read(fd, buffer, offsetOrOptions, length, position, callback) {
 
   validateOffsetLengthRead(offset, length, buffer.byteLength);
 
-  if (position == null) {
-    position = -1;
-  } else {
-    validatePosition(position, 'position', length);
-  }
-
   function wrapper(err, bytesRead) {
     // Retain a reference to buffer so that it can't be GC'ed too soon.
     callback(err, bytesRead || 0, buffer);
@@ -712,6 +712,12 @@ function readSync(fd, buffer, offsetOrOptions, length, position) {
 
   length |= 0;
 
+  if (position == null) {
+    position = -1;
+  } else {
+    validatePosition(position, 'position', length);
+  }
+
   if (length === 0) {
     return 0;
   }
@@ -723,12 +729,6 @@ function readSync(fd, buffer, offsetOrOptions, length, position) {
 
   validateOffsetLengthRead(offset, length, buffer.byteLength);
 
-  if (position == null) {
-    position = -1;
-  } else {
-    validatePosition(position, 'position', length);
-  }
-
   return binding.read(fd, buffer, offset, length, position);
 }
 

lib/internal/fs/promises.js

@@ -1319,6 +1319,12 @@ async function read(handle, bufferOrParams, offset, length, position) {
 
   length ??= buffer.byteLength - offset;
 
+  if (position == null) {
+    position = -1;
+  } else {
+    validatePosition(position, 'position', length);
+  }
+
   if (length === 0)
     return { __proto__: null, bytesRead: length, buffer };
 
@@ -1329,12 +1335,6 @@ async function read(handle, bufferOrParams, offset, length, position) {
 
   validateOffsetLengthRead(offset, length, buffer.byteLength);
 
-  if (position == null) {
-    position = -1;
-  } else {
-    validatePosition(position, 'position', length);
-  }
-
   const bytesRead = (await PromisePrototypeThen(
     binding.read(handle.fd, buffer, offset, length, position, kUsePromises),
     undefined,

test/parallel/test-fs-read-position-validation.mjs

@@ -91,3 +91,26 @@ async function testInvalid(code, position) {
     await testInvalid('ERR_INVALID_ARG_TYPE', badTypeValue);
   }
 }
+
+{
+  const emptyBuffer = Buffer.alloc(0);
+  await new Promise((resolve, reject) => {
+    fs.open(filepath, 'r', common.mustSucceed((fd) => {
+      try {
+        assert.throws(
+          () => fs.read(fd, emptyBuffer, 0, 0, { not: 'a number' }, common.mustNotCall()),
+          { code: 'ERR_INVALID_ARG_TYPE' }
+        );
+        assert.throws(
+          () => fs.read(fd, { buffer: emptyBuffer, offset: 0, length: 0, position: 'string' }, common.mustNotCall()),
+          { code: 'ERR_INVALID_ARG_TYPE' }
+        );
+        resolve();
+      } catch (err) {
+        reject(err);
+      } finally {
+        fs.close(fd, common.mustSucceed());
+      }
+    }));
+  });
+}

test/parallel/test-fs-read-promises-position-validation.mjs

@@ -79,6 +79,30 @@ async function testInvalid(code, position) {
   for (const badTypeValue of [
     false, true, '1', Symbol(1), {}, [], () => {}, Promise.resolve(1),
   ]) {
-    testInvalid('ERR_INVALID_ARG_TYPE', badTypeValue);
+    await testInvalid('ERR_INVALID_ARG_TYPE', badTypeValue);
+  }
+}
+
+{
+  const emptyBuffer = Buffer.alloc(0);
+  let fh;
+  try {
+    fh = await fs.promises.open(filepath, 'r');
+    await assert.rejects(
+      fh.read(emptyBuffer, 0, 0, { not: 'a number' }),
+      { code: 'ERR_INVALID_ARG_TYPE' }
+    );
+  } finally {
+    await fh?.close();
+  }
+
+  try {
+    fh = await fs.promises.open(filepath, 'r');
+    await assert.rejects(
+      fh.read({ buffer: emptyBuffer, offset: 0, length: 0, position: 'string' }),
+      { code: 'ERR_INVALID_ARG_TYPE' }
+    );
+  } finally {
+    await fh?.close();
   }
 }

test/parallel/test-fs-readSync-position-validation.mjs

@@ -77,3 +77,21 @@ function testInvalid(code, position) {
     testInvalid('ERR_INVALID_ARG_TYPE', badTypeValue);
   }
 }
+
+{
+  const emptyBuffer = Buffer.alloc(0);
+  let fdSync;
+  try {
+    fdSync = fs.openSync(filepath, 'r');
+    assert.throws(
+      () => fs.readSync(fdSync, emptyBuffer, 0, 0, { not: 'a number' }),
+      { code: 'ERR_INVALID_ARG_TYPE' }
+    );
+    assert.throws(
+      () => fs.readSync(fdSync, emptyBuffer, { offset: 0, length: 0, position: 'string' }),
+      { code: 'ERR_INVALID_ARG_TYPE' }
+    );
+  } finally {
+    if (fdSync) fs.closeSync(fdSync);
+  }
+}