◐ Shell
reader mode source ↗
Skip to content

Commit 63c111c

Browse files
geeksilva97aduh95
geeksilva97
authored and
aduh95
committed
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>
1 parent 6cd368b commit 63c111c

5 files changed

Lines changed: 84 additions & 19 deletions

File tree

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -646,6 +646,12 @@ function read(fd, buffer, offsetOrOptions, length, position, callback) {
646646

647647
length |= 0;
648648

649+
if (position == null) {
650+
position = -1;
651+
} else {
652+
validatePosition(position, 'position', length);
653+
}
654+
649655
if (length === 0) {
650656
return process.nextTick(function tick() {
651657
callback(null, 0, buffer);
@@ -659,12 +665,6 @@ function read(fd, buffer, offsetOrOptions, length, position, callback) {
659665

660666
validateOffsetLengthRead(offset, length, buffer.byteLength);
661667

662-
if (position == null) {
663-
position = -1;
664-
} else {
665-
validatePosition(position, 'position', length);
666-
}
667-
668668
function wrapper(err, bytesRead) {
669669
// Retain a reference to buffer so that it can't be GC'ed too soon.
670670
callback(err, bytesRead || 0, buffer);
@@ -717,6 +717,12 @@ function readSync(fd, buffer, offsetOrOptions, length, position) {
717717

718718
length |= 0;
719719

720+
if (position == null) {
721+
position = -1;
722+
} else {
723+
validatePosition(position, 'position', length);
724+
}
725+
720726
if (length === 0) {
721727
return 0;
722728
}
@@ -728,12 +734,6 @@ function readSync(fd, buffer, offsetOrOptions, length, position) {
728734

729735
validateOffsetLengthRead(offset, length, buffer.byteLength);
730736

731-
if (position == null) {
732-
position = -1;
733-
} else {
734-
validatePosition(position, 'position', length);
735-
}
736-
737737
return binding.read(fd, buffer, offset, length, position);
738738
}
739739

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -678,6 +678,12 @@ async function read(handle, bufferOrParams, offset, length, position) {
678678

679679
length ??= buffer.byteLength - offset;
680680

681+
if (position == null) {
682+
position = -1;
683+
} else {
684+
validatePosition(position, 'position', length);
685+
}
686+
681687
if (length === 0)
682688
return { __proto__: null, bytesRead: length, buffer };
683689

@@ -688,12 +694,6 @@ async function read(handle, bufferOrParams, offset, length, position) {
688694

689695
validateOffsetLengthRead(offset, length, buffer.byteLength);
690696

691-
if (position == null) {
692-
position = -1;
693-
} else {
694-
validatePosition(position, 'position', length);
695-
}
696-
697697
const bytesRead = (await PromisePrototypeThen(
698698
binding.read(handle.fd, buffer, offset, length, position, kUsePromises),
699699
undefined,
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,3 +91,26 @@ async function testInvalid(code, position) {
9191
await testInvalid('ERR_INVALID_ARG_TYPE', badTypeValue);
9292
}
9393
}
94+
95+
{
96+
const emptyBuffer = Buffer.alloc(0);
97+
await new Promise((resolve, reject) => {
98+
fs.open(filepath, 'r', common.mustSucceed((fd) => {
99+
try {
100+
assert.throws(
101+
() => fs.read(fd, emptyBuffer, 0, 0, { not: 'a number' }, common.mustNotCall()),
102+
{ code: 'ERR_INVALID_ARG_TYPE' }
103+
);
104+
assert.throws(
105+
() => fs.read(fd, { buffer: emptyBuffer, offset: 0, length: 0, position: 'string' }, common.mustNotCall()),
106+
{ code: 'ERR_INVALID_ARG_TYPE' }
107+
);
108+
resolve();
109+
} catch (err) {
110+
reject(err);
111+
} finally {
112+
fs.close(fd, common.mustSucceed());
113+
}
114+
}));
115+
});
116+
}
Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,30 @@ async function testInvalid(code, position) {
7979
for (const badTypeValue of [
8080
false, true, '1', Symbol(1), {}, [], () => {}, Promise.resolve(1),
8181
]) {
82-
testInvalid('ERR_INVALID_ARG_TYPE', badTypeValue);
82+
await testInvalid('ERR_INVALID_ARG_TYPE', badTypeValue);
83+
}
84+
}
85+
86+
{
87+
const emptyBuffer = Buffer.alloc(0);
88+
let fh;
89+
try {
90+
fh = await fs.promises.open(filepath, 'r');
91+
await assert.rejects(
92+
fh.read(emptyBuffer, 0, 0, { not: 'a number' }),
93+
{ code: 'ERR_INVALID_ARG_TYPE' }
94+
);
95+
} finally {
96+
await fh?.close();
97+
}
98+
99+
try {
100+
fh = await fs.promises.open(filepath, 'r');
101+
await assert.rejects(
102+
fh.read({ buffer: emptyBuffer, offset: 0, length: 0, position: 'string' }),
103+
{ code: 'ERR_INVALID_ARG_TYPE' }
104+
);
105+
} finally {
106+
await fh?.close();
83107
}
84108
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,3 +77,21 @@ function testInvalid(code, position) {
7777
testInvalid('ERR_INVALID_ARG_TYPE', badTypeValue);
7878
}
7979
}
80+
81+
{
82+
const emptyBuffer = Buffer.alloc(0);
83+
let fdSync;
84+
try {
85+
fdSync = fs.openSync(filepath, 'r');
86+
assert.throws(
87+
() => fs.readSync(fdSync, emptyBuffer, 0, 0, { not: 'a number' }),
88+
{ code: 'ERR_INVALID_ARG_TYPE' }
89+
);
90+
assert.throws(
91+
() => fs.readSync(fdSync, emptyBuffer, { offset: 0, length: 0, position: 'string' }),
92+
{ code: 'ERR_INVALID_ARG_TYPE' }
93+
);
94+
} finally {
95+
if (fdSync) fs.closeSync(fdSync);
96+
}
97+
}

0 commit comments

Comments
 (0)