Skip to content

module: validate paths early, use internal/errors.js #18359

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.

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

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions lib/internal/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,12 +270,14 @@ class AssertionError extends Error {
if (message != null) {
super(message);
} else {
const util = lazyUtil();
if (process.stdout.isTTY && process.stdout.getColorDepth() !== 1) {
if (util_ === null &&
process.stdout.isTTY &&
process.stdout.getColorDepth() !== 1) {
green = '\u001b[32m';
white = '\u001b[39m';
red = '\u001b[31m';
}
const util = lazyUtil();

if (actual && actual.stack && actual instanceof Error)
actual = `${actual.name}: ${actual.message}`;
Expand Down
10 changes: 10 additions & 0 deletions lib/internal/module.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const errors = require('internal/errors');

// Invoke with makeRequireFunction(module) where |module| is the Module object
// to use as the context for the require() function.
function makeRequireFunction(mod) {
Expand All @@ -15,12 +17,20 @@ function makeRequireFunction(mod) {
}

function resolve(request, options) {
if (typeof request !== 'string') {
throw new errors.Error('ERR_INVALID_ARG_TYPE',
'request', 'string', request);
}
return Module._resolveFilename(request, mod, false, options);
}

require.resolve = resolve;

function paths(request) {
if (typeof request !== 'string') {
throw new errors.Error('ERR_INVALID_ARG_TYPE',
'request', 'string', request);
}
return Module._resolveLookupPaths(request, mod, true);
}

Expand Down
13 changes: 9 additions & 4 deletions lib/module.js
Original file line number Diff line number Diff line change
Expand Up @@ -603,10 +603,15 @@ Module.prototype.load = function(filename) {

// Loads a module at the given file path. Returns that module's
// `exports` property.
Module.prototype.require = function(path) {
assert(path, 'missing path');
assert(typeof path === 'string', 'path must be a string');
return Module._load(path, this, /* isMain */ false);
Module.prototype.require = function(id) {
if (typeof id !== 'string') {
throw new errors.TypeError('ERR_INVALID_ARG_TYPE', 'id', 'string', id);
}
if (id === '') {
throw new errors.Error('ERR_INVALID_ARG_VALUE',
'id', id, 'must be a non-empty string');
}
return Module._load(id, this, /* isMain */ false);
};


Expand Down
24 changes: 15 additions & 9 deletions test/parallel/test-module-loading-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,16 +59,22 @@ assert.throws(
}
);

common.expectsError(
require,
{
code: 'ERR_ASSERTION',
message: /^missing path$/
});
const re = /^The "id" argument must be of type string\. Received type \w+$/;
[1, false, null, undefined, {}].forEach((value) => {
common.expectsError(
() => { require(value); },
{
type: TypeError,
code: 'ERR_INVALID_ARG_TYPE',
message: re
});
});


common.expectsError(
() => { require({}); },
() => { require(''); },
{
code: 'ERR_ASSERTION',
message: /^path must be a string$/
type: Error,
code: 'ERR_INVALID_ARG_VALUE',
message: 'The argument \'id\' must be a non-empty string. Received \'\''
});
19 changes: 18 additions & 1 deletion test/parallel/test-require-resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
require('../common');
const common = require('../common');
const fixtures = require('../common/fixtures');
const assert = require('assert');

Expand All @@ -38,3 +38,20 @@ assert.strictEqual('path', require.resolve('path'));
// Test configurable resolve() paths.
require(fixtures.path('require-resolve.js'));
require(fixtures.path('resolve-paths', 'default', 'verify-paths.js'));

const re = /^The "request" argument must be of type string\. Received type \w+$/;
[1, false, null, undefined, {}].forEach((value) => {
common.expectsError(
() => { require.resolve(value); },
{
code: 'ERR_INVALID_ARG_TYPE',
message: re
});

common.expectsError(
() => { require.resolve.paths(value); },
{
code: 'ERR_INVALID_ARG_TYPE',
message: re
});
});
11 changes: 0 additions & 11 deletions test/sequential/test-module-loading.js
Original file line number Diff line number Diff line change
Expand Up @@ -297,17 +297,6 @@ try {
}


// require() must take string, and must be truthy
assert.throws(function() {
console.error('require non-string');
require({ foo: 'bar' });
}, /path must be a string/);

assert.throws(function() {
console.error('require empty string');
require('');
}, /missing path/);

process.on('exit', function() {
assert.ok(a.A instanceof Function);
assert.strictEqual(a.A(), 'A done');
Expand Down