Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fs: properly handle fd passed to truncate() #866

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion doc/api/fs.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,8 @@ Synchronous ftruncate(2).
## fs.truncate(path, len, callback)

Asynchronous truncate(2). No arguments other than a possible exception are
given to the completion callback.
given to the completion callback. A file descriptor can also be passed as the
first argument. In this case, `fs.ftruncate()` is called.

## fs.truncateSync(path, len)

Expand Down
10 changes: 6 additions & 4 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,14 @@ function maybeCallback(cb) {
// for callbacks that are passed to the binding layer, callbacks that are
// invoked from JS already run in the proper scope.
function makeCallback(cb) {
if (typeof cb !== 'function') {
if (cb === undefined) {
return rethrow();
}

if (typeof cb !== 'function') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the point of an extra check? this function is internal anyway never mind

throw new TypeError('callback must be a function');
}

return function() {
return cb.apply(null, arguments);
};
Expand Down Expand Up @@ -679,9 +683,7 @@ fs.renameSync = function(oldPath, newPath) {

fs.truncate = function(path, len, callback) {
if (typeof path === 'number') {
var req = new FSReqWrap();
req.oncomplete = callback;
return fs.ftruncate(path, len, req);
return fs.ftruncate(path, len, callback);
}
if (typeof len === 'function') {
callback = len;
Expand Down
27 changes: 27 additions & 0 deletions test/parallel/test-fs-make-callback.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
var common = require('../common');
var assert = require('assert');
var fs = require('fs');

function test(cb) {
return function() {
// fs.stat() calls makeCallback() on its second argument
fs.stat(__filename, cb);
};
}

// Verify the case where a callback function is provided
assert.doesNotThrow(test(function() {}));

// Passing undefined calls rethrow() internally, which is fine
assert.doesNotThrow(test(undefined));

// Anything else should throw
assert.throws(test(null));
assert.throws(test(true));
assert.throws(test(false));
assert.throws(test(1));
assert.throws(test(0));
assert.throws(test('foo'));
assert.throws(test(/foo/));
assert.throws(test([]));
assert.throws(test({}));
25 changes: 25 additions & 0 deletions test/parallel/test-fs-truncate-fd.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
var common = require('../common');
var assert = require('assert');
var path = require('path');
var fs = require('fs');
var tmp = common.tmpDir;
if (!fs.existsSync(tmp))
fs.mkdirSync(tmp);
var filename = path.resolve(tmp, 'truncate-file.txt');

var success = 0;

fs.writeFileSync(filename, 'hello world', 'utf8');
var fd = fs.openSync(filename, 'r+');
fs.truncate(fd, 5, function(err) {
assert.ok(!err);
assert.equal(fs.readFileSync(filename, 'utf8'), 'hello');
success++;
});

process.on('exit', function() {
fs.closeSync(fd);
fs.unlinkSync(filename);
assert.equal(success, 1);
console.log('ok');
});