feat(rest): add strict option for routers

packages/rest/src/__tests__/unit/router/trie-router.unit.ts

@@ -10,6 +10,7 @@ import {
   stubExpressContext,
 } from '@loopback/testlab';
 import {Request, ResolvedRoute, RouteEntry, TrieRouter} from '../../..';
+import {RestRouterOptions} from '../../../router';
 
 class TestTrieRouter extends TrieRouter {
   get staticRoutes() {
@@ -146,18 +147,6 @@ describe('trie router - overlapping paths with different var names', () => {
     });
   });
 
-  function getRouteInfo(r?: ResolvedRoute) {
-    return {
-      verb: r && r.verb,
-      path: r && r.path,
-      params: r && r.pathParams,
-    };
-  }
-
-  function givenRequest(options?: ShotRequestOptions): Request {
-    return stubExpressContext(options).request;
-  }
-
   function givenRoutesWithDifferentVars() {
     const routes: RouteEntry[] = [];
 
@@ -173,8 +162,104 @@ describe('trie router - overlapping paths with different var names', () => {
   }
 });
 
-function givenTrieRouter(routes: RouteEntry[]) {
-  const router = new TestTrieRouter();
+describe('trie router with options', () => {
+  let router: TestTrieRouter;
+
+  describe('strict: true', () => {
+    before(givenStrictRouter);
+    it('finds route for GET /users/', () => {
+      testStrictRouter('/users/');
+    });
+
+    it('fails to find route for GET /users', () => {
+      const req = givenRequest({method: 'get', url: '/users'});
+      const route = router.find(req);
+      expect(route).to.be.undefined();
+    });
+
+    it('finds route for GET /orders', () => {
+      testStrictRouter('/orders');
+    });
+
+    it('fails to find route for GET /orders/', () => {
+      const req = givenRequest({method: 'get', url: '/orders/'});
+      const route = router.find(req);
+      expect(route).to.be.undefined();
+    });
+
+    function testStrictRouter(path: string) {
+      const req = givenRequest({method: 'get', url: path});
+      const route = router.find(req);
+      expect(getRouteInfo(route)).to.containEql({
+        verb: 'get',
+        path,
+        params: {},
+      });
+    }
+
+    function givenStrictRouter() {
+      router = givenTrieRouter(givenRoutesWithDifferentVars(), {strict: true});
+    }
+  });
+
+  describe('strict: false', () => {
+    before(givenNonStrictRouter);
+    it('finds route for GET /users/', () => {
+      testNonStrictRouter('/users/');
+    });
+
+    it('finds route for GET /users', () => {
+      testNonStrictRouter('/users', '/users/');
+    });
+
+    it('finds route for GET /orders/', () => {
+      testNonStrictRouter('/orders/', '/orders');
+    });
+
+    it('finds route for GET /orders', () => {
+      testNonStrictRouter('/orders');
+    });
+
+    function testNonStrictRouter(path: string, expected?: string) {
+      expected = expected || path;
+      const req = givenRequest({method: 'get', url: path});
+      const route = router.find(req);
+      expect(getRouteInfo(route)).to.containEql({
+        verb: 'get',
+        path: expected,
+        params: {},
+      });
+    }
+
+    function givenNonStrictRouter() {
+      router = givenTrieRouter(givenRoutesWithDifferentVars(), {strict: false});
+    }
+  });
+
+  function givenRoutesWithDifferentVars() {
+    const routes: RouteEntry[] = [];
+
+    addRoute(routes, 'getUsers', 'get', '/users/');
+    addRoute(routes, 'getOrders', 'get', '/orders');
+
+    return routes;
+  }
+});
+
+function getRouteInfo(r?: ResolvedRoute) {
+  return {
+    verb: r && r.verb,
+    path: r && r.path,
+    params: r && r.pathParams,
+  };
+}
+
+function givenRequest(options?: ShotRequestOptions): Request {
+  return stubExpressContext(options).request;
+}
+
+function givenTrieRouter(routes: RouteEntry[], options?: RestRouterOptions) {
+  const router = new TestTrieRouter(options);
   for (const r of routes) {
     router.add(r);
   }

packages/rest/src/keys.ts

@@ -31,7 +31,7 @@ import {
 import {HttpProtocol} from '@loopback/http-server';
 import * as https from 'https';
 import {ErrorWriterOptions} from 'strong-error-handler';
-import {RestRouter} from './router';
+import {RestRouter, RestRouterOptions} from './router';
 import {RequestBodyParser, BodyParser} from './body-parsers';
 
 /**
@@ -80,6 +80,10 @@ export namespace RestBindings {
    */
   export const ROUTER = BindingKey.create<RestRouter>('rest.router');
 
+  export const ROUTER_OPTIONS = BindingKey.create<RestRouterOptions>(
+    'rest.router.options',
+  );
+
   /**
    * Binding key for setting and injecting Reject action's error handling
    * options.

packages/rest/src/rest.server.ts

@@ -42,6 +42,7 @@ import {
   RouteEntry,
   RoutingTable,
   StaticAssetsRoute,
+  RestRouterOptions,
 } from './router';
 import {DefaultSequence, SequenceFunction, SequenceHandler} from './sequence';
 import {
@@ -194,6 +195,10 @@ export class RestServer extends Context implements Server, HttpServerLike {
       this.sequence(config.sequence);
     }
 
+    if (config.router) {
+      this.bind(RestBindings.ROUTER_OPTIONS).to(config.router);
+    }
+
     this.basePath(config.basePath);
 
     this.bind(RestBindings.BASE_PATH).toDynamicValue(() => this._basePath);
@@ -204,6 +209,9 @@ export class RestServer extends Context implements Server, HttpServerLike {
     if (this._expressApp) return;
     this._expressApp = express();
     this._expressApp.set('query parser', 'extended');
+    if (this.config.router && typeof this.config.router.strict === 'boolean') {
+      this._expressApp.set('strict routing', this.config.router.strict);
+    }
     this._requestHandler = this._expressApp;
 
     // Allow CORS support for all endpoints so that users
@@ -937,6 +945,7 @@ export interface RestServerOptions {
   openApiSpec?: OpenApiSpecOptions;
   apiExplorer?: ApiExplorerOptions;
   sequence?: Constructor<SequenceHandler>;
+  router?: RestRouterOptions;
 }
 
 /**

packages/rest/src/router/regexp-router.ts

@@ -3,13 +3,16 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import {RouteEntry, createResolvedRoute, ResolvedRoute} from './route-entry';
-import {Request, PathParameterValues} from '../types';
+import {inject} from '@loopback/context';
 import {inspect} from 'util';
+import {RestBindings} from '../keys';
+import {PathParameterValues, Request} from '../types';
+import {toExpressPath} from './openapi-path';
+import {RestRouterOptions} from './rest-router';
+import {createResolvedRoute, ResolvedRoute, RouteEntry} from './route-entry';
 import {compareRoute} from './route-sort';
-import pathToRegExp = require('path-to-regexp');
 import {BaseRouter} from './router-base';
-import {toExpressPath} from './openapi-path';
+import pathToRegExp = require('path-to-regexp');
 
 const debug = require('debug')('loopback:rest:router:regexp');
 
@@ -36,10 +39,20 @@ export class RegExpRouter extends BaseRouter {
     }
   }
 
+  constructor(
+    @inject(RestBindings.ROUTER_OPTIONS, {optional: true})
+    options?: RestRouterOptions,
+  ) {
+    super(options);
+  }
+
   protected addRouteWithPathVars(route: RouteEntry) {
     const path = toExpressPath(route.path);
     const keys: pathToRegExp.Key[] = [];
-    const regexp = pathToRegExp(path, keys, {strict: false, end: true});
+    const regexp = pathToRegExp(path, keys, {
+      strict: this.options.strict,
+      end: true,
+    });
     const entry: RegExpRouteEntry = Object.assign(route, {keys, regexp});
     this.routes.push(entry);
     this._sorted = false;

packages/rest/src/router/rest-router.ts

@@ -29,3 +29,12 @@ export interface RestRouter {
    */
   list(): RouteEntry[];
 }
+
+export type RestRouterOptions = {
+  /**
+   * When true it allows an optional trailing slash to match. (default: false)
+   *
+   * See `strict routing` at http://expressjs.com/en/4x/api.html#app
+   */
+  strict?: boolean;
+};

packages/rest/src/router/router-base.ts

@@ -7,7 +7,7 @@ import {Request} from '../types';
 import {getPathVariables} from './openapi-path';
 import {createResolvedRoute, ResolvedRoute, RouteEntry} from './route-entry';
 import {compareRoute} from './route-sort';
-import {RestRouter} from './rest-router';
+import {RestRouter, RestRouterOptions} from './rest-router';
 
 /**
  * Base router implementation that only handles path without variables
@@ -18,8 +18,10 @@ export abstract class BaseRouter implements RestRouter {
    */
   protected routesWithoutPathVars: {[path: string]: RouteEntry} = {};
 
+  constructor(protected options: RestRouterOptions = {strict: false}) {}
+
   protected getKeyForRoute(route: RouteEntry) {
-    return getKey(route.verb, route.path);
+    return this.getKey(route.verb, route.path);
   }
 
   add(route: RouteEntry) {
@@ -32,7 +34,7 @@ export abstract class BaseRouter implements RestRouter {
   }
 
   protected getKeyForRequest(request: Request) {
-    return getKey(request.method, request.path);
+    return this.getKey(request.method, request.path);
   }
 
   find(request: Request) {
@@ -49,6 +51,24 @@ export abstract class BaseRouter implements RestRouter {
     return routes.sort(compareRoute);
   }
 
+  /**
+   * Build a key for verb+path
+   * @param verb HTTP verb/method
+   * @param path URL path
+   */
+  protected getKey(verb: string, path: string) {
+    // Use lower case
+    verb = (verb && verb.toLowerCase()) || 'get';
+    // Prepend `/` if needed
+    path = path || '/';
+    path = path.startsWith('/') ? path : `/${path}`;
+    if (!this.options.strict && path !== '/' && path.endsWith('/')) {
+      // Remove trailing `/`
+      path = path.substring(0, path.length - 1);
+    }
+    return `/${verb}${path}`;
+  }
+
   // The following abstract methods need to be implemented by its subclasses
   /**
    * Add a route with path variables
@@ -69,22 +89,3 @@ export abstract class BaseRouter implements RestRouter {
    */
   protected abstract listRoutesWithPathVars(): RouteEntry[];
 }
-
-/**
- * Build a key for verb+path
- * @param verb HTTP verb/method
- * @param path URL path
- */
-function getKey(verb: string, path: string) {
-  // Use lower case
-  verb = (verb && verb.toLowerCase()) || 'get';
-  // Prepend `/` if needed
-  path = path || '/';
-  path = path.startsWith('/') ? path : `/${path}`;
-  if (path !== '/' && path.endsWith('/')) {
-    // Remove trailing `/`
-    // See https://github.com/strongloop/loopback-next/issues/2188
-    path = path.substring(0, path.length - 1);
-  }
-  return `/${verb}${path}`;
-}

packages/rest/src/router/trie-router.ts

@@ -3,11 +3,14 @@
 // This file is licensed under the MIT License.
 // License text available at https://opensource.org/licenses/MIT
 
-import {RouteEntry, createResolvedRoute} from './route-entry';
-import {Trie} from './trie';
-import {Request} from '../types';
+import {inject} from '@loopback/context';
 import {inspect} from 'util';
+import {RestBindings} from '../keys';
+import {Request} from '../types';
+import {RestRouterOptions} from './rest-router';
+import {createResolvedRoute, RouteEntry} from './route-entry';
 import {BaseRouter} from './router-base';
+import {Trie} from './trie';
 
 const debug = require('debug')('loopback:rest:router:trie');
 
@@ -17,6 +20,13 @@ const debug = require('debug')('loopback:rest:router:trie');
 export class TrieRouter extends BaseRouter {
   private trie = new Trie<RouteEntry>();
 
+  constructor(
+    @inject(RestBindings.ROUTER_OPTIONS, {optional: true})
+    options?: RestRouterOptions,
+  ) {
+    super(options);
+  }
+
   protected addRouteWithPathVars(route: RouteEntry) {
     // Add the route to the trie
     const key = this.getKeyForRoute(route);