Skip to content

Commit

Permalink
module: fix inconsistency between load and _findPath
Browse files Browse the repository at this point in the history
Files with multiple extensions are not handled by require-module system
therefore if you have file 'file.foo.bar' and require('./file') it won't
be found even while using require.extensions['foo.bar'] but before this
commit if you have require.extensions['foo.bar'] and
require.extensions['bar'] set then the latter will be called if you do
require('./file') but if you remove the former the former ('foo.bar')
property it will fail.
This commit makes it always fail in such cases.

Fixes: #4778

PR-URL: #22382
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: John-David Dalton <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
  • Loading branch information
lundibundi authored and addaleax committed Sep 17, 2018
1 parent 3c2aa4b commit 1b92214
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 20 deletions.
14 changes: 12 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,16 @@ function tryExtensions(p, exts, isMain) {
return false;
}

function readExtensions() {
const exts = Object.keys(Module._extensions);
for (var i = 0, j = 0; i < exts.length; ++i) {
if (path.extname(exts[i]) === '')
exts[j++] = exts[i];
}
exts.length = j;
return exts;
}

var warned = false;
Module._findPath = function(request, paths, isMain) {
if (path.isAbsolute(request)) {
Expand Down Expand Up @@ -273,15 +283,15 @@ Module._findPath = function(request, paths, isMain) {
if (!filename) {
// try it with each of the extensions
if (exts === undefined)
exts = Object.keys(Module._extensions);
exts = readExtensions();
filename = tryExtensions(basePath, exts, isMain);
}
}

if (!filename && rc === 1) { // Directory.
// try it with each of the extensions at "index"
if (exts === undefined)
exts = Object.keys(Module._extensions);
exts = readExtensions();
filename = tryPackage(basePath, exts, isMain);
if (!filename) {
filename = tryExtensions(path.resolve(basePath, 'index'), exts, isMain);
Expand Down
18 changes: 0 additions & 18 deletions test/known_issues/test-module-deleted-extensions.js

This file was deleted.

58 changes: 58 additions & 0 deletions test/parallel/test-module-deleted-extensions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
'use strict';

// Refs: https://github.com/nodejs/node/issues/4778

const common = require('../common');
const assert = require('assert');
const fs = require('fs');
const path = require('path');
const tmpdir = require('../common/tmpdir');
const file = path.join(tmpdir.path, 'test-extensions.foo.bar');

tmpdir.refresh();
fs.writeFileSync(file, '', 'utf8');

{
require.extensions['.bar'] = common.mustNotCall();
require.extensions['.foo.bar'] = common.mustNotCall();
const modulePath = path.join(tmpdir.path, 'test-extensions');
assert.throws(
() => require(modulePath),
new Error(`Cannot find module '${modulePath}'`)
);
}

{
delete require.extensions['.bar'];
require.extensions['.foo.bar'] = common.mustNotCall();
const modulePath = path.join(tmpdir.path, 'test-extensions');
assert.throws(
() => require(modulePath),
new Error(`Cannot find module '${modulePath}'`)
);
assert.throws(
() => require(modulePath + '.foo'),
new Error(`Cannot find module '${modulePath}.foo'`)
);
}

{
delete require.extensions['.bar'];
delete require.extensions['.foo.bar'];
const modulePath = path.join(tmpdir.path, 'test-extensions');
assert.throws(
() => require(modulePath),
new Error(`Cannot find module '${modulePath}'`)
);
}

{
delete require.extensions['.foo.bar'];
require.extensions['.bar'] = common.mustCall((module, path) => {
assert.strictEqual(module.id, file);
assert.strictEqual(path, file);
});

const modulePath = path.join(tmpdir.path, 'test-extensions.foo');
require(modulePath);
}

0 comments on commit 1b92214

Please sign in to comment.