Skip to content

Commit

Permalink
fs: ensure readFile[Sync] reads from the beginning
Browse files Browse the repository at this point in the history
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: nodejs#9699
Fixes: nodejs#9671
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
  • Loading branch information
seishun authored and italoacasas committed Jan 18, 2017
1 parent 5fb91ed commit a500d30
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 5 deletions.
10 changes: 5 additions & 5 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ ReadFileContext.prototype.read = function() {
req.oncomplete = readFileAfterRead;
req.context = this;

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

ReadFileContext.prototype.close = function(err) {
Expand Down Expand Up @@ -447,11 +447,11 @@ function tryCreateBuffer(size, fd, isUserFd) {
return buffer;
}

function tryReadSync(fd, isUserFd, buffer, pos, len) {
function tryReadSync(fd, isUserFd, buffer, pos, len, offset) {
var threw = true;
var bytesRead;
try {
bytesRead = fs.readSync(fd, buffer, pos, len);
bytesRead = fs.readSync(fd, buffer, pos, len, offset);
threw = false;
} finally {
if (threw && !isUserFd) fs.closeSync(fd);
Expand Down Expand Up @@ -480,15 +480,15 @@ fs.readFileSync = function(path, options) {

if (size !== 0) {
do {
bytesRead = tryReadSync(fd, isUserFd, buffer, pos, size - pos);
bytesRead = tryReadSync(fd, isUserFd, buffer, pos, size - pos, pos);
pos += bytesRead;
} while (bytesRead !== 0 && pos < size);
} else {
do {
// the kernel lies about many files.
// Go ahead and try to read some bytes.
buffer = Buffer.allocUnsafe(8192);
bytesRead = tryReadSync(fd, isUserFd, buffer, 0, 8192);
bytesRead = tryReadSync(fd, isUserFd, buffer, 0, 8192, pos);
if (bytesRead !== 0) {
buffers.push(buffer.slice(0, bytesRead));
}
Expand Down
23 changes: 23 additions & 0 deletions test/parallel/test-fs-readfile-fd-offset.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const fs = require('fs');
const path = require('path');

const filename = path.join(common.tmpDir, 'readfile.txt');
const dataExpected = 'a'.repeat(100);
fs.writeFileSync(filename, dataExpected);
const fileLength = dataExpected.length;

['r', 'a+'].forEach((mode) => {
const fd = fs.openSync(filename, mode);
assert.strictEqual(fs.readFileSync(fd).length, fileLength);

// Reading again should result in the same length.
assert.strictEqual(fs.readFileSync(fd).length, fileLength);

fs.readFile(fd, common.mustCall((err, buf) => {
assert.ifError(err);
assert.strictEqual(buf.length, fileLength);
}));
});

0 comments on commit a500d30

Please sign in to comment.