Skip to content

Commit 4444e73

Browse files
authored
fs: ensure readFile[Sync] reads from the beginning
It would previously read from the current file position, which can be non-zero if the `fd` has been read from or written to. This contradicts the documentation which states that it "reads the entire contents of a file". PR-URL: #9699 Fixes: #9671 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Sam Roberts <[email protected]>
1 parent f18b463 commit 4444e73

File tree

2 files changed

+28
-5
lines changed

2 files changed

+28
-5
lines changed

lib/fs.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,7 @@ ReadFileContext.prototype.read = function() {
310310
req.oncomplete = readFileAfterRead;
311311
req.context = this;
312312

313-
binding.read(this.fd, buffer, offset, length, -1, req);
313+
binding.read(this.fd, buffer, offset, length, this.pos, req);
314314
};
315315

316316
ReadFileContext.prototype.close = function(err) {
@@ -447,11 +447,11 @@ function tryCreateBuffer(size, fd, isUserFd) {
447447
return buffer;
448448
}
449449

450-
function tryReadSync(fd, isUserFd, buffer, pos, len) {
450+
function tryReadSync(fd, isUserFd, buffer, pos, len, offset) {
451451
var threw = true;
452452
var bytesRead;
453453
try {
454-
bytesRead = fs.readSync(fd, buffer, pos, len);
454+
bytesRead = fs.readSync(fd, buffer, pos, len, offset);
455455
threw = false;
456456
} finally {
457457
if (threw && !isUserFd) fs.closeSync(fd);
@@ -480,15 +480,15 @@ fs.readFileSync = function(path, options) {
480480

481481
if (size !== 0) {
482482
do {
483-
bytesRead = tryReadSync(fd, isUserFd, buffer, pos, size - pos);
483+
bytesRead = tryReadSync(fd, isUserFd, buffer, pos, size - pos, pos);
484484
pos += bytesRead;
485485
} while (bytesRead !== 0 && pos < size);
486486
} else {
487487
do {
488488
// the kernel lies about many files.
489489
// Go ahead and try to read some bytes.
490490
buffer = Buffer.allocUnsafe(8192);
491-
bytesRead = tryReadSync(fd, isUserFd, buffer, 0, 8192);
491+
bytesRead = tryReadSync(fd, isUserFd, buffer, 0, 8192, pos);
492492
if (bytesRead !== 0) {
493493
buffers.push(buffer.slice(0, bytesRead));
494494
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
'use strict';
2+
const common = require('../common');
3+
const assert = require('assert');
4+
const fs = require('fs');
5+
const path = require('path');
6+
7+
const filename = path.join(common.tmpDir, 'readfile.txt');
8+
const dataExpected = 'a'.repeat(100);
9+
fs.writeFileSync(filename, dataExpected);
10+
const fileLength = dataExpected.length;
11+
12+
['r', 'a+'].forEach((mode) => {
13+
const fd = fs.openSync(filename, mode);
14+
assert.strictEqual(fs.readFileSync(fd).length, fileLength);
15+
16+
// Reading again should result in the same length.
17+
assert.strictEqual(fs.readFileSync(fd).length, fileLength);
18+
19+
fs.readFile(fd, common.mustCall((err, buf) => {
20+
assert.ifError(err);
21+
assert.strictEqual(buf.length, fileLength);
22+
}));
23+
});

0 commit comments

Comments
 (0)