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.

Sign up for GitHub

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

Jump to bottom

fix: fail early if there is no tmp dir specified #177

Merged
merged 1 commit into from Mar 20, 2019
Merged

fix: fail early if there is no tmp dir specified #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
99 changes: 68 additions & 31 deletions lib/tmp.js
View file
Open in desktop
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
3 changes: 1 addition & 2 deletions package.json
View file
Open in desktop
Expand Up @@ -22,9 +22,8 @@
"node": ">=6"
},
"dependencies": {
"rimraf": "^2.6.2"
"rimraf": "^2.6.3"
},
"dependencies": {},
"devDependencies": {
"eslint": "^4.19.1",
"eslint-plugin-mocha": "^5.0.0",
Expand Down
1 change: 0 additions & 1 deletion test/dir-test.js
View file
Open in desktop
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
Open in desktop
@@ -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
Open in desktop
@@ -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