Skip to content

Commit 698a3e1

Browse files
ljharbljharb
committed
[Breaking] add isDirectory; error early if provided basedir is not a directory
Fixes #154
1 parent bab0475 commit 698a3e1

File tree

8 files changed

+146
-32
lines changed

8 files changed

+146
-32
lines changed

‎lib/async.js

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,16 @@ var defaultIsFile = function isFile(file, cb) {
1414
});
1515
};
1616

17+
var defaultIsDir = function isDirectory(dir, cb) {
18+
fs.stat(dir, function (err, stat) {
19+
if (!err) {
20+
return cb(null, stat.isDirectory());
21+
}
22+
if (err.code === 'ENOENT' || err.code === 'ENOTDIR') return cb(null, false);
23+
return cb(err);
24+
});
25+
};
26+
1727
module.exports = function resolve(x, options, callback) {
1828
var cb = callback;
1929
var opts = options || {};
@@ -29,6 +39,7 @@ module.exports = function resolve(x, options, callback) {
2939
}
3040

3141
var isFile = opts.isFile || defaultIsFile;
42+
var isDirectory = opts.isDirectory || defaultIsDir;
3243
var readFile = opts.readFile || fs.readFile;
3344

3445
var extensions = opts.extensions || ['.js'];
@@ -37,22 +48,35 @@ module.exports = function resolve(x, options, callback) {
3748

3849
opts.paths = opts.paths || [];
3950

40-
if (/^(?:\.\.?(?:\/|$)|\/|([A-Za-z]:)?[/\\])/.test(x)) {
41-
var res = path.resolve(parent, x);
42-
if (x === '..' || x.slice(-1) === '/') res += '/';
43-
if (/\/$/.test(x) && res === parent) {
44-
loadAsDirectory(res, opts.package, onfile);
45-
} else loadAsFile(res, opts.package, onfile);
46-
} else loadNodeModules(x, parent, function (err, n, pkg) {
47-
if (err) cb(err);
48-
else if (core[x]) return cb(null, x);
49-
else if (n) return cb(null, n, pkg);
50-
else {
51-
var moduleError = new Error("Cannot find module '" + x + "' from '" + parent + "'");
52-
moduleError.code = 'MODULE_NOT_FOUND';
53-
cb(moduleError);
54-
}
55-
});
51+
if (opts.basedir) {
52+
var basedirError = new TypeError('Provided basedir "' + opts.basedir + '" is not a directory');
53+
isDirectory(opts.basedir, function (err, result) {
54+
if (err) return cb(err);
55+
if (!result) { return cb(basedirError); }
56+
validBasedir();
57+
});
58+
} else {
59+
validBasedir();
60+
}
61+
var res;
62+
function validBasedir() {
63+
if (/^(?:\.\.?(?:\/|$)|\/|([A-Za-z]:)?[/\\])/.test(x)) {
64+
res = path.resolve(parent, x);
65+
if (x === '..' || x.slice(-1) === '/') res += '/';
66+
if (/\/$/.test(x) && res === parent) {
67+
loadAsDirectory(res, opts.package, onfile);
68+
} else loadAsFile(res, opts.package, onfile);
69+
} else loadNodeModules(x, parent, function (err, n, pkg) {
70+
if (err) cb(err);
71+
else if (core[x]) return cb(null, x);
72+
else if (n) return cb(null, n, pkg);
73+
else {
74+
var moduleError = new Error("Cannot find module '" + x + "' from '" + parent + "'");
75+
moduleError.code = 'MODULE_NOT_FOUND';
76+
cb(moduleError);
77+
}
78+
});
79+
}
5680

5781
function onfile(err, m, pkg) {
5882
if (err) cb(err);

‎lib/sync.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,23 @@ var defaultIsFile = function isFile(file) {
1414
return stat.isFile() || stat.isFIFO();
1515
};
1616

17+
var defaultIsDir = function isDirectory(dir) {
18+
try {
19+
var stat = fs.statSync(dir);
20+
} catch (e) {
21+
if (e && (e.code === 'ENOENT' || e.code === 'ENOTDIR')) return false;
22+
throw e;
23+
}
24+
return stat.isDirectory();
25+
};
26+
1727
module.exports = function (x, options) {
1828
if (typeof x !== 'string') {
1929
throw new TypeError('Path must be a string.');
2030
}
2131
var opts = options || {};
2232
var isFile = opts.isFile || defaultIsFile;
33+
var isDirectory = opts.isDirectory || defaultIsDir;
2334
var readFileSync = opts.readFileSync || fs.readFileSync;
2435

2536
var extensions = opts.extensions || ['.js'];
@@ -28,6 +39,10 @@ module.exports = function (x, options) {
2839

2940
opts.paths = opts.paths || [];
3041

42+
if (opts.basedir && !isDirectory(opts.basedir)) {
43+
throw new TypeError('Provided basedir "' + opts.basedir + '" is not a directory');
44+
}
45+
3146
if (/^(?:\.\.?(?:\/|$)|\/|([A-Za-z]:)?[/\\])/.test(x)) {
3247
var res = path.resolve(parent, x);
3348
if (x === '..' || x.slice(-1) === '/') res += '/';

‎readme.markdown

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ options are:
5959

6060
* opts.isFile - function to asynchronously test whether a file exists
6161

62+
* opts.isDirectory - function to asynchronously test whether a file exists and is a directory
63+
6264
* `opts.packageFilter(pkg, pkgfile, dir)` - transform the parsed package.json contents before looking at the "main" field
6365
* pkg - package data
6466
* pkgfile - path to package.json
@@ -94,6 +96,15 @@ default `opts` values:
9496
return cb(err);
9597
});
9698
},
99+
isDirectory: function isDirectory(dir, cb) {
100+
fs.stat(dir, function (err, stat) {
101+
if (!err) {
102+
return cb(null, stat.isDirectory());
103+
}
104+
if (err.code === 'ENOENT' || err.code === 'ENOTDIR') return cb(null, false);
105+
return cb(err);
106+
});
107+
},
97108
moduleDirectory: 'node_modules',
98109
preserveSymlinks: false
99110
}
@@ -114,6 +125,8 @@ options are:
114125

115126
* opts.isFile - function to synchronously test whether a file exists
116127

128+
* opts.isDirectory - function to synchronously test whether a file exists and is a directory
129+
117130
* `opts.packageFilter(pkg, pkgfile, dir)` - transform the parsed package.json contents before looking at the "main" field
118131
* pkg - package data
119132
* pkgfile - path to package.json
@@ -149,6 +162,15 @@ default `opts` values:
149162
}
150163
return stat.isFile() || stat.isFIFO();
151164
},
165+
isDirectory: function isDirectory(dir) {
166+
try {
167+
var stat = fs.statSync(dir);
168+
} catch (e) {
169+
if (e && (e.code === 'ENOENT' || e.code === 'ENOTDIR')) return false;
170+
throw e;
171+
}
172+
return stat.isDirectory();
173+
},
152174
moduleDirectory: 'node_modules',
153175
preserveSymlinks: false
154176
}

