From 4444e731f218edf265a0b160bf1d561df2d5e5b3 Mon Sep 17 00:00:00 2001 From: Nikolai Vavilov Date: Sat, 14 Jan 2017 20:33:18 +0200 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/9699 Fixes: https://github.com/nodejs/node/issues/9671 Reviewed-By: Colin Ihrig Reviewed-By: Ben Noordhuis Reviewed-By: Sam Roberts --- lib/fs.js | 10 ++++----- test/parallel/test-fs-readfile-fd-offset.js | 23 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) create mode 100644 test/parallel/test-fs-readfile-fd-offset.js diff --git a/lib/fs.js b/lib/fs.js index 6362fab54a1c98..b6af9e25539b69 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -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) { @@ -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); @@ -480,7 +480,7 @@ 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 { @@ -488,7 +488,7 @@ fs.readFileSync = function(path, options) { // 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)); } diff --git a/test/parallel/test-fs-readfile-fd-offset.js b/test/parallel/test-fs-readfile-fd-offset.js new file mode 100644 index 00000000000000..0a02685319369e --- /dev/null +++ b/test/parallel/test-fs-readfile-fd-offset.js @@ -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); + })); +});