feat(rest): add strict option for routers

packages/rest/src/__tests__/unit/router/routing-table.unit.ts

@@ -6,19 +6,19 @@
 import {anOpenApiSpec} from '@loopback/openapi-spec-builder';
 import {get, getControllerSpec, param} from '@loopback/openapi-v3';
 import {
-  ShotRequestOptions,
   expect,
+  ShotRequestOptions,
   stubExpressContext,
 } from '@loopback/testlab';
+import * as HttpErrors from 'http-errors';
 import {
   ControllerRoute,
+  RegExpRouter,
   Request,
-  RoutingTable,
   RestRouter,
-  RegExpRouter,
+  RoutingTable,
   TrieRouter,
 } from '../../..';
-import * as HttpErrors from 'http-errors';
 
 describe('RoutingTable', () => {
   it('joins basePath and path', () => {
@@ -220,10 +220,6 @@ function runTestsWithRouter(router: RestRouter) {
       async getOrderById(@param.path.number('id') id: number): Promise<object> {
         return {id};
       }
-      @get('/orders')
-      async findOrders(): Promise<object[]> {
-        return [];
-      }
       // A path that overlaps with `/orders/{id}`. Please note a different var
       // name is used - `{orderId}`
       @get('/orders/{orderId}/shipments')
@@ -232,6 +228,16 @@ function runTestsWithRouter(router: RestRouter) {
       ): Promise<object> {
         return [];
       }
+      // With trailing `/`
+      @get('/orders/')
+      async findOrders(): Promise<object[]> {
+        return [];
+      }
+      // Without trailing `/`
+      @get('/pendingOrders')
+      async findPendingOrders(): Promise<object[]> {
+        return [];
+      }
     }
 
     const table = givenRoutingTable();
@@ -250,6 +256,9 @@ function runTestsWithRouter(router: RestRouter) {
     findAndCheckRoute('/orders/1', '/orders/{id}');
     findAndCheckRoute('/orders/1/shipments', '/orders/{orderId}/shipments');
     findAndCheckRoute('/orders', '/orders');
+    findAndCheckRoute('/orders/', '/orders');
+    findAndCheckRoute('/pendingOrders', '/pendingOrders');
+    findAndCheckRoute('/pendingOrders/', '/pendingOrders');
   });
 
   it('throws if router is not found', () => {

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,25 +39,38 @@ 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;
   }
 
-  protected findRouteWithPathVars(request: Request): ResolvedRoute | undefined {
+  protected findRouteWithPathVars(
+    verb: string,
+    path: string,
+  ): ResolvedRoute | undefined {
     this._sort();
     for (const r of this.routes) {
       debug('trying endpoint %s', inspect(r, {depth: 5}));
-      if (r.verb !== request.method.toLowerCase()) {
+      if (r.verb !== verb.toLowerCase()) {
         debug(' -> verb mismatch');
         continue;
       }
 
-      const match = r.regexp.exec(request.path);
+      const match = r.regexp.exec(path);
       if (!match) {
         debug(' -> path mismatch');
         continue;

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;
+};