diff --git a/CHANGELOG.md b/CHANGELOG.md index c8793d3..a236a46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## tmp v0.2.0 +- drop support for node version < v8 + + ***BREAKING CHANGE*** + + node version < v8 are no longer supported. + - [#216](https://github.com/raszi/node-tmp/issues/216) ***BREAKING CHANGE*** @@ -11,46 +17,83 @@ Users must install their own SIGINT handler and call process.exit() so that tmp's process exit handler can do the cleanup. - The simplest handler possible would be + A simple handler would be ``` process.on('SIGINT', process.exit); ``` -- [#206](https://github.com/raszi/node-tmp/issues/206) +- [#179](https://github.com/raszi/node-tmp/issues/179) - document name option. + ***BREAKING CHANGE*** + + template option no longer accepts arbitrary paths. all paths must be relative to os.tmpdir(). + +- [#207](https://github.com/raszi/node-tmp/issues/TBD) + + ***BREAKING CHANGE*** + + dir option no longer accepts arbitrary paths. all paths must be relative to os.tmpdir(). + +- [#218](https://github.com/raszi/node-tmp/issues/TBD) + + ***BREAKING CHANGE*** + + name option no longer accepts arbitrary paths. name must no longer contain a path and will always be made relative + to the current os.tmpdir() and the optional dir option. - [#197](https://github.com/raszi/node-tmp/issues/197) + ***BUG FIX*** + sync cleanup callback must be returned when using the sync API functions. fs.rmdirSync() must not be called with a second parameter that is a function. -- [#179](https://github.com/raszi/node-tmp/issues/179) +- [#176](https://github.com/raszi/node-tmp/issues/176) - template option no longer accepts arbitrary paths. + ***BUG FIX*** -- [#176](https://github.com/raszi/node-tmp/issues/176) + fail early if no os.tmpdir() was specified. + previous versions of Electron did return undefined when calling os.tmpdir(). + _getTmpDir() now tries to resolve the path returned by os.tmpdir(). + + now using rimraf for removing directory trees. - fail early if no tmp dir was specified. +- [#206](https://github.com/raszi/node-tmp/issues/206) + + **DOCUMENTATION** - use rimraf for removing directory trees. + document name option. + +- [#236](https://github.com/raszi/node-tmp/issues/236) + + **DOCUMENTATION** + + document discardDescriptor option. + +- [#237](https://github.com/raszi/node-tmp/issues/237) + + **DOCUMENTATION** + + document detachDescriptor option. + +- [#238](https://github.com/raszi/node-tmp/issues/238) + + **DOCUMENTATION** + + document mode option. - [#175](https://github.com/raszi/node-tmp/issues/175) - add unsafeCleanup option to jsdoc. + **DOCUMENTATION** -- drop support for node version < v8 + document unsafeCleanup option. - ***BREAKING CHANGE*** - - node version < v8 are no longer supported. +### Miscellaneous - stabilized tests - - general clean up - - update jsdoc diff --git a/README.md b/README.md index e5ca66f..ff49862 100644 --- a/README.md +++ b/README.md @@ -343,16 +343,25 @@ tmp.setGracefulCleanup(); All options are optional :) - * `name`: a fixed name that overrides random name generation - * `mode`: the file mode to create with, it fallbacks to `0600` on file creation and `0700` on directory creation - * `prefix`: the optional prefix, fallbacks to `tmp-` if not provided - * `postfix`: the optional postfix, fallbacks to `.tmp` on file creation - * `template`: [`mkstemp`][3] like filename template, no default - * `dir`: the optional temporary directory, fallbacks to system default (guesses from environment) + * `name`: a fixed name that overrides random name generation, the name must be relative and must not contain path segments + * `mode`: the file mode to create with, falls back to `0o600` on file creation and `0o700` on directory creation + * `prefix`: the optional prefix, defaults to `tmp` + * `postfix`: the optional postfix + * `template`: [`mkstemp`][3] like filename template, no default, can be either an absolute or a relative path that resolves + to a relative path of the system's default temporary directory, must include `XXXXXX` once for random name generation, e.g. + 'foo/bar/XXXXXX'. Absolute paths are also fine as long as they are relative to os.tmpdir(). + Any directories along the so specified path must exist, otherwise a ENOENT error will be thrown upon access, + as tmp will not check the availability of the path, nor will it establish the requested path for you. + * `dir`: the optional temporary directory that must be relative to the system's default temporary directory. + absolute paths are fine as long as they point to a location under the system's default temporary directory. + Any directories along the so specified path must exist, otherwise a ENOENT error will be thrown upon access, + as tmp will not check the availability of the path, nor will it establish the requested path for you. * `tries`: how many times should the function try to get a unique filename before giving up, default `3` * `keep`: signals that the temporary file or directory should not be deleted on exit, default is `false` * In order to clean up, you will have to call the provided `cleanupCallback` function manually. * `unsafeCleanup`: recursively removes the created temporary directory, even when it's not empty. default is `false` + * `detachDescriptor`: detaches the file descriptor, caller is responsible for closing the file, tmp will no longer try closing the file during garbage collection + * `discardDescriptor`: discards the file descriptor (closes file, fd is -1), tmp will no longer try closing the file during garbage collection [1]: http://nodejs.org/ [2]: https://www.npmjs.com/browse/depended/tmp diff --git a/lib/tmp.js b/lib/tmp.js index bb7ae16..75373ab 100644 --- a/lib/tmp.js +++ b/lib/tmp.js @@ -34,8 +34,8 @@ const EBADF = _c.EBADF || _c.os.errno.EBADF, ENOENT = _c.ENOENT || _c.os.errno.ENOENT, - DIR_MODE = 448 /* 0o700 */, - FILE_MODE = 384 /* 0o600 */, + DIR_MODE = 0o700 /* 448 */, + FILE_MODE = 0o600 /* 384 */, EXIT = 'exit', @@ -46,7 +46,7 @@ const FN_RMDIR_SYNC = fs.rmdirSync.bind(fs), FN_RIMRAF_SYNC = rimraf.sync; -var +let _gracefulCleanup = false; /** @@ -76,6 +76,17 @@ function _randomChars(howMany) { return value.join(''); } +/** + * 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 || _isUndefined(s) || !s.trim(); +} + /** * Checks whether the `obj` parameter is defined or not. * @@ -122,37 +133,119 @@ 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 (!isBlank(opts.name)) { - return path.join(opts.dir || tmpDir, opts.name); - } + if (!_isUndefined(opts.name)) + return path.join(tmpDir, opts.dir, opts.name); - // mkstemps like template - // opts.template has already been guarded in tmpName() below /* istanbul ignore else */ - if (opts.template) { - var template = opts.template; - // make sure that we prepend the tmp path if none was given - /* istanbul ignore else */ - if (path.basename(template) === template) - template = path.join(opts.dir || tmpDir, template); - return template.replace(TEMPLATE_PATTERN, _randomChars(6)); - } + if (!_isUndefined(opts.template)) + return path.join(tmpDir, opts.dir, opts.template).replace(TEMPLATE_PATTERN, _randomChars(6)); // prefix and postfix const name = [ - (isBlank(opts.prefix) ? 'tmp-' : opts.prefix), + opts.prefix ? opts.prefix : 'tmp', + '-', process.pid, + '-', _randomChars(12), - (opts.postfix ? opts.postfix : '') + '-', + opts.postfix ? opts.postfix : '' ].join(''); - return path.join(opts.dir || tmpDir, name); + return path.join(tmpDir, opts.dir, name); +} + +/** + * Asserts whether the specified options are valid, also sanitizes options and provides sane defaults for missing + * options. + * + * @param {Options} options + * @private + */ +function _assertAndSanitizeOptions(options) { + const tmpDir = _getTmpDir(); + /* istanbul ignore else */ + if (!_isUndefined(options.name)) + _assertIsRelative(options.name, 'name', tmpDir); + /* istanbul ignore else */ + if (!_isUndefined(options.dir)) + _assertIsRelative(options.dir, 'dir', tmpDir); + /* istanbul ignore else */ + if (!_isUndefined(options.template)) { + _assertIsRelative(options.template, 'template', tmpDir); + if (!options.template.match(TEMPLATE_PATTERN)) + throw new Error(`Invalid template, found "${options.template}".`); + } + /* istanbul ignore else */ + if (!_isUndefined(options.tries) && isNaN(options.tries) || options.tries < 0) + throw new Error(`Invalid tries, found "${options.tries}".`); + + // if a name was specified we will try once + options.tries = _isUndefined(options.name) ? options.tries || DEFAULT_TRIES : 1; + options.keep = !!options.keep; + options.detachDescriptor = !!options.detachDescriptor; + options.discardDescriptor = !!options.discardDescriptor; + options.unsafeCleanup = !!options.unsafeCleanup; + + // sanitize dir, also keep (multiple) blanks if the user, purportedly sane, requests us to + options.dir = _isUndefined(options.dir) ? '' : path.relative(tmpDir, _resolvePath(options.dir, tmpDir)); + options.template = _isUndefined(options.template) ? undefined : path.relative(tmpDir, _resolvePath(options.template, tmpDir)); + // sanitize further if template is relative to options.dir + options.template = _isBlank(options.template) ? undefined : path.relative(options.dir, options.template); + + // for completeness' sake only, also keep (multiple) blanks if the user, purportedly sane, requests us to + options.name = _isUndefined(options.name) ? undefined : options.name; + options.prefix = _isUndefined(options.prefix) ? '' : options.prefix; + options.postfix = _isUndefined(options.postfix) ? '' : options.postfix; +} + +/** + * Resolve the specified path name in respect to tmpDir. + * + * The specified name might include relative path components, e.g. ../ + * so we need to resolve in order to be sure that is is located inside tmpDir + * + * @param name + * @param tmpDir + * @returns {string} + * @private + */ +function _resolvePath(name, tmpDir) { + if (name.startsWith(tmpDir)) { + return path.resolve(name); + } else { + return path.resolve(path.join(tmpDir, name)); + } +} + +/** + * Asserts whether specified name is relative to the specified tmpDir. + * + * @param {string} name + * @param {string} option + * @param {string} tmpDir + * @throws {Error} + * @private + */ +function _assertIsRelative(name, option, tmpDir) { + if (option === 'name') { + // assert that name is not absolute and does not contain a path + if (path.isAbsolute(name)) + throw new Error(`${option} option must not contain an absolute path, found "${name}".`); + // must not fail on valid . or .. or similar such constructs + let basename = path.basename(name); + if (basename === '..' || basename === '.' || basename !== name) + throw new Error(`${option} option must not contain a path, found "${name}".`); + } + else { // if (option === 'dir' || option === 'template') { + // assert that dir or template are relative to tmpDir + if (path.isAbsolute(name) && !name.startsWith(tmpDir)) { + throw new Error(`${option} option must be relative to "${tmpDir}", found "${name}".`); + } + let resolvedPath = _resolvePath(name, tmpDir); + if (!resolvedPath.startsWith(tmpDir)) + throw new Error(`${option} option must be relative to "${tmpDir}", found "${resolvedPath}".`); + } } /** @@ -162,20 +255,18 @@ function _generateTmpName(opts) { * @param {?tmpNameCallback} callback the callback function */ function tmpName(options, callback) { - var + let args = _parseArguments(options, callback), opts = args[0], - cb = args[1], - tries = !isBlank(opts.name) ? 1 : opts.tries || DEFAULT_TRIES; - - /* istanbul ignore else */ - if (isNaN(tries) || tries < 0) - return cb(new Error('Invalid tries')); + cb = args[1]; - /* istanbul ignore else */ - if (opts.template && !opts.template.match(TEMPLATE_PATTERN)) - return cb(new Error('Invalid template provided')); + try { + _assertAndSanitizeOptions(opts); + } catch (err) { + return cb(err); + } + let tries = opts.tries; (function _getUniqueName() { try { const name = _generateTmpName(opts); @@ -208,17 +299,11 @@ function tmpName(options, callback) { function tmpNameSync(options) { var args = _parseArguments(options), - opts = args[0], - tries = !isBlank(opts.name) ? 1 : opts.tries || DEFAULT_TRIES; - - /* istanbul ignore else */ - if (isNaN(tries) || tries < 0) - throw new Error('Invalid tries'); + opts = args[0]; - /* istanbul ignore else */ - if (opts.template && !opts.template.match(TEMPLATE_PATTERN)) - throw new Error('Invalid template provided'); + _assertAndSanitizeOptions(opts); + let tries = opts.tries; do { const name = _generateTmpName(opts); try { @@ -533,17 +618,6 @@ 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. */ @@ -558,7 +632,7 @@ function setGracefulCleanup() { * @returns {string} the currently configured tmp dir */ function _getTmpDir() { - return os.tmpdir(); + return path.resolve(os.tmpdir()); } // Install process exit listener @@ -570,12 +644,15 @@ process.addListener(EXIT, _garbageCollector); * @typedef {Object} Options * @property {?boolean} keep the temporary object (file or dir) will not be garbage collected * @property {?number} tries the number of tries before give up the name generation + * @property (?int) mode the access mode, defaults are 0o700 for directories and 0o600 for files * @property {?string} template the "mkstemp" like filename template * @property {?string} name fixed name relative to tmpdir or the specified dir option * @property {?string} dir tmp directory relative to the system tmp directory to use * @property {?string} prefix prefix for the generated name * @property {?string} postfix postfix for the generated name * @property {?boolean} unsafeCleanup recursively removes the created temporary directory, even when it's not empty + * @property {?boolean} detachDescriptor detaches the file descriptor, caller is responsible for closing the file, tmp will no longer try closing the file during garbage collection + * @property {?boolean} discardDescriptor discards the file descriptor (closes file, fd is -1), tmp will no longer try closing the file during garbage collection */ /** diff --git a/test/name-sync-test.js b/test/name-sync-test.js index 8921150..c996f82 100644 --- a/test/name-sync-test.js +++ b/test/name-sync-test.js @@ -38,66 +38,15 @@ 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; - } - }); - } + it('must fail on invalid os.tmpdir()', function () { + os.tmpdir = function () { return undefined; }; + try { + tmp.tmpNameSync(); + assert.fail('should have failed'); + } catch (err) { + assert.ok(err instanceof Error); + } finally { + os.tmpdir = origfn; } }); }); diff --git a/test/name-test.js b/test/name-test.js index 7dac189..99659b0 100644 --- a/test/name-test.js +++ b/test/name-test.js @@ -48,75 +48,18 @@ 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(); - }); - }); + it('must fail on invalid os.tmpdir()', function (done) { + os.tmpdir = function () { return undefined; }; + tmp.tmpName(function (err) { + try { + assert.ok(err instanceof Error, 'should have failed'); + } catch (err) { + return done(err); + } finally { + os.tmpdir = origfn; } - } + done(); + }); }); }); }); diff --git a/test/template-sync-test.js b/test/template-sync-test.js index a0e7b39..0a96054 100644 --- a/test/template-sync-test.js +++ b/test/template-sync-test.js @@ -11,7 +11,7 @@ describe('tmp', function () { try { tmp.dirSync({template:'invalid'}); } catch (err) { - assert.equal(err.message, 'Invalid template provided', 'should have thrown the expected error'); + assert.equal(err.message, 'Invalid template, found "invalid".', 'should have thrown the expected error'); } }); }); @@ -21,7 +21,7 @@ describe('tmp', function () { try { tmp.fileSync({template:'invalid'}); } catch (err) { - assert.equal(err.message, 'Invalid template provided', 'should have thrown the expected error'); + assert.equal(err.message, 'Invalid template, found "invalid".', 'should have thrown the expected error'); } }); }); diff --git a/test/template-test.js b/test/template-test.js index 339f51a..ed31f7e 100644 --- a/test/template-test.js +++ b/test/template-test.js @@ -11,9 +11,9 @@ describe('tmp', function () { tmp.dir({template:'invalid'}, function (err) { if (!err) return done(new Error('err expected')); try { - assert.equal(err.message, 'Invalid template provided', 'should have thrown the expected error'); + assert.equal(err.message, 'Invalid template, found "invalid".', 'should have thrown the expected error'); } catch (err2) { - done(err); + return done(err2); } done(); }); @@ -25,9 +25,9 @@ describe('tmp', function () { tmp.file({template:'invalid'}, function (err) { if (!err) return done(new Error('err expected')); try { - assert.equal(err.message, 'Invalid template provided', 'should have thrown the expected error'); + assert.equal(err.message, 'Invalid template, found "invalid".', 'should have thrown the expected error'); } catch (err2) { - done(err); + return done(err2); } done(); });