fix(rest): parse query string even when there is no rest query param

See #2088

packages/rest/src/parser.ts

@@ -10,20 +10,17 @@ import {
   ParameterObject,
   SchemasObject,
 } from '@loopback/openapi-v3-types';
-import * as debugModule from 'debug';
-import * as parseUrl from 'parseurl';
-import {parse as parseQuery} from 'qs';
+import * as debugFactory from 'debug';
 import {RequestBody, RequestBodyParser} from './body-parsers';
 import {coerceParameter} from './coercion/coerce-parameter';
 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 = debugFactory('loopback:rest:parser');
 
-const debug = debugModule('loopback:rest:parser');
-
-export const QUERY_NOT_PARSED = {};
-Object.freeze(QUERY_NOT_PARSED);
+// See https://github.com/expressjs/express/blob/master/lib/express.js#L79
+const expressQuery = require('express').query;
 
 /**
  * Parses the request to derive arguments to be passed in for the Application
@@ -106,22 +103,23 @@ 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:
       throw RestHttpErrors.invalidParamLocation(spec.in);
   }
 }
 
+/**
+ * This method is mostly used for unit testing where Express is not set up
+ * to parse query string into `request.query` object
+ * @param request
+ */
 function ensureRequestQueryWasParsed(request: Request) {
-  if (request.query && request.query !== QUERY_NOT_PARSED) return;
-
-  const input = parseUrl(request)!.query;
-  if (input && typeof input === 'string') {
-    request.query = parseQuery(input);
-  } else {
-    request.query = {};
-  }
+  if (request.query) return;
+  // Use `express.query` to parse the query string
+  // See https://github.com/expressjs/express/blob/master/lib/express.js#L79
+  // See https://github.com/expressjs/express/blob/master/lib/middleware/query.js
+  expressQuery()(request, {}, () => {});
   debug('Parsed request query: ', request.query);
 }

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

packages/rest/test/integration/rest.server.integration.ts

@@ -18,6 +18,7 @@ import {
   RestComponent,
   get,
   Request,
+  Response,
   RestServerConfig,
   BodyParser,
 } from '../..';
@@ -52,6 +53,28 @@ describe('RestServer (integration)', () => {
     expect(server.url).to.be.undefined();
   });
 
+  it('parses query without decorated rest query params', async () => {
+    // This handler responds with the query object (which is expected to
+    // be parsed by Express)
+    function requestWithQueryHandler(handler: {
+      request: Request;
+      response: Response;
+    }) {
+      const {request, response} = handler;
+      response.json(request.query);
+      response.end();
+    }
+
+    // See https://github.com/strongloop/loopback-next/issues/2088
+    const server = await givenAServer({rest: {port: 0, host: '127.0.0.1'}});
+    server.handler(requestWithQueryHandler);
+    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();