module: fix node_modules search path in edge case

The `p < nmLen` condition will fail when a module's name is end with `node_modules` like `foo_node_modules`. The old logic will miss the `foo_node_modules/node_modules` in node_modules paths. TL;TR, a module named like `foo_node_modules` can't require any module in the node_modules folder. Fixes: #6679 PR-URL: #6670 Reviewed-By: Evan Lucas <[email protected]>

Author: hefangshi

lib/module.js

@@ -221,17 +221,29 @@ if (process.platform === 'win32'){
 // note: this approach *only* works when the path is guaranteed
 // to be absolute. Doing a fully-edge-case-correct path.split
 // that works on both Windows and Posix is non-trivial.
+
+// return root node_modules when path is 'D:\\'.
+// path.resolve will make sure from.length >=3 in Windows.
+if(from.charCodeAt(from.length-1)===92/*\*/&&
+from.charCodeAt(from.length-2)===58/*:*/)
+return[from+'node_modules'];
+
 constpaths=[];
 varp=0;
 varlast=from.length;
 for(vari=from.length-1;i>=0;--i){
 constcode=from.charCodeAt(i);
-if(code===92/*\*/||code===47/*/*/){
+// The path segment separator check ('\' and '/') was used to get
+// node_modules path for every path segment.
+// Use colon as an extra condition since we can get node_modules
+// path for dirver root like 'C:\node_modules' and don't need to
+// parse driver name.
+if(code===92/*\*/||code===47/*/*/||code===58/*:*/){
 if(p!==nmLen)
 paths.push(from.slice(0,last)+'\\node_modules');
 last=i;
 p=0;
-}elseif(p!==-1&&p<nmLen){
+}elseif(p!==-1){
 if(nmChars[p]===code){
 ++p;
 }else{
@@ -265,7 +277,7 @@ if (process.platform === 'win32'){
 paths.push(from.slice(0,last)+'/node_modules');
 last=i;
 p=0;
-}elseif(p!==-1&&p<nmLen){
+}elseif(p!==-1){
 if(nmChars[p]===code){
 ++p;
 }else{
@@ -274,6 +286,9 @@ if (process.platform === 'win32'){
 }
 }
 
+// Append /node_modules to handle root paths.
+paths.push('/node_modules');
+
 returnpaths;
 };
 }

test/parallel/test-module-nodemodulepaths.js

@@ -1,20 +1,105 @@
 'use strict';
-varcommon=require('../common');
-varassert=require('assert');
 
-varmodule=require('module');
+constcommon=require('../common');
+constassert=require('assert');
+const_module=require('module');
 
-varfile,delimiter,paths;
+constcases={
+'WIN': [{
+file: 'C:\\Users\\hefangshi\\AppData\\Roaming\
+\\npm\\node_modules\\npm\\node_modules\\minimatch',
+expect: [
+'C:\\Users\\hefangshi\\AppData\\Roaming\
+\\npm\\node_modules\\npm\\node_modules\\minimatch\\node_modules',
+'C:\\Users\\hefangshi\\AppData\\Roaming\
+\\npm\\node_modules\\npm\\node_modules',
+'C:\\Users\\hefangshi\\AppData\\Roaming\\npm\\node_modules',
+'C:\\Users\\hefangshi\\AppData\\Roaming\\node_modules',
+'C:\\Users\\hefangshi\\AppData\\node_modules',
+'C:\\Users\\hefangshi\\node_modules',
+'C:\\Users\\node_modules',
+'C:\\node_modules'
+]
+},{
+file: 'C:\\Users\\Rocko Artischocko\\node_stuff\\foo',
+expect: [
+'C:\\Users\\Rocko Artischocko\\node_stuff\\foo\\node_modules',
+'C:\\Users\\Rocko Artischocko\\node_stuff\\node_modules',
+'C:\\Users\\Rocko Artischocko\\node_modules',
+'C:\\Users\\node_modules',
+'C:\\node_modules'
+]
+},{
+file: 'C:\\Users\\Rocko Artischocko\\node_stuff\\foo_node_modules',
+expect: [
+'C:\\Users\\Rocko \
+Artischocko\\node_stuff\\foo_node_modules\\node_modules',
+'C:\\Users\\Rocko Artischocko\\node_stuff\\node_modules',
+'C:\\Users\\Rocko Artischocko\\node_modules',
+'C:\\Users\\node_modules',
+'C:\\node_modules'
+]
+},{
+file: 'C:\\node_modules',
+expect: [
+'C:\\node_modules'
+]
+},{
+file: 'C:\\',
+expect: [
+'C:\\node_modules'
+]
+}],
+'POSIX': [{
+file: '/usr/lib/node_modules/npm/node_modules/\
+node-gyp/node_modules/glob/node_modules/minimatch',
+expect: [
+'/usr/lib/node_modules/npm/node_modules/\
+node-gyp/node_modules/glob/node_modules/minimatch/node_modules',
+'/usr/lib/node_modules/npm/node_modules/\
+node-gyp/node_modules/glob/node_modules',
+'/usr/lib/node_modules/npm/node_modules/node-gyp/node_modules',
+'/usr/lib/node_modules/npm/node_modules',
+'/usr/lib/node_modules',
+'/usr/node_modules',
+'/node_modules'
+]
+},{
+file: '/usr/test/lib/node_modules/npm/foo',
+expect: [
+'/usr/test/lib/node_modules/npm/foo/node_modules',
+'/usr/test/lib/node_modules/npm/node_modules',
+'/usr/test/lib/node_modules',
+'/usr/test/node_modules',
+'/usr/node_modules',
+'/node_modules'
+]
+},{
+file: '/usr/test/lib/node_modules/npm/foo_node_modules',
+expect: [
+'/usr/test/lib/node_modules/npm/foo_node_modules/node_modules',
+'/usr/test/lib/node_modules/npm/node_modules',
+'/usr/test/lib/node_modules',
+'/usr/test/node_modules',
+'/usr/node_modules',
+'/node_modules'
+]
+},{
+file: '/node_modules',
+expect: [
+'/node_modules'
+]
+},{
+file: '/',
+expect: [
+'/node_modules'
+]
+}]
+};
 
-if(common.isWindows){
-file='C:\\Users\\Rocko Artischocko\\node_stuff\\foo';
-delimiter='\\';
-}else{
-file='/usr/test/lib/node_modules/npm/foo';
-delimiter='/';
-}
-
-paths=module._nodeModulePaths(file);
-
-assert.ok(paths.indexOf(file+delimiter+'node_modules')!==-1);
-assert.ok(Array.isArray(paths));
+constplatformCases=common.isWindows ? cases.WIN : cases.POSIX;
+platformCases.forEach((c)=>{
+constpaths=_module._nodeModulePaths(c.file);
+assert.deepStrictEqual(c.expect,paths,'case '+c.file+
+' failed, actual paths is '+JSON.stringify(paths));
+});