Skip to content
This repository has been archived by the owner on Aug 2, 2018. It is now read-only.

Commit

Permalink
fixing bug in local path resolve
Browse files Browse the repository at this point in the history
if you have a local dep that points to a missing file, it will attempt to resolve it
as a remote. duo-parse treats something like `../../a/b/c` as `{ repo: '..' }`. if you
also have a manifest, it will attempt to find `..` via regexp in the `dependencies`
list. since it's a regexp, `..` will match just about anything.

the end result is that you have an extra dependency being added to your build. in the
case of css, it can be hard to trace this down.

this throws when the dep looks like a path (ie: begins with "." or "/") and is not
resolved locally. I don't believe it is possible to have a github repo that begins
with either of these chars, so we should be safe here.
  • Loading branch information
dominicbarnes committed Apr 20, 2016
1 parent 70a5b22 commit 3d895cd
Show file tree
Hide file tree
Showing 4 changed files with 35 additions and 0 deletions.
13 changes: 13 additions & 0 deletions lib/duo.js
Original file line number Diff line number Diff line change
Expand Up @@ -774,6 +774,9 @@ Duo.prototype.dependency = function* (dep, file, entry) {
if (local) {
debug('%s: local dependency "%s"', file.id, dep);
return relative(entry.root, local);
} else if (isLocal(dep)) {
debug('%s: local dependency "%s" not found', file.id, dep);
throw error('unable to resolve local dependency ' + dep + ' from ' + file.id);
}

// `dep` is a remote dependency
Expand Down Expand Up @@ -1383,3 +1386,13 @@ function isSlug(str) {

return r.test(str);
}

/**
* Tells us whether or not the input path is a local path. (begins with . or /)
*
* @param {String} path
* @return {Boolean}
*/
function isLocal(path) {
return path[0] === '.' || path[0] === '/';
}
18 changes: 18 additions & 0 deletions test/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,24 @@ describe('Duo API', function () {
assert.strictEqual(ctx.a1, ctx.a2);
});

it('should fail when relative requires are not found', function* () {
try {
var js = yield build('resolve-relative-missing', 'index.js').run();
} catch (err) {
assert(err instanceof Error);
}
assert(!js, 'should have failed');
});

it('should fail when absolute requires are not found', function* () {
try {
var js = yield build('resolve-absolute-missing', 'index.js').run();
} catch (err) {
assert(err instanceof Error);
}
assert(!js, 'should have failed');
});

it('should fetch and build direct dependencies', function* () {
this.timeout(15000);
var js = yield build('simple-deps').run();
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/resolve-absolute-missing/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

module.exports = require('/lib'); // should not exist!
2 changes: 2 additions & 0 deletions test/fixtures/resolve-relative-missing/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@

module.exports = require('./lib'); // should not exist!

0 comments on commit 3d895cd

Please sign in to comment.