From 0803677f3b6781e253a52d7b25d70250ae8a0478 Mon Sep 17 00:00:00 2001
From: Raymond Feng <enjoyjava@gmail.com>
Date: Tue, 27 Nov 2018 17:22:41 -0800
Subject: [PATCH] fix(rest): parse query string even when there is no rest
 query param

See https://github.com/strongloop/loopback-next/issues/2088
---
 packages/rest/src/parser.ts                   | 12 +++++------
 packages/rest/src/rest.server.ts              | 11 +---------
 .../integration/rest.server.integration.ts    | 21 +++++++++++++++++++
 3 files changed, 28 insertions(+), 16 deletions(-)

diff --git a/packages/rest/src/parser.ts b/packages/rest/src/parser.ts
index d64698c860f8..c8b27b9bf719 100644
--- a/packages/rest/src/parser.ts
+++ b/packages/rest/src/parser.ts
@@ -19,12 +19,8 @@ import {RestHttpErrors} from './rest-http-error';
 import {ResolvedRoute} from './router';
 import {OperationArgs, PathParameterValues, Request} from './types';
 import {validateRequestBody} from './validation/request-body.validator';
-
 const debug = debugModule('loopback:rest:parser');
 
-export const QUERY_NOT_PARSED = {};
-Object.freeze(QUERY_NOT_PARSED);
-
 /**
  * Parses the request to derive arguments to be passed in for the Application
  * controller method
@@ -106,7 +102,6 @@ function getParamFromRequest(
     case 'header':
       // @jannyhou TBD: check edge cases
       return request.headers[spec.name.toLowerCase()];
-      break;
     // TODO(jannyhou) to support `cookie`,
     // see issue https://github.com/strongloop/loopback-next/issues/997
     default:
@@ -114,8 +109,13 @@ function getParamFromRequest(
   }
 }
 
+/**
+ * This method is mostly used for unit testing where Express is not present
+ * to parse query string into `request.query` object
+ * @param request
+ */
 function ensureRequestQueryWasParsed(request: Request) {
-  if (request.query && request.query !== QUERY_NOT_PARSED) return;
+  if (request.query) return;
 
   const input = parseUrl(request)!.query;
   if (input && typeof input === 'string') {
diff --git a/packages/rest/src/rest.server.ts b/packages/rest/src/rest.server.ts
index 8c389c5aa3df..2f41c923560f 100644
--- a/packages/rest/src/rest.server.ts
+++ b/packages/rest/src/rest.server.ts
@@ -31,7 +31,6 @@ import {ServeStaticOptions} from 'serve-static';
 import {BodyParser, REQUEST_BODY_PARSER_TAG} from './body-parsers';
 import {HttpHandler} from './http-handler';
 import {RestBindings} from './keys';
-import {QUERY_NOT_PARSED} from './parser';
 import {RequestContext} from './request-context';
 import {
   ControllerClass,
@@ -193,15 +192,7 @@ export class RestServer extends Context implements Server, HttpServerLike {
 
   protected _setupRequestHandler() {
     this._expressApp = express();
-
-    // Disable express' built-in query parser, we parse queries ourselves
-    // Note that when disabled, express sets query to an empty object,
-    // which makes it difficult for us to detect whether the query
-    // has been parsed or not. At the same time, we want `request.query`
-    // to remain as an object, because everybody in express ecosystem expects
-    // that property to be defined. A static singleton object to the rescue!
-    this._expressApp.set('query parser fn', (str: string) => QUERY_NOT_PARSED);
-
+    this._expressApp.set('query parser', 'extended');
     this.requestHandler = this._expressApp;
 
     // Allow CORS support for all endpoints so that users
diff --git a/packages/rest/test/integration/rest.server.integration.ts b/packages/rest/test/integration/rest.server.integration.ts
index f2748aefdf36..e5010af86b5e 100644
--- a/packages/rest/test/integration/rest.server.integration.ts
+++ b/packages/rest/test/integration/rest.server.integration.ts
@@ -18,6 +18,7 @@ import {
   RestComponent,
   get,
   Request,
+  Response,
   RestServerConfig,
   BodyParser,
 } from '../..';
@@ -52,6 +53,17 @@ describe('RestServer (integration)', () => {
     expect(server.url).to.be.undefined();
   });
 
+  it('parses query without decorated rest query params', async () => {
+    // See https://github.com/strongloop/loopback-next/issues/2088
+    const server = await givenAServer({rest: {port: 0, host: '127.0.0.1'}});
+    server.handler(queryRequestHandler);
+    await server.start();
+    await supertest(server.url)
+      .get('/?x=1&y[a]=2')
+      .expect(200, {x: '1', y: {a: '2'}});
+    await server.stop();
+  });
+
   it('updates rest.port binding when listening on ephemeral port', async () => {
     const server = await givenAServer({rest: {port: 0}});
     await server.start();
@@ -716,6 +728,15 @@ paths:
     response.end();
   }
 
+  function queryRequestHandler(handler: {
+    request: Request;
+    response: Response;
+  }) {
+    const {request, response} = handler;
+    response.json(request.query);
+    response.end();
+  }
+
   class DummyController {
     constructor() {}
     @get('/html', {