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

@@ -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){
 returnprocess.nextTick(functiontick(){
 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);
-}
-
 functionwrapper(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){
 return0;
 }
@@ -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);
-}
-
 returnbinding.read(fd,buffer,offset,length,position);
 }
 

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);
-}
-
 constbytesRead=(awaitPromisePrototypeThen(
 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) {
 awaittestInvalid('ERR_INVALID_ARG_TYPE',badTypeValue);
 }
 }
+
+{
+constemptyBuffer=Buffer.alloc(0);
+awaitnewPromise((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(constbadTypeValueof[
 false,true,'1',Symbol(1),{},[],()=>{},Promise.resolve(1),
 ]){
-testInvalid('ERR_INVALID_ARG_TYPE',badTypeValue);
+awaittestInvalid('ERR_INVALID_ARG_TYPE',badTypeValue);
+}
+}
+
+{
+constemptyBuffer=Buffer.alloc(0);
+letfh;
+try{
+fh=awaitfs.promises.open(filepath,'r');
+awaitassert.rejects(
+fh.read(emptyBuffer,0,0,{not: 'a number'}),
+{code: 'ERR_INVALID_ARG_TYPE'}
+);
+}finally{
+awaitfh?.close();
+}
+
+try{
+fh=awaitfs.promises.open(filepath,'r');
+awaitassert.rejects(
+fh.read({buffer: emptyBuffer,offset: 0,length: 0,position: 'string'}),
+{code: 'ERR_INVALID_ARG_TYPE'}
+);
+}finally{
+awaitfh?.close();
 }
 }

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

@@ -77,3 +77,21 @@ function testInvalid(code, position) {
 testInvalid('ERR_INVALID_ARG_TYPE',badTypeValue);
 }
 }
+
+{
+constemptyBuffer=Buffer.alloc(0);
+letfdSync;
+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);
+}
+}