From f3cf8e98081b52003bfe3c10f27f29c1c49e800a Mon Sep 17 00:00:00 2001 From: Brian White Date: Thu, 17 Nov 2016 22:03:39 -0500 Subject: [PATCH] fs: do not pass Buffer when toString() fails MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Even though an Error object is passed to the callback when readFile() fails due to toString() failing, it is a bit strange to still see data passed as the second argument. This commit changes that and only passes the Error object in that case. PR-URL: https://github.com/nodejs/node/pull/9670 Reviewed-By: Colin Ihrig Reviewed-By: Michaƫl Zasso Reviewed-By: Sakthipriyan Vairamani Reviewed-By: Prince John Wesley Reviewed-By: James M Snell --- lib/fs.js | 11 ++++------- test/parallel/test-fs-readfile-tostring-fail.js | 1 + 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 27614e5cf1febc..7aafc170d79739 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -396,8 +396,8 @@ function readFileAfterClose(err) { var buffer = null; var callback = context.callback; - if (context.err) - return callback(context.err); + if (context.err || err) + return callback(context.err || err); if (context.size === 0) buffer = Buffer.concat(context.buffers, context.pos); @@ -406,8 +406,6 @@ function readFileAfterClose(err) { else buffer = context.buffer; - if (err) return callback(err, buffer); - if (context.encoding) { return tryToString(buffer, context.encoding, callback); } @@ -416,13 +414,12 @@ function readFileAfterClose(err) { } function tryToString(buf, encoding, callback) { - var e = null; try { buf = buf.toString(encoding); } catch (err) { - e = err; + return callback(err); } - callback(e, buf); + callback(null, buf); } function tryStatSync(fd, isUserFd) { diff --git a/test/parallel/test-fs-readfile-tostring-fail.js b/test/parallel/test-fs-readfile-tostring-fail.js index 8ed9658a25889a..f3d728f57f7aeb 100644 --- a/test/parallel/test-fs-readfile-tostring-fail.js +++ b/test/parallel/test-fs-readfile-tostring-fail.js @@ -33,6 +33,7 @@ stream.on('finish', common.mustCall(function() { fs.readFile(file, 'utf8', common.mustCall(function(err, buf) { assert.ok(err instanceof Error); assert.strictEqual('"toString()" failed', err.message); + assert.strictEqual(buf, undefined); })); }));