From 1600966f599fd274195028329abba22c7e2e9952 Mon Sep 17 00:00:00 2001 From: Sakthipriyan Vairamani Date: Mon, 30 May 2016 23:15:01 +0530 Subject: [PATCH] fs: execute mkdtemp's callback with no context 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: https://github.com/nodejs/node/pull/7068 Reviewed-By: Anna Henningsen Reviewed-By: Colin Ihrig Reviewed-By: James M Snell --- lib/fs.js | 4 ++-- test/parallel/test-fs-mkdtemp.js | 21 ++++++++++++++------- 2 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/fs.js b/lib/fs.js index 7642b5cb74616f..58d3f8b18f1a59 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -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'); @@ -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; } diff --git a/test/parallel/test-fs-mkdtemp.js b/test/parallel/test-fs-mkdtemp.js index d3def97fef1588..dd4ab75c22c7aa 100644 --- a/test/parallel/test-fs-mkdtemp.js +++ b/test/parallel/test-fs-mkdtemp.js @@ -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-'), {}));