fix(rest): remove trailing slash for routing match

See #2188
loopbackio · Feb 12, 2019 · 856f332 · 856f332
1 parent 669ede1
commit 856f332
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 13 deletions.
diff --git a/packages/rest/src/__tests__/unit/router/routing-table.unit.ts b/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', () => {

diff --git a/packages/rest/src/router/router-base.ts b/packages/rest/src/router/router-base.ts
@@ -19,9 +19,7 @@ export abstract class BaseRouter implements RestRouter {
   protected routesWithoutPathVars: {[path: string]: RouteEntry} = {};
 
   protected getKeyForRoute(route: RouteEntry) {
-    const path = route.path.startsWith('/') ? route.path : `/${route.path}`;
-    const verb = route.verb.toLowerCase() || 'get';
-    return `/${verb}${path}`;
+    return getKey(route.verb, route.path);
   }
 
   add(route: RouteEntry) {
@@ -34,8 +32,7 @@ export abstract class BaseRouter implements RestRouter {
   }
 
   protected getKeyForRequest(request: Request) {
-    const method = request.method.toLowerCase();
-    return `/${method}${request.path}`;
+    return getKey(request.method, request.path);
   }
 
   find(request: Request) {
@@ -72,3 +69,22 @@ 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}`;
+}