diff --git a/lib/fs.js b/lib/fs.js index 6345497c4f2c48..42d02baa9e5d91 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -599,10 +599,13 @@ fs.read = function(fd, buffer, offset, length, position, callback) { callback = function(err, bytesRead) { if (!cb) return; + if (err) return cb(err); - var str = (bytesRead > 0) ? buffer.toString(encoding, 0, bytesRead) : ''; - - (cb)(err, str, bytesRead); + if (bytesRead > 0) { + tryToStringWithEnd(buffer, encoding, bytesRead, cb); + } else { + (cb)(err, '', bytesRead); + } }; } @@ -617,6 +620,16 @@ fs.read = function(fd, buffer, offset, length, position, callback) { binding.read(fd, buffer, offset, length, position, req); }; +function tryToStringWithEnd(buf, encoding, end, callback) { + var e; + try { + buf = buf.toString(encoding, 0, end); + } catch (err) { + e = err; + } + callback(e, buf, end); +} + fs.readSync = function(fd, buffer, offset, length, position) { var legacy = false; var encoding; diff --git a/test/parallel/test-fs-read-buffer-tostring-fail.js b/test/parallel/test-fs-read-buffer-tostring-fail.js new file mode 100644 index 00000000000000..3ec1491d2b4768 --- /dev/null +++ b/test/parallel/test-fs-read-buffer-tostring-fail.js @@ -0,0 +1,58 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const Buffer = require('buffer').Buffer; +const kStringMaxLength = process.binding('buffer').kStringMaxLength; +const kMaxLength = process.binding('buffer').kMaxLength; + +var fd; + +common.refreshTmpDir(); + +const file = path.join(common.tmpDir, 'toobig2.txt'); +const stream = fs.createWriteStream(file, { + flags: 'a' +}); + +const size = kStringMaxLength / 200; +const a = new Buffer(size).fill('a'); + +for (var i = 0; i < 201; i++) { + stream.write(a); +} + +stream.end(); +stream.on('finish', common.mustCall(function() { + fd = fs.openSync(file, 'r'); + fs.read(fd, kStringMaxLength + 1, 0, 'utf8', common.mustCall(function(err) { + assert.ok(err instanceof Error); + assert.strictEqual('toString failed', err.message); + })); +})); + +function destroy() { + try { + // Make sure we close fd and unlink the file + fs.closeSync(fd); + fs.unlinkSync(file); + } catch (err) { + // it may not exist + } +} + +process.on('exit', destroy); + +process.on('SIGINT', function() { + destroy(); + process.exit(); +}); + +// To make sure we don't leave a very large file +// on test machines in the event this test fails. +process.on('uncaughtException', function(err) { + destroy(); + throw err; +});