diff --git a/docs/App.md b/docs/App.md index 12c087e..4bf1a0c 100644 --- a/docs/App.md +++ b/docs/App.md @@ -209,6 +209,33 @@ const app = medley(); app.createSubApp('/api').register(require('./routes')); ``` +The path in the route definition will be directly appended to the path prefix +passed to `createSubApp()`. There is no special handling of `/` characters +(except for one case described below). + +```js +const medley = require('@medley/medley'); +const app = medley(); + +const subApp = app.createSubApp('/user'); +subApp.get('/', (req, res) => {}); // Creates a route for '/user/' +// The above route will not run for: '/user'. +// The following would create a route on the sub-app for '/user': +subApp.get('', (req, res) => {}); +``` + +The only case where registering a route on a sub-app with a prefix includes +special handling for the `/` character is when the sub-app’s prefix ends +with a `/` and the route path begins with a `/`. + +```js +const medley = require('@medley/medley'); +const app = medley(); + +const subApp = app.createSubApp('/users/'); +subApp.get('/:id', (req, res) => {}); // Creates a route for '/users/:id' +``` + ### `app.decorate(name, value)` diff --git a/docs/Lifecycle.md b/docs/Lifecycle.md index 5f98a47..cb9bb38 100644 --- a/docs/Lifecycle.md +++ b/docs/Lifecycle.md @@ -33,7 +33,7 @@ Incoming Request The first step Medley takes after receiving a request is to find the route that matches the URL of the request. -Medley uses the [`find-my-way`](https://www.npmjs.com/package/find-my-way) router to make this step fast and efficient. +Medley uses [`@medley/router`](https://www.npmjs.com/package/@medley/router) for routing. ## `onRequest` Hooks @@ -80,6 +80,10 @@ See the [`Routes` documentation](Routes.md) for more information on route handle If the request URL does not match any routes, the [`notFoundHandler`](Medley.md#notfoundhandler) is invoked. Global hooks **are** run before/after this handler. +#### Method-Not-Allowed Handler + +If the request URL matches a route but has no handler for the request method, the [`methodNotAllowedHandler`](Medley.md#methodnotallowedhandler) is invoked. Global hooks **are** run before/after this handler. + ## Serialize Body In this step, the body that was passed to `res.send()` is serialized (if it needs to be) and an appropriate `Content-Type` is set (if one was not already set). diff --git a/docs/Medley.md b/docs/Medley.md index a5d9b99..a915d73 100644 --- a/docs/Medley.md +++ b/docs/Medley.md @@ -6,12 +6,11 @@ object which is used to customize the resulting instance. The options are: + [`http2`](#http2) + [`https`](#https) -+ [`maxParamLength`](#maxparamlength) ++ [`methodNotAllowedHandler`](#methodnotallowedhandler) + [`notFoundHandler`](#notfoundhandler) + [`onErrorSending`](#onerrorsending) + [`queryParser`](#queryparser) + [`server`](#server) -+ [`strictRouting`](#strictrouting) + [`trustProxy`](#trustproxy) ## Options @@ -52,18 +51,59 @@ const app = medley({ }); ``` -### `maxParamLength` +### `methodNotAllowedHandler` -Type: `number`
-Default: `100` +Type: `function(req, res)` (`req` - [Request](Request.md), `res` - [Response](Response.md)) + +A handler function that is called when the request URL matches a route but +there is no handler for the request method. + +```js +const medley = require('@medley/medley'); +const app = medley({ + methodNotAllowedHandler: (req, res) => { + res.status(405).send('Method Not Allowed: ' + req.method); + } +}); +``` + +[`405 Method Not Allowed`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405) +responses require the [`Allow`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Allow) +header to be set. To make this convenient, the [`res.config`](Response.md#resconfig) +object in the handler will contain an `allowedMethods` property, which will be an +array of the HTTP methods that have handlers for the route. + +```js +const medley = require('@medley/medley'); +const app = medley({ + methodNotAllowedHandler: (req, res) => { + res.config.allowedMethods; // ['GET', 'HEAD', 'POST'] + res.setHeader('allow', res.config.allowedMethods.join()); + res.status(405).send('Method Not Allowed: ' + req.method); + } +}); + +app.get('/users', (req, res) => { /* ... */ }); +app.post('/users', (req, res) => { /* ... */ }); +``` + +While this handler is intended to be used to send a [`405 Method Not Allowed`](https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/405) +response, it could be used to send a `404 Not Found` response instead. + +```js +const medley = require('@medley/medley'); -This option sets a limit on the number of characters in the parameters of -parametric (standard, regex, and multi-parametric) routes. +function notFoundHandler(req, res) { + res.status(404).send('Not Found: ' + req.url); +} -This can be useful to protect against [DoS attacks](https://www.owasp.org/index.php/Regular_expression_Denial_of_Service_-_ReDoS) -for routes with regex parameters. +const app = medley({ + notFoundHandler, + methodNotAllowedHandler: notFoundHandler, +}); +``` -*If the maximum length limit is reached, the request will not match the route.* +[Hooks](Hooks.md) that are added to the root `app` will run before/after the `methodNotAllowedHandler`. ### `notFoundHandler` @@ -75,7 +115,7 @@ A handler function that is called when no routes match the request URL. const medley = require('@medley/medley'); const app = medley({ notFoundHandler: (req, res) => { - res.status(404).send('Route Not Found'); + res.status(404).send('Not Found: ' + req.url); } }); ``` @@ -138,38 +178,6 @@ const server = http.createServer() const app = medley({ server }); ``` -### `strictRouting` - -Default: `false` - -Enables strict routing. When `true`, the router treats "/foo" and "/foo/" as -different. Otherwise, the router treats "/foo" and "/foo/" as the same. - -```js -const medley = require('@medley/medley'); -const app = medley({ strictRouting: false }); - -// Registers both "/foo" and "/foo/" -app.get('/foo/', (req, res) => { - res.send('foo'); -}); - -// Registers both "/bar" and "/bar/" -app.get('/bar', (req, res) => { - res.send('bar'); -}); - -const strictApp = medley({ strictRouting: true }); - -strictApp.get('/foo', (req, res) => { - res.send('foo'); -}); - -strictApp.get('/foo/', (req, res) => { - res.send('different foo'); -}); -``` - ### `trustProxy` Default: `false` diff --git a/docs/Routes.md b/docs/Routes.md index 82a9293..e03139d 100644 --- a/docs/Routes.md +++ b/docs/Routes.md @@ -22,9 +22,7 @@ app.route(options) Type: `string` | `Array` -The HTTP method(s) the route will run for. Can be any method found in the -[`http.METHODS`](https://nodejs.org/api/http.html#http_http_methods) array -(except for `CONNECT`). +The HTTP method(s) the route will run for. ```js app.route({ @@ -55,12 +53,12 @@ app.route({ method: 'GET', path: '/user/:id', handler: function(req, res) { - console.log(req.params.id); // '1003' (for example) + console.log(req.url); // '/user/1003' (for example) } }); ```` -See the [URL-Building](#url-building) section for details on the formats +See the [**Route Path Formats**](#route-path-formats) section for details on the formats the `path` option can take. #### `handler` (required) @@ -186,7 +184,7 @@ app.patch(path, [options,] handler) app.delete(path, [options,] handler) app.options(path, [options,] handler) -// Registers a route that handles all supported methods +// Registers a route that handles all methods in `require('http').METHODS` app.all(path, [options,] handler) ``` @@ -245,64 +243,91 @@ app.get('/path', { *If the `handler` is specified in both the `options` object and as the third parameter, the third parameter will take precedence.* -## URL-Building +## Route Path Formats -Medley supports any route paths supported by -[`find-my-way`](https://github.com/delvedor/find-my-way). +Medley supports any route path format supported by [`@medley/router`](https://github.com/@medley/router). -URL parameters are specified with a colon (`:`) before the parameter -name, and wildcard paths use an asterisk (`*`). +The path formats are as follows: -_Note that static routes are always checked before parametric and wildcard routes._ +### 1. Static -```js -// Static -app.get('/api/user', (req, res) => {}); - -// Parametric -app.get('/api/:userId', (req, res) => {}); -app.get('/api/:userId/:secretToken', (req, res) => {}); +Static routes match exactly the path provided. -// Wildcard -app.get('/api/*', (req, res) => {}); +``` +/ +/about +/api/login ``` -Regular expression routes are also supported, but be aware that they are -expensive in terms of performance. +### 2. Parametric -```js -// Parametric with regex -app.get('/api/:file(^\\d+).png', (req, res) => {}); +Path segments that begin with a `:` denote a parameter and will match anything +up to the next `/` or to the end of the path. + +``` +/users/:userID +/users/:userID/posts +/users/:userID/posts/:postID ``` -To define a path with more than one parameter within the same path part, -a hyphen (`-`) can be used to separate the parameters: +Everything after the `:` character will be the name of the parameter in the +`req.params` object. ```js -// Multi-parametric -app.get('/api/near/:lat-:lng/radius/:r', (req, res) => { - // Matches: '/api/near/10.856-32.284/radius/50' - req.params // { lat: '10.856', lng: '32.284', r: '50' } +app.get('/users/:userID', (req, res) => { + // Request URL: /users/100 + console.log(req.params); // { userID: '100' } }); ``` -Multiple parameters also work with regular expressions: +If multiple routes have a parameter in the same part of the route, the +parameter names must be the same. For example, registering the following two +routes would be an error because the `:id` and `:userID` parameters conflict +with each other: + +``` +/users/:id +/users/:userID/posts +``` + +Parameters may start anywhere in the path. For example, the following are valid routes: ```js -app.get('/api/at/:hour(^\\d{2})h:minute(^\\d{2})m', (req, res) => { - // Matches: '/api/at/02h:50m' - req.params // { hour: '02', minute: '50' } -}); +'/api/v:version' // Matches '/api/v1' +'/on-:event' // Matches '/on-click' +``` + +### 3. Wildcard + +Routes that end with a `*` are wildcard routes. The `*` will match any +characters in the rest of the path, including `/` characters or no characters. + +For example, the following route: + +``` +/static/* ``` -In this case, the parameter separator can be any character that is not -matched by the regular expression. +will match all of these URLs: + +``` +/static/ +/static/favicon.ico +/static/js/main.js +/static/css/vendor/bootstrap.css +``` -Having a route with multiple parameters may affect negatively the performance, -so prefer the single parameter approach whenever possible. +The wildcard value will be set in the route `params` object with `'*'` as the key. -For more information on the router used by Medley, check out -[`find-my-way`](https://github.com/delvedor/find-my-way). +```js +app.get('/static/*', (req, res) => { + if (req.url === '/static/favicon.ico') { + console.log(req.params); // { '*': 'favicon.ico' } + } else if (req.url === '/static/') { + console.log(req.params); // { '*': '' } + } +}); +``` ## Async-Await / Promises diff --git a/lib/RequestHandlers.js b/lib/RequestHandlers.js index 9532f70..1ca5446 100644 --- a/lib/RequestHandlers.js +++ b/lib/RequestHandlers.js @@ -4,15 +4,15 @@ const compileJSONStringify = require('compile-json-stringify') const {runOnRequestHooks, runOnErrorHooks} = require('./HookRunners') const {STATUS_CODES} = require('http') -function createRequestHandler(router, notFoundRoute, Request, Response) { +function createRequestHandler(router, notFoundContext, Request, Response) { return function requestHandler(req, res) { - var queryIndex = req.url.indexOf('?') // find-my-way needs the query string removed - var url = queryIndex >= 0 ? req.url.slice(0, queryIndex) : req.url + var route = router.find(req.url) + var routeContext = route === null + ? notFoundContext + : route.store.methodContexts[req.method] || route.store.fallbackContext + var params = route === null ? {} : route.params - var route = router.find(req.method, url, undefined) || notFoundRoute - - var routeContext = route.store - var request = new Request(req, req.headers, route.params) + var request = new Request(req, req.headers, params) var response = new Response(res, request, routeContext) if (routeContext.onFinishedHooks !== null) { @@ -67,20 +67,18 @@ function onHandlerReject(err) { runOnErrorHooks(err, this) } -function createOptionsHandler(allowedMethods) { - return function autoOptionsHandler(req, res) { - res.statusCode = 200 - res.headers.allow = allowedMethods - res.send(allowedMethods) - } +function defaultOptionsHandler(req, res) { + const allowedMethods = res.config.allowedMethods.join() + + res.statusCode = 200 + res.headers.allow = allowedMethods + res.send(allowedMethods) } -function create405Handler(allowedMethods) { - return function auto405Handler(req, res) { - res.statusCode = 405 - res.headers.allow = allowedMethods - res.send(req.method === 'HEAD' ? null : `Method Not Allowed: ${req.method} ${req.url}`) - } +function defaultMethodNotAllowedHandler(req, res) { + res.statusCode = 405 + res.headers.allow = res.config.allowedMethods.join() + res.send(req.method === 'HEAD' ? null : `Method Not Allowed: ${req.method} ${req.url}`) } function defaultNotFoundHandler(req, res) { @@ -142,8 +140,8 @@ function buildErrorBody(err, statusCode) { module.exports = { createRequestHandler, - createOptionsHandler, - create405Handler, + defaultOptionsHandler, + defaultMethodNotAllowedHandler, defaultNotFoundHandler, defaultErrorHandler, finalErrorHandler, diff --git a/medley.js b/medley.js index 02a775f..75733bb 100644 --- a/medley.js +++ b/medley.js @@ -1,11 +1,11 @@ 'use strict' -const findMyWay = require('find-my-way') const debug = require('debug')('medley') const http = require('http') const Hooks = require('./lib/Hooks') const RouteContext = require('./lib/RouteContext') +const Router = require('@medley/router') const runOnCloseHandlers = require('./lib/utils/runOnCloseHandlers') const runOnLoadHandlers = require('./lib/utils/runOnLoadHandlers') @@ -15,16 +15,11 @@ const {buildResponse} = require('./lib/Response') const {buildSerializers} = require('./lib/Serializer') const { createRequestHandler, - createOptionsHandler, - create405Handler, + defaultOptionsHandler, + defaultMethodNotAllowedHandler, defaultNotFoundHandler, } = require('./lib/RequestHandlers') -const supportedMethods = http.METHODS.filter(method => method !== 'CONNECT') - -/* istanbul ignore next - This is never used. It's just needed to appease find-my-way. */ -const noop = () => {} - function defaultOnErrorSending(err) { debug('Error occurred while sending:\n%O', err) } @@ -85,9 +80,18 @@ function medley(options) { throw new TypeError(`'notFoundHandler' option must be a function. Got value of type '${typeof options.notFoundHandler}'`) } - const router = findMyWay({ - ignoreTrailingSlash: !options.strictRouting, - maxParamLength: options.maxParamLength, + if ( + options.methodNotAllowedHandler !== undefined && + typeof options.methodNotAllowedHandler !== 'function' + ) { + throw new TypeError(`'methodNotAllowedHandler' option must be a function. Got value of type '${typeof options.methodNotAllowedHandler}'`) + } + + const router = new Router({ + storeFactory: () => ({ + methodContexts: Object.create(null), + fallbackContext: null, // For 405 Method Not Allowed responses + }), }) const rootAppHooks = new Hooks() const onErrorSending = options.onErrorSending || defaultOnErrorSending @@ -95,18 +99,14 @@ function medley(options) { null, // Serializers options.notFoundHandler || defaultNotFoundHandler, undefined, // config - null, // preHandler + undefined, // preHandler rootAppHooks, onErrorSending ) - const notFoundRoute = { - handler: noop, // To match shape of find-my-way routes - params: {}, - store: notFoundRouteContext, - } const Request = buildRequest(!!options.trustProxy, options.queryParser) const Response = buildResponse() - const requestHandler = createRequestHandler(router, notFoundRoute, Request, Response) + const requestHandler = createRequestHandler(router, notFoundRouteContext, Request, Response) + const methodNotAllowedHandler = options.methodNotAllowedHandler || defaultMethodNotAllowedHandler var loadCallbackQueue = null var loaded = false @@ -139,7 +139,9 @@ function medley(options) { put: createShorthandRouteMethod('PUT'), patch: createShorthandRouteMethod('PATCH'), options: createShorthandRouteMethod('OPTIONS'), - all: createShorthandRouteMethod(supportedMethods), + all: createShorthandRouteMethod( + http.METHODS.filter(method => method !== 'CONNECT') + ), [Symbol.iterator]: routesIterator, @@ -173,8 +175,6 @@ function medley(options) { const onLoadHandlers = [] const onCloseHandlers = [] - var registeringAutoHandlers = false - function throwIfAppIsLoaded(msg) { if (loaded) { throw new Error(msg) @@ -254,12 +254,10 @@ function medley(options) { throw new TypeError('Route `method` is required') } - const methods = Array.isArray(opts.method) ? opts.method : [opts.method] - - for (const method of methods) { - if (supportedMethods.indexOf(method) === -1) { - throw new RangeError(`"${method}" method is not supported`) - } + if (typeof opts.handler !== 'function') { + throw new TypeError( + `Route 'handler' must be a function. Got a value of type '${typeof opts.handler}': ${opts.handler}` + ) } let {path} = opts @@ -274,91 +272,77 @@ function medley(options) { path = this._routePrefix + path } - if (typeof opts.handler !== 'function') { - throw new TypeError( - `Route 'handler' must be a function. Got a value of type '${typeof opts.handler}': ${opts.handler}` + const routeStore = router.register(path) + const appRouteContexts = routeContexts.get(this) || routeContexts.set(this, []).get(this) + + if (routeStore.fallbackContext === null) { // Initial setup for new route store + const config = {allowedMethods: []} + + // 405 Method Not Allowed context + const fallbackContext = RouteContext.create( + null, // serializers + methodNotAllowedHandler, + config, + undefined, // preHandler + rootAppHooks, + onErrorSending + ) + routeStore.fallbackContext = fallbackContext + routeContexts.get(app).push(fallbackContext) // This is a root app context + + // Default OPTIONS context + const optionsContext = RouteContext.create( + null, // serializers + defaultOptionsHandler, + config, + undefined, // preHandler + this._hooks, + onErrorSending ) + routeStore.methodContexts.OPTIONS = optionsContext + appRouteContexts.push(optionsContext) } - const serializers = buildSerializers(opts.responseSchema) const routeContext = RouteContext.create( - serializers, + buildSerializers(opts.responseSchema), opts.handler, opts.config, opts.preHandler, this._hooks, onErrorSending ) + appRouteContexts.push(routeContext) - router.on(methods, path, noop, routeContext) - - if (!registeringAutoHandlers) { - recordRoute(path, methods, routeContext, this) - - const appRouteContexts = routeContexts.get(this) - if (appRouteContexts === undefined) { - routeContexts.set(this, [routeContext]) - } else { - appRouteContexts.push(routeContext) - } - } - - return this // Chainable - } - - function recordRoute(routePath, methods, routeContext, appInstance) { - const methodRoutes = {} - for (var i = 0; i < methods.length; i++) { - methodRoutes[methods[i]] = routeContext - } - - if (!routes.has(routePath)) { - routes.set(routePath, {appInstance, methodRoutes}) - return - } - - const routeData = routes.get(routePath) - Object.assign(routeData.methodRoutes, methodRoutes) - } - - function registerAutoHandlers() { - for (const [routePath, routeData] of routes) { - const {methodRoutes} = routeData - const methods = Object.keys(methodRoutes) + const methods = Array.isArray(opts.method) ? opts.method : [opts.method] + const userMethodContexts = routes.get(path) || routes.set(path, {}).get(path) + const {allowedMethods} = routeStore.fallbackContext.config - // Create a HEAD handler if a GET handler was set and a HEAD handler wasn't - if (methodRoutes.GET !== undefined && methodRoutes.HEAD === undefined) { - router.on('HEAD', routePath, noop, methodRoutes.GET) - methods.push('HEAD') + for (const method of methods) { + // Throw if a context for the route + method already exists, unless it's + // the default HEAD or OPTIONS context (in which case, replace it) + const existingContext = routeStore.methodContexts[method] + if ( + existingContext !== undefined && + (method !== 'HEAD' || existingContext !== routeStore.methodContexts.GET) && + (method !== 'OPTIONS' || existingContext.handler !== defaultOptionsHandler) + ) { + throw new Error(`Cannot create route "${method} ${path}" because it already exists`) } - methods.sort() // For consistent Allow headers + routeStore.methodContexts[method] = routeContext + userMethodContexts[method] = routeContext - // Create an OPTIONS handler if one wasn't set - const optionsIndex = methods.indexOf('OPTIONS') - if (optionsIndex === -1) { - const optionsHandler = createOptionsHandler(methods.join(',')) - routeData.appInstance.options(routePath, optionsHandler) - } else { - // Remove OPTIONS for the next part - methods.splice(optionsIndex, 1) + if (method === 'GET' && routeStore.methodContexts.HEAD === undefined) { + routeStore.methodContexts.HEAD = routeContext // Set default HEAD route + allowedMethods.push('GET', 'HEAD') + } else if (method !== 'HEAD' || !allowedMethods.includes('HEAD')) { + allowedMethods.push(method) } + } - // Create a 405 handler for all unset, supported methods - const unsetMethods = supportedMethods.filter( - method => method !== 'OPTIONS' && methods.indexOf(method) === -1 - ) - if (unsetMethods.length > 0) { - routeData.appInstance.route({ - method: unsetMethods, - path: routePath, - handler: create405Handler(methods.join(',')), - }) - } + allowedMethods.sort() // Keep the allowed methods sorted to ensure consistency - // Try to save memory since this is no longer needed - routeData.appInstance = null - } + return this // Chainable } function onLoad(handler) { @@ -398,9 +382,6 @@ function medley(options) { return } - registeringAutoHandlers = true - registerAutoHandlers() - loaded = true // Hooks can be added after a route context is created, so update @@ -499,8 +480,8 @@ function medley(options) { } function *routesIterator() { - for (const [routePath, {methodRoutes}] of routes) { - yield [routePath, methodRoutes] + for (const [routePath, methodContexts] of routes) { + yield [routePath, methodContexts] } } } diff --git a/package.json b/package.json index ff16bcf..fe1ae97 100644 --- a/package.json +++ b/package.json @@ -40,11 +40,11 @@ "timeout": 10 }, "dependencies": { + "@medley/router": "^0.2.1", "compile-json-stringify": "^0.1.2", "debug": "^4.1.1", "destroy": "^1.0.4", - "end-of-stream": "^1.4.1", - "find-my-way": "2.1.0" + "end-of-stream": "^1.4.1" }, "devDependencies": { "JSONStream": "^1.3.5", diff --git a/test/404s.test.js b/test/404s.test.js index 8eb151d..5ed5f4f 100644 --- a/test/404s.test.js +++ b/test/404s.test.js @@ -1,13 +1,10 @@ 'use strict' const {test} = require('tap') -const http = require('http') const h2url = require('h2url') const request = require('./utils/request') const medley = require('..') -const supportedMethods = http.METHODS.filter(method => method !== 'CONNECT') - test('Default 404 handler', (t) => { t.plan(4) @@ -61,34 +58,34 @@ test('Custom 404 handler - invalid type', (t) => { t.end() }) -test('The default 404 handler runs for all supported HTTP methods', (t) => { - t.plan(4 * supportedMethods.length) +test('The default 404 handler runs for requests with a non-standard method', (t) => { + t.plan(4) - const app = medley() + const app = medley({http2: true}) app.all('/', () => { t.fail('Handler should not be called') }) - for (const method of supportedMethods) { - request(app, {method, url: '/not-found'}, (err, res) => { - t.error(err) - t.equal(res.statusCode, 404) - t.equal(res.headers['content-type'], 'text/plain; charset=utf-8') - - if (method === 'HEAD') { - t.equal(res.body, '') - } else { - t.equal(res.body, `Not Found: ${method} /not-found`) - } + app.listen(0, 'localhost', async (err) => { + app.server.unref() + t.error(err) + + const res = await h2url.concat({ + method: 'NONSTANDARD', + url: `http://localhost:${app.server.address().port}/not-found`, }) - } + t.equal(res.headers[':status'], 404) + t.equal(res.headers['content-type'], 'text/plain; charset=utf-8') + t.equal(res.body, 'Not Found: NONSTANDARD /not-found') + }) }) -test('Custom 404 handlers run for all supported HTTP methods', (t) => { - t.plan(4 * supportedMethods.length) +test('Custom 404 handler runs for requests with a non-standard method', (t) => { + t.plan(4) const app = medley({ + http2: true, notFoundHandler: (req, res) => { res.status(404).send(`Custom Not Found: ${req.method} ${req.url}`) }, @@ -98,19 +95,18 @@ test('Custom 404 handlers run for all supported HTTP methods', (t) => { t.fail('Handler should not be called') }) - for (const method of supportedMethods) { - request(app, {method, url: '/not-found'}, (err, res) => { - t.error(err) - t.equal(res.statusCode, 404) - t.equal(res.headers['content-type'], 'text/plain; charset=utf-8') - - if (method === 'HEAD') { - t.equal(res.body, '') - } else { - t.equal(res.body, `Custom Not Found: ${method} /not-found`) - } + app.listen(0, 'localhost', async (err) => { + app.server.unref() + t.error(err) + + const res = await h2url.concat({ + method: 'NONSTANDARD', + url: `http://localhost:${app.server.address().port}/not-found`, }) - } + t.equal(res.headers[':status'], 404) + t.equal(res.headers['content-type'], 'text/plain; charset=utf-8') + t.equal(res.body, 'Custom Not Found: NONSTANDARD /not-found') + }) }) test('Hooks on the root app run for the default 404 handler', (t) => { @@ -236,56 +232,3 @@ test('not-found route lookups do not fail with the Accept-Version header', (t) = t.equal(res.body, 'not found') }) }) - -test('Sends a 404 for methods not supported by find-my-way', (t) => { - t.plan(5) - - const app = medley({http2: true}) - - app.all('/', () => { - t.fail('The handler should not be called') - }) - - app.listen(0, 'localhost', async (err) => { - app.server.unref() - t.error(err) - - const res = await h2url.concat({ - method: 'TROLL', - url: `http://localhost:${app.server.address().port}/`, - }) - t.equal(res.headers[':status'], 404) - t.equal(res.headers['content-type'], 'text/plain; charset=utf-8') - t.equal(res.headers['content-length'], '18') - t.equal(res.body, 'Not Found: TROLL /') - }) -}) - -test('Calls the custom 404 handler for methods not supported by find-my-way', (t) => { - t.plan(5) - - const app = medley({ - http2: true, - notFoundHandler: (req, res) => { - res.status(404).send(req.method + ' not found') - }, - }) - - app.all('/', () => { - t.fail('The handler should not be called') - }) - - app.listen(0, 'localhost', async (err) => { - app.server.unref() - t.error(err) - - const res = await h2url.concat({ - method: 'TROLL', - url: `http://localhost:${app.server.address().port}/`, - }) - t.equal(res.headers[':status'], 404) - t.equal(res.headers['content-type'], 'text/plain; charset=utf-8') - t.equal(res.headers['content-length'], '15') - t.equal(res.body, 'TROLL not found') - }) -}) diff --git a/test/405.test.js b/test/405.test.js index d9c94cc..9dc79ec 100644 --- a/test/405.test.js +++ b/test/405.test.js @@ -1,10 +1,11 @@ 'use strict' const t = require('tap') +const h2url = require('h2url') const medley = require('..') const request = require('./utils/request') -t.test('auto 405 response for unset methods', (t) => { +t.test('Default 405 response for unset methods', (t) => { t.plan(10) const app = medley() @@ -36,7 +37,7 @@ t.test('auto 405 response for unset methods', (t) => { }) }) -t.test('auto 405 response for non-GET/HEAD routes', (t) => { +t.test('Default 405 response for non-GET/HEAD routes', (t) => { t.plan(14) const app = medley() @@ -80,7 +81,46 @@ t.test('auto 405 response for non-GET/HEAD routes', (t) => { }) }) -t.test('hooks run on auto 405 response', (t) => { +t.test('Sends a 405 response for unset methods that are plain object properties', (t) => { + t.plan(9) + + const app = medley({http2: true}) + + app.get('/', (req, res) => { + res.send('hello') + }) + + app.listen(0, 'localhost', async (err) => { + app.server.unref() + t.error(err) + + const url = `http://localhost:${app.server.address().port}/` + + { + const res = await h2url.concat({ + method: 'toString', + url, + }) + t.equal(res.headers[':status'], 405) + t.equal(res.headers['content-type'], 'text/plain; charset=utf-8') + t.equal(res.headers.allow, 'GET,HEAD') + t.equal(res.body, 'Method Not Allowed: toString /') + } + + { + const res = await h2url.concat({ + method: '__proto__', + url, + }) + t.equal(res.headers[':status'], 405) + t.equal(res.headers['content-type'], 'text/plain; charset=utf-8') + t.equal(res.headers.allow, 'GET,HEAD') + t.equal(res.body, 'Method Not Allowed: __proto__ /') + } + }) +}) + +t.test('Hooks run on default 405 response', (t) => { t.plan(16) const app = medley() @@ -124,3 +164,202 @@ t.test('hooks run on auto 405 response', (t) => { t.equal(res.body, 'Method Not Allowed: POST /?foo=bar') }) }) + +t.test('Sub-app hooks should *not* run for 405 responses', (t) => { + t.plan(8) + + const app = medley() + + app.createSubApp() + .addHook('onRequest', () => { + t.fail('Sub-app hooks should not be called') + }) + .addHook('onSend', () => { + t.fail('Sub-app hooks should not be called') + }) + .addHook('onFinished', () => { + t.fail('Sub-app hooks should not be called') + }) + .get('/', (req, res) => { + res.send('GET response') + }) + + app.createSubApp() + .addHook('onRequest', () => { + t.fail('Sub-app hooks should not be called') + }) + .addHook('onSend', () => { + t.fail('Sub-app hooks should not be called') + }) + .addHook('onFinished', () => { + t.fail('Sub-app hooks should not be called') + }) + .post('/user', (req, res) => { + res.send('POST response') + }) + + request(app, { + method: 'DELETE', + url: '/', + }, (err, res) => { + t.error(err) + t.equal(res.statusCode, 405) + t.equal(res.headers.allow, 'GET,HEAD') + t.equal(res.body, 'Method Not Allowed: DELETE /') + }) + + request(app, { + method: 'PUT', + url: '/user', + }, (err, res) => { + t.error(err) + t.equal(res.statusCode, 405) + t.equal(res.headers.allow, 'POST') + t.equal(res.body, 'Method Not Allowed: PUT /user') + }) +}) + +t.test('Default 405 handler runs for requests with a non-standard method', (t) => { + t.plan(5) + + const app = medley({http2: true}) + + app.get('/', () => { + t.fail('The handler should not be called') + }) + + app.listen(0, 'localhost', async (err) => { + app.server.unref() + t.error(err) + + const res = await h2url.concat({ + method: 'NONSTANDARD', + url: `http://localhost:${app.server.address().port}/`, + }) + t.equal(res.headers[':status'], 405) + t.equal(res.headers['content-type'], 'text/plain; charset=utf-8') + t.equal(res.headers.allow, 'GET,HEAD') + t.equal(res.body, 'Method Not Allowed: NONSTANDARD /') + }) +}) + +t.test('Custom 405 handler - invalid type', (t) => { + t.throws( + () => medley({methodNotAllowedHandler: null}), + new TypeError("'methodNotAllowedHandler' option must be a function. Got value of type 'object'") + ) + t.throws( + () => medley({methodNotAllowedHandler: true}), + new TypeError("'methodNotAllowedHandler' option must be a function. Got value of type 'boolean'") + ) + t.throws( + () => medley({methodNotAllowedHandler: 'str'}), + new TypeError("'methodNotAllowedHandler' option must be a function. Got value of type 'string'") + ) + t.end() +}) + +t.test('Custom 405 Method Not Allowed handler', (t) => { + t.plan(4) + + const app = medley({ + methodNotAllowedHandler: (req, res) => { + res.headers.allow = res.config.allowedMethods.join(',') + res.status(405).send('method not allowed') + }, + }) + + app.get('/', () => { + t.fail('the handler should not be called') + }) + + request(app, { + method: 'POST', + url: '/', + }, (err, res) => { + t.error(err) + t.equal(res.statusCode, 405) + t.equal(res.headers.allow, 'GET,HEAD') + t.equal(res.body, 'method not allowed') + }) +}) + +t.test('Hooks run on custom 405 handler', (t) => { + t.plan(16) + + const app = medley({ + methodNotAllowedHandler: (req, res) => { + res.headers.allow = res.config.allowedMethods.join(',') + res.status(405).send(`Not allowed: ${req.method} ${req.url}`) + }, + }) + + app.addHook('onRequest', (req, res, next) => { + t.deepEqual(req.query, {foo: 'bar'}) + next() + }) + + app.get('/', (req, res) => { + res.send({hello: 'world'}) + }) + + app.addHook('onSend', (req, res, body, next) => { + t.deepEqual(req.query, {foo: 'bar'}) + next() + }) + + app.addHook('onFinished', (req, res) => { + t.deepEqual(req.query, {foo: 'bar'}) + t.equal(res.headersSent, true) + }) + + request(app, { + method: 'DELETE', + url: '/?foo=bar', + }, (err, res) => { + t.error(err) + t.equal(res.statusCode, 405) + t.equal(res.headers.allow, 'GET,HEAD') + t.equal(res.body, 'Not allowed: DELETE /?foo=bar') + }) + + request(app, { + method: 'POST', + url: '/?foo=bar', + }, (err, res) => { + t.error(err) + t.equal(res.statusCode, 405) + t.equal(res.headers.allow, 'GET,HEAD') + t.equal(res.body, 'Not allowed: POST /?foo=bar') + }) +}) + +t.test('Calls the custom 405 handler for non-standard methods', (t) => { + t.plan(5) + + const app = medley({ + http2: true, + methodNotAllowedHandler: (req, res) => { + res.headers.allow = res.config.allowedMethods.join(',') + res.status(405).send(req.method + ' method not allowed') + }, + }) + + app.get('/', () => { + t.fail('The handler should not be called') + }) + + app.listen(0, 'localhost', async (err) => { + app.server.unref() + t.error(err) + + const res = await h2url.concat({ + method: 'NONSTANDARD', + url: `http://localhost:${app.server.address().port}/`, + }) + t.equal(res.headers[':status'], 405) + t.equal(res.headers['content-type'], 'text/plain; charset=utf-8') + t.equal(res.headers.allow, 'GET,HEAD') + t.equal(res.body, 'NONSTANDARD method not allowed') + }) +}) diff --git a/test/createSubApp.test.js b/test/createSubApp.test.js index ec74864..58a4a64 100644 --- a/test/createSubApp.test.js +++ b/test/createSubApp.test.js @@ -72,19 +72,19 @@ t.test('.createSubApp() creates different sub-apps that can both define routes', }) }) -t.test('.createSubApp() nested calls with prefix', (t) => { +t.test('.createSubApp() nested calls with prefix (route does not use slash)', (t) => { t.plan(4) const app = medley() const subApp = app.createSubApp('/parent') subApp.createSubApp('/child1') - .get('/', (req, res) => { + .get('', (req, res) => { res.send('child 1') }) subApp.createSubApp('/child2') - .get('/', (req, res) => { + .get('', (req, res) => { res.send('child 2') }) @@ -98,3 +98,30 @@ t.test('.createSubApp() nested calls with prefix', (t) => { t.equal(res.body, 'child 2') }) }) + +t.test('.createSubApp() nested calls with prefix (route uses slash)', (t) => { + t.plan(4) + + const app = medley() + const subApp = app.createSubApp('/parent') + + subApp.createSubApp('/child1') + .get('/', (req, res) => { + res.send('child 1') + }) + + subApp.createSubApp('/child2') + .get('/', (req, res) => { + res.send('child 2') + }) + + request(app, '/parent/child1/', (err, res) => { + t.error(err) + t.equal(res.body, 'child 1') + }) + + request(app, '/parent/child2/', (err, res) => { + t.error(err) + t.equal(res.body, 'child 2') + }) +}) diff --git a/test/hooks-onError.test.js b/test/hooks-onError.test.js index 0175664..44a0a40 100644 --- a/test/hooks-onError.test.js +++ b/test/hooks-onError.test.js @@ -178,7 +178,7 @@ test('encapsulated onError hook', (t) => { }) }) - request(app, '/test', (err, res) => { + request(app, '/test/', (err, res) => { t.error(err) t.strictEqual(res.statusCode, 500) t.strictEqual(res.headers['content-type'], 'text/plain; charset=utf-8') @@ -253,7 +253,7 @@ test('async onError hook can send a response by returning a value', (t) => { t.strictEqual(res.body, 'Error: promise error') }) - request(app, '/async', (err, res) => { + request(app, '/async/', (err, res) => { t.error(err) t.strictEqual(res.statusCode, 500) t.strictEqual(res.headers['content-type'], 'text/plain; charset=utf-8') diff --git a/test/http-methods/options.test.js b/test/http-methods/options.test.js index 8a74f98..199e53b 100644 --- a/test/http-methods/options.test.js +++ b/test/http-methods/options.test.js @@ -4,7 +4,7 @@ const t = require('tap') const medley = require('../..') const request = require('../utils/request') -t.test('auto OPTIONS response', (t) => { +t.test('Default OPTIONS response', (t) => { t.plan(12) const app = medley() @@ -56,8 +56,8 @@ t.test('auto OPTIONS response', (t) => { }) }) -t.test('hooks run on auto OPTIONS response', (t) => { - t.plan(8) +t.test('Hooks run on default OPTIONS response', (t) => { + t.plan(20) const app = medley() @@ -77,9 +77,26 @@ t.test('hooks run on auto OPTIONS response', (t) => { }) app.delete('/', (req, res) => { - res.send({hello: 'world'}) + res.send('DELETE response') }) + app.createSubApp() + .addHook('onRequest', (req, res, next) => { + t.deepEqual(req.query, {foo: 'asd'}) + next() + }) + .addHook('onSend', (req, res, body, next) => { + t.deepEqual(req.query, {foo: 'asd'}) + next() + }) + .addHook('onFinished', (req, res) => { + t.deepEqual(req.query, {foo: 'asd'}) + t.equal(res.headersSent, true) + }) + .post('/user', (req, res) => { + res.send('POST response') + }) + request(app, { method: 'OPTIONS', url: '/?foo=asd', @@ -89,4 +106,14 @@ t.test('hooks run on auto OPTIONS response', (t) => { t.equal(res.headers.allow, 'DELETE') t.equal(res.body, 'DELETE') }) + + request(app, { + method: 'OPTIONS', + url: '/user?foo=asd', + }, (err, res) => { + t.error(err) + t.equal(res.statusCode, 200) + t.equal(res.headers.allow, 'POST') + t.equal(res.body, 'POST') + }) }) diff --git a/test/route-prefix.test.js b/test/route-prefix.test.js index 011a433..a2ed6e8 100644 --- a/test/route-prefix.test.js +++ b/test/route-prefix.test.js @@ -26,31 +26,10 @@ test('route without a prefix', (t) => { }) }) -test('prefix joined with "/" route path when strictRouting=false', (t) => { +test('prefix joined with "" and "/" route path', (t) => { t.plan(4) - const app = medley({strictRouting: false}) - - app.createSubApp('/v1') - .get('/', (req, res) => { - res.send('payload') - }) - - request(app, '/v1', (err, res) => { - t.error(err) - t.equal(res.body, 'payload') - }) - - request(app, '/v1/', (err, res) => { - t.error(err) - t.equal(res.body, 'payload') - }) -}) - -test('prefix joined with "" and "/" route path when strictRouting=true', (t) => { - t.plan(4) - - const app = medley({strictRouting: true}) + const app = medley() app.createSubApp('/v1') .get('', (req, res) => { @@ -211,7 +190,7 @@ test('prefix works many levels deep', (t) => { .createSubApp('/v2') .createSubApp() // No prefix on this level .createSubApp('/v3') - .get('/', (req, res) => { + .get('', (req, res) => { res.send('payload') }) diff --git a/test/route.test.js b/test/route.test.js index 4f23ac6..010a922 100644 --- a/test/route.test.js +++ b/test/route.test.js @@ -14,26 +14,6 @@ test('.route() should throw on missing method', (t) => { t.end() }) -test('.route() should throw on unsupported method', (t) => { - const app = medley() - - t.throws( - () => app.route({method: 'TROLL', path: '/', handler: _ => _}), - new RangeError('"TROLL" method is not supported') - ) - t.end() -}) - -test('.route() should throw if one method in an array is unsupported', (t) => { - const app = medley() - - t.throws( - () => app.route({method: ['GET', 'TROLL'], path: '/', handler: _ => _}), - new RangeError('"TROLL" method is not supported') - ) - t.end() -}) - test('Should throw on missing handler', (t) => { const app = medley() @@ -70,14 +50,34 @@ test('.route() throws if path is not a string', (t) => { t.end() }) -test('.route() should throw on multiple assignment to the same route', (t) => { +test('.route() should throw if the route was already defined', (t) => { const app = medley() app.get('/', _ => _) t.throws( () => app.get('/', _ => _), - new Error("Method 'GET' already declared for route '/'") + new Error('Cannot create route "GET /" because it already exists') + ) + t.end() +}) + +test('.route() should throw if the user already defined a HEAD or OPTIONS route', (t) => { + const app = medley() + + app.route({ + method: ['HEAD', 'OPTIONS'], + path: '/', + handler: _ => _, + }) + + t.throws( + () => app.head('/', _ => _), + new Error('Cannot create route "HEAD /" because it already exists') + ) + t.throws( + () => app.options('/', _ => _), + new Error('Cannot create route "OPTIONS /" because it already exists') ) t.end() }) diff --git a/test/router-options.test.js b/test/router-options.test.js deleted file mode 100644 index 1e791fa..0000000 --- a/test/router-options.test.js +++ /dev/null @@ -1,64 +0,0 @@ -'use strict' - -const {test} = require('tap') -const medley = require('../') -const request = require('./utils/request') - -test('Should honor strictRouting option', (t) => { - t.plan(12) - const app1 = medley() - const app2 = medley({ - strictRouting: true, - }) - - app1.get('/test', (req, res) => { - res.send('test') - }) - app2.get('/test', (req, res) => { - res.send('test') - }) - - request(app1, '/test', (err, res) => { - t.error(err) - t.equal(res.statusCode, 200) - t.equal(res.body, 'test') - }) - - request(app1, '/test/', (err, res) => { - t.error(err) - t.equal(res.statusCode, 200) - t.equal(res.body, 'test') - }) - - request(app2, '/test', (err, res) => { - t.error(err) - t.equal(res.statusCode, 200) - t.equal(res.body, 'test') - }) - - request(app2, '/test/', (err, res) => { - t.error(err) - t.equal(res.statusCode, 404) - t.equal(res.body, 'Not Found: GET /test/') - }) - -}) - -test('Should honor maxParamLength option', (t) => { - t.plan(4) - const app = medley({maxParamLength: 10}) - - app.get('/test/:id', (req, response) => { - response.send({hello: 'world'}) - }) - - request(app, '/test/123456789', (error, res) => { - t.error(error) - t.strictEqual(res.statusCode, 200) - }) - - request(app, '/test/123456789abcd', (error, res) => { - t.error(error) - t.strictEqual(res.statusCode, 404) - }) -})