Skip to content

Commit

Permalink
Merge pull request #177 from raszi/gh-176
Browse files Browse the repository at this point in the history 
fix: fail early if there is no tmp dir specified
  • Loading branch information
@silkentrance
silkentrance authored Mar 20, 2019
2 parents 84cea56 + 48b9f72 commit 69ad512
Show file tree
Hide file tree
Showing 4 changed files with 210 additions and 34 deletions.
99 changes: 68 additions & 31 deletions lib/tmp.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,6 @@ const rimraf = require('rimraf');
* The working inner variables.
*/
const
/**
* The temporary directory.
* @type {string}
*/
tmpDir = os.tmpdir(),

// the random characters to choose from
RANDOM_CHARS = '0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz',

Expand Down Expand Up @@ -123,12 +117,21 @@ function _parseArguments(options, callback) {
* @private
*/
function _generateTmpName(opts) {

const tmpDir = _getTmpDir();

// fail early on missing tmp dir
if (isBlank(opts.dir) && isBlank(tmpDir)) {
throw new Error('No tmp dir specified');
}

/* istanbul ignore else */
if (opts.name) {
if (!isBlank(opts.name)) {
return path.join(opts.dir || tmpDir, opts.name);
}

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

// prefix and postfix
const name = [
opts.prefix || 'tmp-',
(isBlank(opts.prefix) ? 'tmp-' : opts.prefix),
process.pid,
_randomChars(12),
opts.postfix || ''
(isBlank(opts.postfix) ? '' : opts.postfix)
].join('');

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

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

(function _getUniqueName() {
const name = _generateTmpName(opts);
try {
const name = _generateTmpName(opts);

// check whether the path exists then retry if needed
fs.stat(name, function (err) {
/* istanbul ignore else */
if (!err) {
// check whether the path exists then retry if needed
fs.stat(name, function (err) {
/* istanbul ignore else */
if (tries-- > 0) return _getUniqueName();
if (!err) {
/* istanbul ignore else */
if (tries-- > 0) return _getUniqueName();

return cb(new Error('Could not get a unique tmp filename, max tries reached ' + name));
}
return cb(new Error('Could not get a unique tmp filename, max tries reached ' + name));
}

cb(null, name);
});
cb(null, name);
});
} catch (err) {
cb(err);
}
}());
}

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

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

opts.postfix = (_isUndefined(opts.postfix)) ? '.tmp' : opts.postfix;
opts.postfix = isBlank(opts.postfix) ? '.tmp' : opts.postfix;

// gets a temporary filename
tmpName(opts, function _tmpNameCreated(err, name) {
Expand Down Expand Up @@ -287,7 +294,7 @@ function fileSync(options) {
args = _parseArguments(options),
opts = args[0];

opts.postfix = (_isUndefined(opts.postfix)) ? '.tmp' : opts.postfix;
opts.postfix = isBlank(opts.postfix) ? '.tmp' : opts.postfix;

const discardOrDetachDescriptor = opts.discardDescriptor || opts.detachDescriptor;
const name = tmpNameSync(opts);
Expand Down Expand Up @@ -536,32 +543,53 @@ function isENOENT(error) {
* which will differ between the supported node versions.
*
* - Node >= 7.0:
* error.code {String}
* error.errno {String|Number} any numerical value will be negated
* error.code {string}
* error.errno {string|number} any numerical value will be negated
*
* - Node >= 6.0 < 7.0:
* error.code {String}
* error.errno {Number} negated
* error.code {string}
* error.errno {number} negated
*
* - Node >= 4.0 < 6.0: introduces SystemError
* error.code {String}
* error.errno {Number} negated
* error.code {string}
* error.errno {number} negated
*
* - Node >= 0.10 < 4.0:
* error.code {Number} negated
* error.code {number} negated
* error.errno n/a
*/
function isExpectedError(error, code, errno) {
return error.code === code || error.code === errno;
}

/**
* Helper which determines whether a string s is blank, that is undefined, or empty or null.
*
* @private
* @param {string} s
* @returns {Boolean} true whether the string s is blank, false otherwise
*/
function isBlank(s) {
return s === null || s === undefined || !s.trim();
}

/**
* Sets the graceful cleanup.
*/
function setGracefulCleanup() {
_gracefulCleanup = true;
}

/**
* Returns the currently configured tmp dir from os.tmpdir().
*
* @private
* @returns {string} the currently configured tmp dir
*/
function _getTmpDir() {
return os.tmpdir();
}

/**
* If there are multiple different versions of tmp in place, make sure that
* we recognize the old listeners.
Expand Down Expand Up @@ -715,7 +743,16 @@ _safely_install_sigint_listener();
*/

// exporting all the needed methods
module.exports.tmpdir = tmpDir;

// evaluate os.tmpdir() lazily, mainly for simplifying testing but it also will
// allow users to reconfigure the temporary directory
Object.defineProperty(module.exports, 'tmpdir', {
enumerable: true,
configurable: false,
get: function () {
return _getTmpDir();
}
});

module.exports.dir = dir;
module.exports.dirSync = dirSync;
Expand Down
1 change: 0 additions & 1 deletion test/dir-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ describe('tmp', function () {
});

describe('when running issue specific inband tests', function () {
// add your issue specific tests here
});

describe('when running standard outband tests', function () {
Expand Down
68 changes: 67 additions & 1 deletion test/name-sync-test.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/* eslint-disable no-octal */
// vim: expandtab:ts=2:sw=2

var
const
assert = require('assert'),
os = require('os'),
inbandStandardTests = require('./name-inband-standard'),
tmp = require('../lib/tmp');

Expand Down Expand Up @@ -35,6 +36,71 @@ describe('tmp', function () {
});

describe('when running issue specific inband tests', function () {
describe('on issue #176', function () {
const origfn = os.tmpdir;

function _generateSpecName(optsDir, osTmpDir) {
return 'opts.dir = "$1", os.tmpdir() = "$2"'.replace('$1', optsDir).replace('$2', osTmpDir);
}

const failing = ['', ' ', undefined, null];
const nonFailing = ['tmp']; // the origfn cannot be trusted as the os may or may not have a valid tmp dir

describe('must fail on invalid os.tmpdir() and invalid opts.dir', function () {
// test all failing permutations
for (let oidx = 0; oidx < failing.length; oidx++) {
for (let iidx = 0; iidx < failing.length; iidx++) {
it(_generateSpecName(failing[iidx], failing[oidx]), function () {
os.tmpdir = function () { return failing[oidx]; };
try {
tmp.tmpNameSync({ dir: failing[iidx] });
assert.fail('expected this to fail');
} catch (err) {
assert.ok(err instanceof Error, 'error expected');
} finally {
os.tmpdir = origfn;
}
});
}
}
});

describe('must not fail on invalid os.tmpdir() and valid opts.dir', function () {
// test all non failing permutations for non failing opts.dir and failing osTmpDir
for (let oidx = 0; oidx < failing.length; oidx++) {
for (let iidx = 0; iidx < nonFailing.length; iidx++) {
it(_generateSpecName(nonFailing[iidx], failing[oidx]), function () {
os.tmpdir = function () { return failing[oidx]; };
try {
tmp.tmpNameSync({ dir: nonFailing[iidx] });
} catch (err) {
assert.fail(err);
} finally {
os.tmpdir = origfn;
}
});
}
}
});

describe('must not fail on valid os.tmpdir() and invalid opts.dir', function () {
// test all non failing permutations for failing opts.dir and non failing osTmpDir
for (let oidx = 0; oidx < nonFailing.length; oidx++) {
for (let iidx = 0; iidx < failing.length; iidx++) {
it(_generateSpecName(failing[iidx], nonFailing[oidx]), function () {
os.tmpdir = function () { return nonFailing[oidx]; };
try {
tmp.tmpNameSync({ dir: failing[iidx] });
} catch (err) {
assert.fail(err);
} finally {
os.tmpdir = origfn;
}
});
}
}
});
});
});

describe('when running standard outband tests', function () {
Expand Down
76 changes: 75 additions & 1 deletion test/name-test.js
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
/* eslint-disable no-octal */
// vim: expandtab:ts=2:sw=2

var
const
assert = require('assert'),
os = require('os'),
inbandStandardTests = require('./name-inband-standard'),
tmp = require('../lib/tmp');

Expand Down Expand Up @@ -45,6 +46,79 @@ describe('tmp', function () {
});

describe('when running issue specific inband tests', function () {
describe('on issue #176', function () {
const origfn = os.tmpdir;

function _generateSpecName(optsDir, osTmpDir) {
return 'opts.dir = "$1", os.tmpdir() = "$2"'.replace('$1', optsDir).replace('$2', osTmpDir);
}

const failing = ['', ' ', undefined, null];
const nonFailing = ['tmp']; // the origfn cannot be trusted as the os may or may not have a valid tmp dir

describe('must fail on invalid os.tmpdir() and invalid opts.dir', function () {
// test all failing permutations
for (let oidx = 0; oidx < failing.length; oidx++) {
for (let iidx = 0; iidx < failing.length; iidx++) {
it(_generateSpecName(failing[iidx], failing[oidx]), function (done) {
os.tmpdir = function () { return failing[oidx]; };
tmp.tmpName({ dir: failing[iidx] }, function (err) {
try {
assert.ok(err instanceof Error, 'should have failed');
} catch (err) {
return done(err);
} finally {
os.tmpdir = origfn;
}
done();
});
});
}
}
});

describe('must not fail on invalid os.tmpdir() and valid opts.dir', function () {
// test all non failing permutations for non failing opts.dir and failing osTmpDir
for (let oidx = 0; oidx < failing.length; oidx++) {
for (let iidx = 0; iidx < nonFailing.length; iidx++) {
it(_generateSpecName(nonFailing[iidx], failing[oidx]), function (done) {
os.tmpdir = function () { return failing[oidx]; };
tmp.tmpName({ dir: nonFailing[iidx] }, function (err) {
try {
assert.ok(err === null || err === undefined, 'should not have failed');
} catch (err) {
return done(err);
} finally {
os.tmpdir = origfn;
}
done();
});
});
}
}
});

describe('must not fail on valid os.tmpdir() and invalid opts.dir', function () {
// test all non failing permutations for failing opts.dir and non failing osTmpDir
for (let oidx = 0; oidx < nonFailing.length; oidx++) {
for (let iidx = 0; iidx < failing.length; iidx++) {
it(_generateSpecName(failing[iidx], nonFailing[oidx]), function (done) {
os.tmpdir = function () { return nonFailing[oidx]; };
tmp.tmpName({ dir: failing[iidx] }, function (err) {
try {
assert.ok(err === null || err === undefined, 'should not have failed');
} catch (err) {
return done(err);
} finally {
os.tmpdir = origfn;
}
done();
});
});
}
}
});
});
});

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

0 comments on commit 69ad512

Please sign in to comment.