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

geeksilva97 · aduh95 · commit 63c111cd608e · 2026-05-07T15:41:39.000+02:00
PR-URL: #62674 Reviewed-By: Aviv Keller <me@aviv.sh> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
diff --git a/lib/fs.js b/lib/fs.js
@@ -646,6 +646,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);
@@ -659,12 +665,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);
@@ -717,6 +717,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;
   }
@@ -728,12 +734,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);
 }
 
diff --git a/lib/internal/fs/promises.js b/lib/internal/fs/promises.js
@@ -678,6 +678,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 };
 
@@ -688,12 +694,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,
diff --git a/test/parallel/test-fs-read-position-validation.mjs b/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());
+      }
+    }));
+  });
+}
diff --git a/test/parallel/test-fs-read-promises-position-validation.mjs b/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();
   }
 }
diff --git a/test/parallel/test-fs-readSync-position-validation.mjs b/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);
+  }
+}
