Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

Commit

Permalink
fix: fs.readSync with large position value
Browse files Browse the repository at this point in the history
Backport of nodejs/node#21003; fixed after 10.3.0 so this does not also need to be applied to the 10.6.0 upgrade
  • Loading branch information
codebytere committed Aug 15, 2018
1 parent ed9e26b commit c86e3d3
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 15 deletions.
4 changes: 2 additions & 2 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ fs.read = function(fd, buffer, offset, length, position, callback) {

validateOffsetLengthRead(offset, length, buffer.length);

if (!isUint32(position))
if (!Number.isSafeInteger(position))
position = -1;

function wrapper(err, bytesRead) {
Expand Down Expand Up @@ -623,7 +623,7 @@ fs.readSync = function(fd, buffer, offset, length, position) {

validateOffsetLengthRead(offset, length, buffer.length);

if (!isUint32(position))
if (!Number.isSafeInteger(position))
position = -1;

const ctx = {};
Expand Down
41 changes: 28 additions & 13 deletions test/parallel/test-fs-read.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,40 @@ const expected = Buffer.from('xyz\n');

function test(bufferAsync, bufferSync, expected) {
fs.read(fd,
bufferAsync,
0,
expected.length,
0,
common.mustCall((err, bytesRead) => {
assert.ifError(err);
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(bufferAsync, expected);
}));
bufferAsync,
0,
expected.length,
0,
common.mustCall((err, bytesRead) => {
assert.ifError(err);
assert.strictEqual(bytesRead, expected.length);
assert.deepStrictEqual(bufferAsync, expected);
}));

const r = fs.readSync(fd, bufferSync, 0, expected.length, 0);
assert.deepStrictEqual(bufferSync, expected);
assert.strictEqual(r, expected.length);
}

test(Buffer.allocUnsafe(expected.length),
Buffer.allocUnsafe(expected.length),
expected);
Buffer.allocUnsafe(expected.length),
expected);

test(new Uint8Array(expected.length),
new Uint8Array(expected.length),
Uint8Array.from(expected));
new Uint8Array(expected.length),
Uint8Array.from(expected));

{
// Reading beyond file length (3 in this case) should return no data.
// This is a test for a bug where reads > uint32 would return data
// from the current position in the file.
const fd = fs.openSync(filepath, 'r');
const pos = 0xffffffff + 1; // max-uint32 + 1
const nRead = fs.readSync(fd, Buffer.alloc(1), 0, 1, pos);
assert.strictEqual(nRead, 0);

fs.read(fd, Buffer.alloc(1), 0, 1, pos, common.mustCall((err, nRead) => {
assert.ifError(err);
assert.strictEqual(nRead, 0);
}));
}

0 comments on commit c86e3d3

Please sign in to comment.