Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 63c111c

Browse filesBrowse 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
Copy full SHA for 63c111c

5 files changed

+84-19Lines changed: 84 additions & 19 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/fs.js‎

Copy file name to clipboardExpand all lines: lib/fs.js
+12-12Lines 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

Collapse file

‎lib/internal/fs/promises.js‎

Copy file name to clipboardExpand all lines: lib/internal/fs/promises.js
+6-6Lines 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,
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-fs-read-position-validation.mjs
+23Lines 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+
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-fs-read-promises-position-validation.mjs
+25-1Lines 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
}
Collapse file

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

Copy file name to clipboardExpand all lines: test/parallel/test-fs-readSync-position-validation.mjs
+18Lines 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 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.