From b6207906c452be4c5f049a098fc645fdfb897306 Mon Sep 17 00:00:00 2001 From: Evan Lucas Date: Thu, 22 Oct 2015 06:58:26 -0500 Subject: [PATCH] fs: pass err to callback if buffer is too big In fs.readFile, if an encoding is specified and toString fails, do not throw an error. Instead, pass the error to the callback. Fixes: https://github.com/nodejs/node/issues/2767 PR-URL: https://github.com/nodejs/node/pull/3485 Reviewed-By: James M Snell Reviewed-By: Trevor Norris --- lib/fs.js | 18 +++++-- .../test-fs-readfile-tostring-fail.js | 52 +++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-fs-readfile-tostring-fail.js diff --git a/lib/fs.js b/lib/fs.js index 76873be14ab3de..6345497c4f2c48 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -395,12 +395,24 @@ function readFileAfterClose(err) { else buffer = context.buffer; - if (context.encoding) - buffer = buffer.toString(context.encoding); + if (err) return callback(err, buffer); - callback(err, buffer); + if (context.encoding) { + return tryToString(buffer, context.encoding, callback); + } + + callback(null, buffer); } +function tryToString(buf, encoding, callback) { + var e; + try { + buf = buf.toString(encoding); + } catch (err) { + e = err; + } + callback(e, buf); +} fs.readFileSync = function(path, options) { if (!options) { diff --git a/test/parallel/test-fs-readfile-tostring-fail.js b/test/parallel/test-fs-readfile-tostring-fail.js new file mode 100644 index 00000000000000..b5c4ea8f7a0b17 --- /dev/null +++ b/test/parallel/test-fs-readfile-tostring-fail.js @@ -0,0 +1,52 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); +const kStringMaxLength = process.binding('buffer').kStringMaxLength; + +common.refreshTmpDir(); + +const file = path.join(common.tmpDir, 'toobig.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() { + // make sure that the toString does not throw an error + fs.readFile(file, 'utf8', common.mustCall(function(err, buf) { + assert.ok(err instanceof Error); + assert.strictEqual('toString failed', err.message); + })); +})); + +function destroy() { + try { + 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; +});