‎test/mock.js

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,18 @@ test('mock', function (t) {
88
var files = {};
99
files[path.resolve('/foo/bar/baz.js')] = 'beep';
1010

11+
var dirs = {};
12+
dirs[path.resolve('/foo/bar')] = true;
13+
1114
function opts(basedir) {
1215
return {
1316
basedir: path.resolve(basedir),
1417
isFile: function (file, cb) {
1518
cb(null, Object.prototype.hasOwnProperty.call(files, path.resolve(file)));
1619
},
20+
isDirectory: function (dir, cb) {
21+
cb(null, !!dirs[path.resolve(dir)]);
22+
},
1723
readFile: function (file, cb) {
1824
cb(null, files[path.resolve(file)]);
1925
}
@@ -49,12 +55,18 @@ test('mock from package', function (t) {
4955
var files = {};
5056
files[path.resolve('/foo/bar/baz.js')] = 'beep';
5157

58+
var dirs = {};
59+
dirs[path.resolve('/foo/bar')] = true;
60+
5261
function opts(basedir) {
5362
return {
5463
basedir: path.resolve(basedir),
5564
isFile: function (file, cb) {
5665
cb(null, Object.prototype.hasOwnProperty.call(files, file));
5766
},
67+
isDirectory: function (dir, cb) {
68+
cb(null, !!dirs[path.resolve(dir)]);
69+
},
5870
'package': { main: 'bar' },
5971
readFile: function (file, cb) {
6072
cb(null, files[file]);
@@ -94,12 +106,18 @@ test('mock package', function (t) {
94106
main: './baz.js'
95107
});
96108

109+
var dirs = {};
110+
dirs[path.resolve('/foo')] = true;
111+
97112
function opts(basedir) {
98113
return {
99114
basedir: path.resolve(basedir),
100115
isFile: function (file, cb) {
101116
cb(null, Object.prototype.hasOwnProperty.call(files, path.resolve(file)));
102117
},
118+
isDirectory: function (dir, cb) {
119+
cb(null, !!dirs[path.resolve(dir)]);
120+
},
103121
readFile: function (file, cb) {
104122
cb(null, files[path.resolve(file)]);
105123
}
@@ -122,12 +140,18 @@ test('mock package from package', function (t) {
122140
main: './baz.js'
123141
});
124142

143+
var dirs = {};
144+
dirs[path.resolve('/foo')] = true;
145+
125146
function opts(basedir) {
126147
return {
127148
basedir: path.resolve(basedir),
128149
isFile: function (file, cb) {
129150
cb(null, Object.prototype.hasOwnProperty.call(files, path.resolve(file)));
130151
},
152+
isDirectory: function (dir, cb) {
153+
cb(null, !!dirs[path.resolve(dir)]);
154+
},
131155
'package': { main: 'bar' },
132156
readFile: function (file, cb) {
133157
cb(null, files[path.resolve(file)]);

‎test/mock_sync.js

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,20 @@ test('mock', function (t) {
88
var files = {};
99
files[path.resolve('/foo/bar/baz.js')] = 'beep';
1010

11+
var dirs = {};
12+
dirs[path.resolve('/foo/bar')] = true;
13+
1114
function opts(basedir) {
1215
return {
1316
basedir: path.resolve(basedir),
1417
isFile: function (file) {
15-
return Object.prototype.hasOwnProperty.call(files, file);
18+
return Object.prototype.hasOwnProperty.call(files, path.resolve(file));
19+
},
20+
isDirectory: function (dir) {
21+
return !!dirs[path.resolve(dir)];
1622
},
1723
readFileSync: function (file) {
18-
return files[file];
24+
return files[path.resolve(file)];
1925
}
2026
};
2127
}
@@ -48,14 +54,20 @@ test('mock package', function (t) {
4854
main: './baz.js'
4955
});
5056

57+
var dirs = {};
58+
dirs[path.resolve('/foo')] = true;
59+
5160
function opts(basedir) {
5261
return {
5362
basedir: path.resolve(basedir),
5463
isFile: function (file) {
55-
return Object.prototype.hasOwnProperty.call(files, file);
64+
return Object.prototype.hasOwnProperty.call(files, path.resolve(file));
65+
},
66+
isDirectory: function (dir) {
67+
return !!dirs[path.resolve(dir)];
5668
},
5769
readFileSync: function (file) {
58-
return files[file];
70+
return files[path.resolve(file)];
5971
}
6072
};
6173
}

‎test/node_path.js

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,66 @@
1+
var fs = require('fs');
12
var path = require('path');
23
var test = require('tape');
34
var resolve = require('../');
45

56
test('$NODE_PATH', function (t) {
67
t.plan(4);
78

9+
var isDir = function (dir, cb) {
10+
if (dir === '/node_path' || dir === 'node_path/x') {
11+
return cb(null, true);
12+
}
13+
fs.stat(dir, function (err, stat) {
14+
if (!err) {
15+
return cb(null, stat.isDirectory());
16+
}
17+
if (err.code === 'ENOENT' || err.code === 'ENOTDIR') return cb(null, false);
18+
return cb(err);
19+
});
20+
};
21+
822
resolve('aaa', {
923
paths: [
1024
path.join(__dirname, '/node_path/x'),
1125
path.join(__dirname, '/node_path/y')
1226
],
13-
basedir: __dirname
27+
basedir: __dirname,
28+
isDirectory: isDir
1429
}, function (err, res) {
15-
t.equal(res, path.join(__dirname, '/node_path/x/aaa/index.js'));
30+
t.equal(res, path.join(__dirname, '/node_path/x/aaa/index.js'), 'aaa resolves');
1631
});
1732

1833
resolve('bbb', {
1934
paths: [
2035
path.join(__dirname, '/node_path/x'),
2136
path.join(__dirname, '/node_path/y')
2237
],
23-
basedir: __dirname
38+
basedir: __dirname,
39+
isDirectory: isDir
2440
}, function (err, res) {
25-
t.equal(res, path.join(__dirname, '/node_path/y/bbb/index.js'));
41+
t.equal(res, path.join(__dirname, '/node_path/y/bbb/index.js'), 'bbb resolves');
2642
});
2743

2844
resolve('ccc', {
2945
paths: [
3046
path.join(__dirname, '/node_path/x'),
3147
path.join(__dirname, '/node_path/y')
3248
],
33-
basedir: __dirname
49+
basedir: __dirname,
50+
isDirectory: isDir
3451
}, function (err, res) {
35-
t.equal(res, path.join(__dirname, '/node_path/x/ccc/index.js'));
52+
t.equal(res, path.join(__dirname, '/node_path/x/ccc/index.js'), 'ccc resolves');
3653
});
3754

38-
// ensure that relative paths still resolve against the
39-
// regular `node_modules` correctly
55+
// ensure that relative paths still resolve against the regular `node_modules` correctly
4056
resolve('tap', {
4157
paths: [
4258
'node_path'
4359
],
44-
basedir: 'node_path/x'
60+
basedir: 'node_path/x',
61+
isDirectory: isDir
4562
}, function (err, res) {
4663
var root = require('tap/package.json').main;
47-
t.equal(res, path.resolve(__dirname, '..', 'node_modules/tap', root));
64+
t.equal(res, path.resolve(__dirname, '..', 'node_modules/tap', root), 'tap resolves');
4865
});
4966
});

‎test/resolver.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ test('not a directory', function (t) {
356356
t.equal(res, undefined);
357357
t.equal(pkg, undefined);
358358

359-
t.equal(err && err.message, 'Cannot find module \'' + path + "' from '" + __filename + "'");
359+
t.equal(err && err.message, 'Provided basedir "' + __filename + '" is not a directory');
360360
});
361361
});
362362

‎test/resolver_sync.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ test('not a directory', function (t) {
273273
t.fail();
274274
} catch (err) {
275275
t.ok(err, 'a non-directory errors');
276-
t.equal(err && err.message, 'Cannot find module \'' + path + "' from '" + __filename + "'");
276+
t.equal(err && err.message, 'Provided basedir "' + __filename + '" is not a directory');
277277
}
278278
t.end();
279279
});

0 commit comments

Comments
 (0)