Skip to content

Commit

Permalink
fix(rest): parse query string even when there is no rest query param
Browse files Browse the repository at this point in the history
See #2088
  • Loading branch information
raymondfeng committed Nov 28, 2018
1 parent 7be76a4 commit 0803677
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 16 deletions.
12 changes: 6 additions & 6 deletions packages/rest/src/parser.ts
Expand Up @@ -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
Expand Down Expand Up @@ -106,16 +102,20 @@ 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 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') {
Expand Down
11 changes: 1 addition & 10 deletions packages/rest/src/rest.server.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
21 changes: 21 additions & 0 deletions packages/rest/test/integration/rest.server.integration.ts
Expand Up @@ -18,6 +18,7 @@ import {
RestComponent,
get,
Request,
Response,
RestServerConfig,
BodyParser,
} from '../..';
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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', {
Expand Down

0 comments on commit 0803677

Please sign in to comment.