Skip to content

Commit

Permalink
fs: execute mkdtemp's callback with no context
Browse files Browse the repository at this point in the history
All the callback functions in `fs` module are supposed to be executed
with no context (`this` value should not be a valid object). But
`mkdtemp`'s callback will have the `FSReqWrap` object as the context.

Sample code to reproduce the problem

    'use strict';

    const fs = require('fs');
    fs.mkdtemp('/tmp/abcd', null, function() {
      console.log(this);
    });

This would print

    FSReqWrap { oncomplete: [Function] }

But that should have printed `null` and this patch fixes that.

PR-URL: #7068
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
thefourtheye authored and evanlucas committed Jun 15, 2016
1 parent 2961f06 commit 1600966
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
4 changes: 2 additions & 2 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1590,8 +1590,7 @@ fs.realpath = function realpath(path, options, callback) {
};


fs.mkdtemp = function(prefix, options, callback_) {
var callback = maybeCallback(callback_);
fs.mkdtemp = function(prefix, options, callback) {
if (!prefix || typeof prefix !== 'string')
throw new TypeError('filename prefix is required');

Expand All @@ -1605,6 +1604,7 @@ fs.mkdtemp = function(prefix, options, callback_) {
if (typeof options !== 'object')
throw new TypeError('"options" must be a string or an object');

callback = makeCallback(callback);
if (!nullCheck(prefix, callback)) {
return;
}
Expand Down
21 changes: 14 additions & 7 deletions test/parallel/test-fs-mkdtemp.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,19 @@ assert.equal(Buffer.byteLength(path.basename(utf8)),
Buffer.byteLength('\u0222abc.XXXXXX'));
assert(common.fileExists(utf8));

fs.mkdtemp(
path.join(common.tmpDir, 'bar.'),
common.mustCall(function(err, folder) {
assert.ifError(err);
assert(common.fileExists(folder));
})
);
function handler(err, folder) {
assert.ifError(err);
assert(common.fileExists(folder));
assert.strictEqual(this, null);
}

fs.mkdtemp(path.join(common.tmpDir, 'bar.'), common.mustCall(handler));

// Same test as above, but making sure that passing an options object doesn't
// affect the way the callback function is handled.
fs.mkdtemp(path.join(common.tmpDir, 'bar.'), {}, common.mustCall(handler));

// Making sure that not passing a callback doesn't crash, as a default function
// is passed internally.
assert.doesNotThrow(() => fs.mkdtemp(path.join(common.tmpDir, 'bar-')));
assert.doesNotThrow(() => fs.mkdtemp(path.join(common.tmpDir, 'bar-'), {}));

0 comments on commit 1600966

Please sign in to comment.