Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
fs: fs.readFile should not throw uncaughtException
Browse files Browse the repository at this point in the history
Reviewed-By: Fedor Indutny <fedor@indutny.com>
  • Loading branch information
JacksonTian authored and indutny committed Aug 5, 2014
1 parent e643fe4 commit 63a4268
Show file tree
Hide file tree
Showing 2 changed files with 67 additions and 4 deletions.
11 changes: 7 additions & 4 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,13 @@ fs.readFile = function(path, options, callback_) {
return read();
}

if (size > kMaxLength)
throw new RangeError('File size is greater than possible Buffer: ' +
'0x3FFFFFFF bytes');

if (size > kMaxLength) {
var err = new RangeError('File size is greater than possible Buffer: ' +
'0x3FFFFFFF bytes');
return fs.close(fd, function() {

This comment has been minimized.

Copy link
@ronkorving

ronkorving Aug 6, 2014

Oh wow, so this was actually leaking file descriptors too?

This comment has been minimized.

Copy link
@JacksonTian

JacksonTian Aug 6, 2014

Author

yes. if uncauthException was threw, the fd was leak too.

callback(err);
});
}
buffer = new Buffer(size);
read();
});
Expand Down
60 changes: 60 additions & 0 deletions test/pummel/test-fs-readfile-large.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Copyright Joyent, Inc. and other Node contributors.
//
// Permission is hereby granted, free of charge, to any person obtaining a
// copy of this software and associated documentation files (the
// "Software"), to deal in the Software without restriction, including
// without limitation the rights to use, copy, modify, merge, publish,
// distribute, sublicense, and/or sell copies of the Software, and to permit
// persons to whom the Software is furnished to do so, subject to the
// following conditions:
//
// The above copyright notice and this permission notice shall be included
// in all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN
// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
// USE OR OTHER DEALINGS IN THE SOFTWARE.

var common = require('../common');
var assert = require('assert');

var path = require('path'),
fs = require('fs'),
filename = path.join(common.fixturesDir, 'large_file.txt');

var filesize = 1024 * 1024 * 1024;

function makeFile(done) {
var buf = new Buffer(filesize / 1024);
buf.fill('a');

try { fs.unlinkSync(filename); } catch (e) {}
var w = 1024;
var ws = fs.createWriteStream(filename);
ws.on('close', done);
ws.on('drain', write);
write();
function write() {
do {
w--;
} while (false !== ws.write(buf) && w > 0);
if (w === 0)
ws.end();
}
}

makeFile(function () {
fs.readFile(filename, function(err) {
assert.ok(err, 'should get RangeError');
assert.equal(err.name, 'RangeError', 'should get RangeError');
try { fs.unlinkSync(filename); } catch (e) {}
});
});

process.on('uncaughtException', function (err) {
assert.ok(!err, 'should not throw uncaughtException');
});

0 comments on commit 63a4268

Please sign in to comment.