Skip to content

Commit

Permalink
fs: pass err to callback if buffer is too big
Browse files Browse the repository at this point in the history
In fs.readFile, if an encoding is specified and toString fails, do not
throw an error. Instead, pass the error to the callback.

Fixes: #2767
PR-URL: #3485
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Trevor Norris <[email protected]>
  • Loading branch information
evanlucas authored and rvagg committed Oct 26, 2015
1 parent 1a41feb commit 7added3
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 3 deletions.
18 changes: 15 additions & 3 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
52 changes: 52 additions & 0 deletions test/parallel/test-fs-readfile-tostring-fail.js
Original file line number Diff line number Diff line change
@@ -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;
});

0 comments on commit 7added3

Please sign in to comment.