Skip to content

Commit

Permalink
fs: fix promises reads with pos > 4GB
Browse files Browse the repository at this point in the history
PR-URL: nodejs#21148
Fixes: nodejs#21121
Refs: nodejs#21003
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
  • Loading branch information
cjihrig committed Jun 7, 2018
1 parent c9d9bf1 commit 5012587
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
3 changes: 1 addition & 2 deletions lib/internal/fs/promises.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const {
validatePath
} = require('internal/fs/utils');
const {
isUint32,
validateMode,
validateInteger,
validateUint32
Expand Down Expand Up @@ -211,7 +210,7 @@ async function read(handle, buffer, offset, length, position) {

validateOffsetLengthRead(offset, length, buffer.length);

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

const bytesRead = (await binding.read(handle.fd, buffer, offset, length,
Expand Down
14 changes: 14 additions & 0 deletions test/parallel/test-fs-promises-file-handle-read.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const common = require('../common');
const fs = require('fs');
const { open } = fs.promises;
const path = require('path');
const fixtures = require('../common/fixtures');
const tmpdir = require('../common/tmpdir');
const assert = require('assert');
const tmpDir = tmpdir.path;
Expand Down Expand Up @@ -40,6 +41,19 @@ async function validateEmptyRead() {
assert.deepStrictEqual(buffer.length, readAsyncHandle.bytesRead);
}

async function validateLargeRead() {
// 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 filePath = fixtures.path('x.txt');
const fileHandle = await open(filePath, 'r');
const pos = 0xffffffff + 1; // max-uint32 + 1
const readHandle = await fileHandle.read(Buffer.alloc(1), 0, 1, pos);

assert.strictEqual(readHandle.bytesRead, 0);
}

validateRead()
.then(validateEmptyRead)
.then(validateLargeRead)
.then(common.mustCall());

0 comments on commit 5012587

Please sign in to comment.