From 15a6f6278f942a1e69f439dd102d70fce70a91e6 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Thu, 16 Aug 2018 20:19:06 +0300 Subject: [PATCH 1/3] module: fix inconsistency between load and _findPath 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: https://github.com/nodejs/node/issues/4778 --- lib/internal/modules/cjs/loader.js | 11 ++++- .../test-module-deleted-extensions.js | 18 -------- .../test-module-deleted-extensions.js | 43 +++++++++++++++++++ 3 files changed, 52 insertions(+), 20 deletions(-) delete mode 100644 test/known_issues/test-module-deleted-extensions.js create mode 100644 test/parallel/test-module-deleted-extensions.js diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index c218dc2cac2a50..8851a60f7f89d6 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -216,6 +216,13 @@ function tryExtensions(p, exts, isMain) { return false; } +function readExtensions() { + const exts = Object.keys(Module._extensions); + for (var i = 0; i < exts.length; ++i) + exts[i] = path.extname(exts[i]) || exts[i]; + return exts; +} + var warned = false; Module._findPath = function(request, paths, isMain) { if (path.isAbsolute(request)) { @@ -272,7 +279,7 @@ 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); } } @@ -280,7 +287,7 @@ Module._findPath = function(request, paths, 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); diff --git a/test/known_issues/test-module-deleted-extensions.js b/test/known_issues/test-module-deleted-extensions.js deleted file mode 100644 index 3a51e8725eec60..00000000000000 --- a/test/known_issues/test-module-deleted-extensions.js +++ /dev/null @@ -1,18 +0,0 @@ -'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['.foo.bar'] = (module, path) => {}; -delete require.extensions['.foo.bar']; -require.extensions['.bar'] = common.mustCall((module, path) => { - assert.strictEqual(module.id, file); - assert.strictEqual(path, file); -}); -require(path.join(tmpdir.path, 'test-extensions')); diff --git a/test/parallel/test-module-deleted-extensions.js b/test/parallel/test-module-deleted-extensions.js new file mode 100644 index 00000000000000..872563e63efde4 --- /dev/null +++ b/test/parallel/test-module-deleted-extensions.js @@ -0,0 +1,43 @@ +'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}'`) + ); +} + +{ + require.extensions['.foo.bar'] = common.mustNotCall(); + delete require.extensions['.foo.bar']; + const modulePath = path.join(tmpdir.path, 'test-extensions'); + assert.throws( + () => require(modulePath), + new Error(`Cannot find module '${modulePath}'`) + ); +} + +{ + 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); +} From 1b51f7e9d47c20a39114ad728473a9045dee9914 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Wed, 22 Aug 2018 12:23:33 +0300 Subject: [PATCH 2/3] fixup! module: fix inconsistency between load and _findPath --- lib/internal/modules/cjs/loader.js | 7 +++++-- test/parallel/test-module-deleted-extensions.js | 11 +++++++++++ 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/lib/internal/modules/cjs/loader.js b/lib/internal/modules/cjs/loader.js index 8851a60f7f89d6..4c1a5556616330 100644 --- a/lib/internal/modules/cjs/loader.js +++ b/lib/internal/modules/cjs/loader.js @@ -218,8 +218,11 @@ function tryExtensions(p, exts, isMain) { function readExtensions() { const exts = Object.keys(Module._extensions); - for (var i = 0; i < exts.length; ++i) - exts[i] = path.extname(exts[i]) || exts[i]; + for (var i = 0, j = 0; i < exts.length; ++i) { + if (path.extname(exts[i]) === '') + exts[j++] = exts[i]; + } + exts.length = j; return exts; } diff --git a/test/parallel/test-module-deleted-extensions.js b/test/parallel/test-module-deleted-extensions.js index 872563e63efde4..cb94f26bceb8ff 100644 --- a/test/parallel/test-module-deleted-extensions.js +++ b/test/parallel/test-module-deleted-extensions.js @@ -22,6 +22,17 @@ fs.writeFileSync(file, '', 'utf8'); ); } +{ + require.extensions['.bar'] = common.mustNotCall(); + require.extensions['.foo.bar'] = common.mustNotCall(); + delete require.extensions['.bar']; + const modulePath = path.join(tmpdir.path, 'test-extensions'); + assert.throws( + () => require(modulePath), + new Error(`Cannot find module '${modulePath}'`) + ); +} + { require.extensions['.foo.bar'] = common.mustNotCall(); delete require.extensions['.foo.bar']; From b00970837365af53466e388f89cad18d48e38646 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Thu, 23 Aug 2018 02:58:03 +0300 Subject: [PATCH 3/3] fixup! module: fix inconsistency between load and _findPath --- test/parallel/test-module-deleted-extensions.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-module-deleted-extensions.js b/test/parallel/test-module-deleted-extensions.js index cb94f26bceb8ff..f14da0a70f7656 100644 --- a/test/parallel/test-module-deleted-extensions.js +++ b/test/parallel/test-module-deleted-extensions.js @@ -23,18 +23,21 @@ fs.writeFileSync(file, '', 'utf8'); } { - require.extensions['.bar'] = common.mustNotCall(); - require.extensions['.foo.bar'] = common.mustNotCall(); 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'`) + ); } { - require.extensions['.foo.bar'] = common.mustNotCall(); + delete require.extensions['.bar']; delete require.extensions['.foo.bar']; const modulePath = path.join(tmpdir.path, 'test-extensions'); assert.throws( @@ -44,6 +47,7 @@ fs.writeFileSync(file, '', 'utf8'); } { + delete require.extensions['.foo.bar']; require.extensions['.bar'] = common.mustCall((module, path) => { assert.strictEqual(module.id, file); assert.strictEqual(path, file);