Skip to content

Commit 48b9f72

Browse files
silkentrancesilkentrance
committed
fix gh-176: fail early if there is no tmp dir specified, add rimraf dependency that somehow got lost
1 parent 150ab4e commit 48b9f72

File tree

5 files changed

+211
-36
lines changed

5 files changed

+211
-36
lines changed

‎lib/tmp.js‎

Lines changed: 68 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,6 @@ const rimraf = require('rimraf');
2222
* The working inner variables.
2323
*/
2424
const
25-
/**
26-
* The temporary directory.
27-
* @type {string}
28-
*/
29-
tmpDir = os.tmpdir(),
30-
3125
// the random characters to choose from
3226
RANDOM_CHARS = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz',
3327

@@ -123,12 +117,21 @@ function _parseArguments(options, callback) {
123117
* @private
124118
*/
125119
function _generateTmpName(opts) {
120+
121+
const tmpDir = _getTmpDir();
122+
123+
// fail early on missing tmp dir
124+
if (isBlank(opts.dir) && isBlank(tmpDir)) {
125+
throw new Error('No tmp dir specified');
126+
}
127+
126128
/* istanbul ignore else */
127-
if (opts.name) {
129+
if (!isBlank(opts.name)) {
128130
return path.join(opts.dir || tmpDir, opts.name);
129131
}
130132

131133
// mkstemps like template
134+
// opts.template has already been guarded in tmpName() below
132135
/* istanbul ignore else */
133136
if (opts.template) {
134137
var template = opts.template;
@@ -141,10 +144,10 @@ function _generateTmpName(opts) {
141144

142145
// prefix and postfix
143146
const name = [
144-
opts.prefix || 'tmp-',
147+
(isBlank(opts.prefix) ? 'tmp-' : opts.prefix),
145148
process.pid,
146149
_randomChars(12),
147-
opts.postfix || ''
150+
(isBlank(opts.postfix) ? '' : opts.postfix)
148151
].join('');
149152

150153
return path.join(opts.dir || tmpDir, name);
@@ -161,7 +164,7 @@ function tmpName(options, callback) {
161164
args = _parseArguments(options, callback),
162165
opts = args[0],
163166
cb = args[1],
164-
tries = opts.name ? 1 : opts.tries || DEFAULT_TRIES;
167+
tries = !isBlank(opts.name) ? 1 : opts.tries || DEFAULT_TRIES;
165168

166169
/* istanbul ignore else */
167170
if (isNaN(tries) || tries < 0)
@@ -172,20 +175,24 @@ function tmpName(options, callback) {
172175
return cb(new Error('Invalid template provided'));
173176

174177
(function _getUniqueName() {
175-
const name = _generateTmpName(opts);
178+
try {
179+
const name = _generateTmpName(opts);
176180

177-
// check whether the path exists then retry if needed
178-
fs.stat(name, function (err) {
179-
/* istanbul ignore else */
180-
if (!err) {
181+
// check whether the path exists then retry if needed
182+
fs.stat(name, function (err) {
181183
/* istanbul ignore else */
182-
if (tries-- > 0) return _getUniqueName();
184+
if (!err) {
185+
/* istanbul ignore else */
186+
if (tries-- > 0) return _getUniqueName();
183187

184-
return cb(new Error('Could not get a unique tmp filename, max tries reached ' + name));
185-
}
188+
return cb(new Error('Could not get a unique tmp filename, max tries reached ' + name));
189+
}
186190

187-
cb(null, name);
188-
});
191+
cb(null, name);
192+
});
193+
} catch (err) {
194+
cb(err);
195+
}
189196
}());
190197
}
191198

@@ -200,7 +207,7 @@ function tmpNameSync(options) {
200207
var
201208
args = _parseArguments(options),
202209
opts = args[0],
203-
tries = opts.name ? 1 : opts.tries || DEFAULT_TRIES;
210+
tries = !isBlank(opts.name) ? 1 : opts.tries || DEFAULT_TRIES;
204211

205212
/* istanbul ignore else */
206213
if (isNaN(tries) || tries < 0)
@@ -234,7 +241,7 @@ function file(options, callback) {
234241
opts = args[0],
235242
cb = args[1];
236243

237-
opts.postfix = (_isUndefined(opts.postfix)) ? '.tmp' : opts.postfix;
244+
opts.postfix = isBlank(opts.postfix) ? '.tmp' : opts.postfix;
238245

239246
// gets a temporary filename
240247
tmpName(opts, function _tmpNameCreated(err, name) {
@@ -287,7 +294,7 @@ function fileSync(options) {
287294
args = _parseArguments(options),
288295
opts = args[0];
289296

290-
opts.postfix = (_isUndefined(opts.postfix)) ? '.tmp' : opts.postfix;
297+
opts.postfix = isBlank(opts.postfix) ? '.tmp' : opts.postfix;
291298

292299
const discardOrDetachDescriptor = opts.discardDescriptor || opts.detachDescriptor;
293300
const name = tmpNameSync(opts);
@@ -536,32 +543,53 @@ function isENOENT(error) {
536543
* which will differ between the supported node versions.
537544
*
538545
* - Node >= 7.0:
539-
* error.code {String}
540-
* error.errno {String|Number} any numerical value will be negated
546+
* error.code {string}
547+
* error.errno {string|number} any numerical value will be negated
541548
*
542549
* - Node >= 6.0 < 7.0:
543-
* error.code {String}
544-
* error.errno {Number} negated
550+
* error.code {string}
551+
* error.errno {number} negated
545552
*
546553
* - Node >= 4.0 < 6.0: introduces SystemError
547-
* error.code {String}
548-
* error.errno {Number} negated
554+
* error.code {string}
555+
* error.errno {number} negated
549556
*
550557
* - Node >= 0.10 < 4.0:
551-
* error.code {Number} negated
558+
* error.code {number} negated
552559
* error.errno n/a
553560
*/
554561
function isExpectedError(error, code, errno) {
555562
return error.code === code || error.code === errno;
556563
}
557564

565+
/**
566+
* Helper which determines whether a string s is blank, that is undefined, or empty or null.
567+
*
568+
* @private
569+
* @param {string} s
570+
* @returns {Boolean} true whether the string s is blank, false otherwise
571+
*/
572+
function isBlank(s) {
573+
return s === null || s === undefined || !s.trim();
574+
}
575+
558576
/**
559577
* Sets the graceful cleanup.
560578
*/
561579
function setGracefulCleanup() {
562580
_gracefulCleanup = true;
563581
}
564582

583+
/**
584+
* Returns the currently configured tmp dir from os.tmpdir().
585+
*
586+
* @private
587+
* @returns {string} the currently configured tmp dir
588+
*/
589+
function _getTmpDir() {
590+
return os.tmpdir();
591+
}
592+
565593
/**
566594
* If there are multiple different versions of tmp in place, make sure that
567595
* we recognize the old listeners.
@@ -715,7 +743,16 @@ _safely_install_sigint_listener();
715743
*/
716744

717745
// exporting all the needed methods
718-
module.exports.tmpdir = tmpDir;
746+
747+
// evaluate os.tmpdir() lazily, mainly for simplifying testing but it also will
748+
// allow users to reconfigure the temporary directory
749+
Object.defineProperty(module.exports, 'tmpdir', {
750+
enumerable: true,
751+
configurable: false,
752+
get: function () {
753+
return _getTmpDir();
754+
}
755+
});
719756

720757
module.exports.dir = dir;
721758
module.exports.dirSync = dirSync;

‎package.json‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,8 @@
2222
"node": ">=6"
2323
},
2424
"dependencies": {
25-
"rimraf": "^2.6.2"
25+
"rimraf": "^2.6.3"
2626
},
27-
"dependencies": {},
2827
"devDependencies": {
2928
"eslint": "^4.19.1",
3029
"eslint-plugin-mocha": "^5.0.0",

‎test/dir-test.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ describe('tmp', function () {
5454
});
5555

5656
describe('when running issue specific inband tests', function () {
57-
// add your issue specific tests here
5857
});
5958

6059
describe('when running standard outband tests', function () {

‎test/name-sync-test.js‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
/* eslint-disable no-octal */
22
// vim: expandtab:ts=2:sw=2
33

4-
var
4+
const
55
assert = require('assert'),
6+
os = require('os'),
67
inbandStandardTests = require('./name-inband-standard'),
78
tmp = require('../lib/tmp');
89

@@ -35,6 +36,71 @@ describe('tmp', function () {
3536
});
3637

3738
describe('when running issue specific inband tests', function () {
39+
describe('on issue #176', function () {
40+
const origfn = os.tmpdir;
41+
42+
function _generateSpecName(optsDir, osTmpDir) {
43+
return 'opts.dir = "$1", os.tmpdir() = "$2"'.replace('$1', optsDir).replace('$2', osTmpDir);
44+
}
45+
46+
const failing = ['', ' ', undefined, null];
47+
const nonFailing = ['tmp']; // the origfn cannot be trusted as the os may or may not have a valid tmp dir
48+
49+
describe('must fail on invalid os.tmpdir() and invalid opts.dir', function () {
50+
// test all failing permutations
51+
for (let oidx = 0; oidx < failing.length; oidx++) {
52+
for (let iidx = 0; iidx < failing.length; iidx++) {
53+
it(_generateSpecName(failing[iidx], failing[oidx]), function () {
54+
os.tmpdir = function () { return failing[oidx]; };
55+
try {
56+
tmp.tmpNameSync({ dir: failing[iidx] });
57+
assert.fail('expected this to fail');
58+
} catch (err) {
59+
assert.ok(err instanceof Error, 'error expected');
60+
} finally {
61+
os.tmpdir = origfn;
62+
}
63+
});
64+
}
65+
}
66+
});
67+
68+
describe('must not fail on invalid os.tmpdir() and valid opts.dir', function () {
69+
// test all non failing permutations for non failing opts.dir and failing osTmpDir
70+
for (let oidx = 0; oidx < failing.length; oidx++) {
71+
for (let iidx = 0; iidx < nonFailing.length; iidx++) {
72+
it(_generateSpecName(nonFailing[iidx], failing[oidx]), function () {
73+
os.tmpdir = function () { return failing[oidx]; };
74+
try {
75+
tmp.tmpNameSync({ dir: nonFailing[iidx] });
76+
} catch (err) {
77+
assert.fail(err);
78+
} finally {
79+
os.tmpdir = origfn;
80+
}
81+
});
82+
}
83+
}
84+
});
85+
86+
describe('must not fail on valid os.tmpdir() and invalid opts.dir', function () {
87+
// test all non failing permutations for failing opts.dir and non failing osTmpDir
88+
for (let oidx = 0; oidx < nonFailing.length; oidx++) {
89+
for (let iidx = 0; iidx < failing.length; iidx++) {
90+
it(_generateSpecName(failing[iidx], nonFailing[oidx]), function () {
91+
os.tmpdir = function () { return nonFailing[oidx]; };
92+
try {
93+
tmp.tmpNameSync({ dir: failing[iidx] });
94+
} catch (err) {
95+
assert.fail(err);
96+
} finally {
97+
os.tmpdir = origfn;
98+
}
99+
});
100+
}
101+
}
102+
});
103+
});
38104
});
39105

40106
describe('when running standard outband tests', function () {

‎test/name-test.js‎

Lines changed: 75 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
/* eslint-disable no-octal */
22
// vim: expandtab:ts=2:sw=2
33

4-
var
4+
const
55
assert = require('assert'),
6+
os = require('os'),
67
inbandStandardTests = require('./name-inband-standard'),
78
tmp = require('../lib/tmp');
89

@@ -45,6 +46,79 @@ describe('tmp', function () {
4546
});
4647

4748
describe('when running issue specific inband tests', function () {
49+
describe('on issue #176', function () {
50+
const origfn = os.tmpdir;
51+
52+
function _generateSpecName(optsDir, osTmpDir) {
53+
return 'opts.dir = "$1", os.tmpdir() = "$2"'.replace('$1', optsDir).replace('$2', osTmpDir);
54+
}
55+
56+
const failing = ['', ' ', undefined, null];
57+
const nonFailing = ['tmp']; // the origfn cannot be trusted as the os may or may not have a valid tmp dir
58+
59+
describe('must fail on invalid os.tmpdir() and invalid opts.dir', function () {
60+
// test all failing permutations
61+
for (let oidx = 0; oidx < failing.length; oidx++) {
62+
for (let iidx = 0; iidx < failing.length; iidx++) {
63+
it(_generateSpecName(failing[iidx], failing[oidx]), function (done) {
64+
os.tmpdir = function () { return failing[oidx]; };
65+
tmp.tmpName({ dir: failing[iidx] }, function (err) {
66+
try {
67+
assert.ok(err instanceof Error, 'should have failed');
68+
} catch (err) {
69+
return done(err);
70+
} finally {
71+
os.tmpdir = origfn;
72+
}
73+
done();
74+
});
75+
});
76+
}
77+
}
78+
});
79+
80+
describe('must not fail on invalid os.tmpdir() and valid opts.dir', function () {
81+
// test all non failing permutations for non failing opts.dir and failing osTmpDir
82+
for (let oidx = 0; oidx < failing.length; oidx++) {
83+
for (let iidx = 0; iidx < nonFailing.length; iidx++) {
84+
it(_generateSpecName(nonFailing[iidx], failing[oidx]), function (done) {
85+
os.tmpdir = function () { return failing[oidx]; };
86+
tmp.tmpName({ dir: nonFailing[iidx] }, function (err) {
87+
try {
88+
assert.ok(err === null || err === undefined, 'should not have failed');
89+
} catch (err) {
90+
return done(err);
91+
} finally {
92+
os.tmpdir = origfn;
93+
}
94+
done();
95+
});
96+
});
97+
}
98+
}
99+
});
100+
101+
describe('must not fail on valid os.tmpdir() and invalid opts.dir', function () {
102+
// test all non failing permutations for failing opts.dir and non failing osTmpDir
103+
for (let oidx = 0; oidx < nonFailing.length; oidx++) {
104+
for (let iidx = 0; iidx < failing.length; iidx++) {
105+
it(_generateSpecName(failing[iidx], nonFailing[oidx]), function (done) {
106+
os.tmpdir = function () { return nonFailing[oidx]; };
107+
tmp.tmpName({ dir: failing[iidx] }, function (err) {
108+
try {
109+
assert.ok(err === null || err === undefined, 'should not have failed');
110+
} catch (err) {
111+
return done(err);
112+
} finally {
113+
os.tmpdir = origfn;
114+
}
115+
done();
116+
});
117+
});
118+
}
119+
}
120+
});
121+
});
48122
});
49123

50124
describe('when running standard outband tests', function () {

0 commit comments

Comments
 (0)