Skip to content

Commit

Permalink
Improve path checking before route registration (#155)
Browse files Browse the repository at this point in the history
* [147] Ensure path sanity checks provide clear errors during registration checks

* [147] Adjust tests to not include unused variables

* [147] Adjust usage of double quotes to single quotes to ensure code style

* [147] Adjust gh-148 to gh-147 mention in tests

* [147] Remove extraneous parameters in test functions, ensure all possible method verbs are tested
  • Loading branch information
peterver committed Jul 4, 2022
1 parent 4fb50ac commit 6aca720
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 6 deletions.
24 changes: 18 additions & 6 deletions lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,17 +189,22 @@ function Router(opts) {
for (let i = 0; i < methods.length; i++) {
function setMethodVerb(method) {
Router.prototype[method] = function(name, path, middleware) {
if (typeof path === "string" || path instanceof RegExp) {
if (typeof path === 'string' || path instanceof RegExp) {
middleware = Array.prototype.slice.call(arguments, 2);
} else {
middleware = Array.prototype.slice.call(arguments, 1);
path = name;
name = null;
}

this.register(path, [method], middleware, {
name: name
});
// Sanity check to ensure we have a viable path candidate (eg: string|regex|non-empty array)
if (
typeof path !== 'string' &&
!(path instanceof RegExp) &&
(!Array.isArray(path) || path.length === 0)
) throw new Error(`You have to provide a path when adding a ${method} handler`);

this.register(path, [method], middleware, {name});

return this;
};
Expand Down Expand Up @@ -498,7 +503,14 @@ Router.prototype.all = function (name, path, middleware) {
name = null;
}

this.register(path, methods, middleware, { name });
// Sanity check to ensure we have a viable path candidate (eg: string|regex|non-empty array)
if (
typeof path !== 'string' &&
!(path instanceof RegExp) &&
(!Array.isArray(path) || path.length === 0)
) throw new Error('You have to provide a path when adding an all handler');

this.register(path, methods, middleware, {name});

return this;
};
Expand Down Expand Up @@ -572,7 +584,7 @@ Router.prototype.register = function (path, methods, middleware, opts) {
name: opts.name,
sensitive: opts.sensitive || this.opts.sensitive || false,
strict: opts.strict || this.opts.strict || false,
prefix: opts.prefix || this.opts.prefix || "",
prefix: opts.prefix || this.opts.prefix || '',
ignoreCaptures: opts.ignoreCaptures
});

Expand Down
19 changes: 19 additions & 0 deletions test/lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,26 @@ describe('Router', function () {
done();
});
});

it ('correctly returns an error when not passed a path for verb-specific registration (gh-147)', function () {
const router = new Router();
for (let el of methods) {
try {
router[el](function () {});
} catch (e) {
expect(e.message).to.be(`You have to provide a path when adding a ${el} handler`);
}
}
});

it ('correctly returns an error when not passed a path for "all" registration (gh-147)', function () {
const router = new Router();
try {
router.all(function () {});
} catch (e) {
expect(e.message).to.be('You have to provide a path when adding an all handler');
}
});
});

describe('Router#use()', function () {
Expand Down

0 comments on commit 6aca720

Please sign in to comment.