feat(rest): switch to express body-parser

raymondfeng · raymondfeng · commit 6d958026cdee · 2018-10-17T22:48:05.000-07:00
diff --git a/packages/rest/package.json b/packages/rest/package.json
@@ -25,13 +25,15 @@
     "@loopback/http-server": "^1.0.0",
     "@loopback/openapi-v3": "^1.0.1",
     "@loopback/openapi-v3-types": "^1.0.0",
+    "@types/body-parser": "^1.17.0",
     "@types/cors": "^2.8.3",
     "@types/express": "^4.11.1",
     "@types/http-errors": "^1.6.1",
     "@types/parseurl": "^1.3.1",
     "@types/qs": "^6.5.1",
+    "@types/type-is": "^1.6.2",
     "ajv": "^6.5.1",
-    "body": "^5.1.0",
+    "body-parser": "^1.18.3",
     "cors": "^2.8.4",
     "debug": "^4.0.1",
     "express": "^4.16.3",
@@ -44,6 +46,7 @@
     "path-to-regexp": "^2.2.0",
     "qs": "^6.5.2",
     "strong-error-handler": "^3.2.0",
+    "type-is": "^1.6.16",
     "validator": "^10.4.0"
   },
   "devDependencies": {
diff --git a/packages/rest/src/parser.ts b/packages/rest/src/parser.ts
@@ -11,22 +11,24 @@ import {
   SchemasObject,
 } from '@loopback/openapi-v3-types';
 import * as debugModule from 'debug';
-import {IncomingMessage} from 'http';
 import * as HttpErrors from 'http-errors';
 import * as parseUrl from 'parseurl';
 import {parse as parseQuery} from 'qs';
-import {promisify} from 'util';
 import {coerceParameter} from './coercion/coerce-parameter';
 import {RestHttpErrors} from './index';
 import {ResolvedRoute} from './router/routing-table';
 import {
   OperationArgs,
   PathParameterValues,
   Request,
+  Response,
   RequestBodyParserOptions,
 } from './types';
 import {validateRequestBody} from './validation/request-body.validator';
 
+import {json, urlencoded, text} from 'body-parser';
+import * as typeis from 'type-is';
+
 type HttpError = HttpErrors.HttpError;
 
 const debug = debugModule('loopback:rest:parser');
@@ -40,31 +42,6 @@ type RequestBody = {
   coercionRequired?: boolean;
 };
 
-const parseJsonBody: (
-  req: IncomingMessage,
-  options: {},
-) => Promise<any> = promisify(require('body/json'));
-
-const parseFormBody: (
-  req: IncomingMessage,
-  options: {},
-) => Promise<any> = promisify(require('body/form'));
-
-/**
- * Get the content-type header value from the request
- * @param req Http request
- */
-function getContentType(req: Request): string | undefined {
-  const val = req.headers['content-type'];
-  if (typeof val === 'string') {
-    return val;
-  } else if (Array.isArray(val)) {
-    // Assume only one value is present
-    return val[0];
-  }
-  return undefined;
-}
-
 /**
  * Parses the request to derive arguments to be passed in for the Application
  * controller method
@@ -90,6 +67,41 @@ export async function parseOperationArgs(
   );
 }
 
+/**
+ * Express body parser function type
+ */
+type BodyParser = (
+  request: Request,
+  response: Response,
+  callback: (err: HttpError) => void,
+) => void;
+
+/**
+ * Parse the body asynchronously
+ * @param handle The express middleware handler
+ * @param request Http request
+ */
+function parse(handle: BodyParser, request: Request): Promise<void> {
+  // A hack to fool TypeScript as we don't need `response`
+  const response = ({} as any) as Response;
+  return new Promise<void>((resolve, reject) => {
+    handle(request, response, err => {
+      if (err) {
+        debug('Cannot parse request body %j', err);
+        if (!err.statusCode || err.statusCode >= 500) {
+          err.statusCode = 400;
+        }
+        reject(err);
+        return;
+      }
+      resolve();
+    });
+  });
+}
+
+// Default limit of the body length
+const DEFAULT_LIMIT = '1mb';
+
 async function loadRequestBodyIfNeeded(
   operationSpec: OperationObject,
   request: Request,
@@ -99,45 +111,65 @@ async function loadRequestBodyIfNeeded(
 
   debug('Request body parser options: %j', options);
 
-  const contentType = getContentType(request);
-  debug('Loading request body with content type %j', contentType);
+  let body = await parseJsonBody(request, options);
+  if (body) return body;
+  body = await parseUrlencodedBody(request, options);
+  if (body) return body;
+  body = await parseTextBody(request, options);
+  if (body) return body;
 
-  if (
-    contentType &&
-    contentType.startsWith('application/x-www-form-urlencoded')
-  ) {
-    const body = await parseFormBody(request, options).catch(
-      (err: HttpError) => {
-        debug('Cannot parse request body %j', err);
-        if (!err.statusCode || err.statusCode >= 500) {
-          err.statusCode = 400;
-        }
-        throw err;
-      },
-    );
-    // form parser returns an object with prototype
-    return {
-      value: Object.assign({}, body),
-      coercionRequired: true,
-    };
-  }
+  throw new HttpErrors.UnsupportedMediaType(
+    `Content-type ${request.get('content-type')} is not supported.`,
+  );
+}
 
-  if (contentType && !/json/.test(contentType)) {
-    throw new HttpErrors.UnsupportedMediaType(
-      `Content-type ${contentType} is not supported.`,
-    );
+async function parseJsonBody(
+  request: Request,
+  options: RequestBodyParserOptions,
+) {
+  const jsonOptions = Object.assign(
+    {type: 'json', limit: DEFAULT_LIMIT},
+    options,
+  );
+  if (typeis(request, jsonOptions.type)) {
+    await parse(json(jsonOptions), request);
+    return {value: request.body};
   }
+  return undefined;
+}
 
-  const jsonBody = await parseJsonBody(request, options).catch(
-    (err: HttpError) => {
-      debug('Cannot parse request body %j', err);
-      if (!err.statusCode || err.statusCode >= 500) {
-        err.statusCode = 400;
-      }
-      throw err;
+async function parseUrlencodedBody(
+  request: Request,
+  options: RequestBodyParserOptions,
+) {
+  const urlencodedOptions = Object.assign(
+    {
+      extended: true,
+      type: 'urlencoded',
+      limit: DEFAULT_LIMIT,
     },
+    options,
+  );
+  if (typeis(request, urlencodedOptions.type)) {
+    await parse(urlencoded(urlencodedOptions), request);
+    return {value: request.body, coercionRequired: true};
+  }
+  return undefined;
+}
+
+async function parseTextBody(
+  request: Request,
+  options: RequestBodyParserOptions,
+) {
+  const textOptions = Object.assign(
+    {type: 'text/*', limit: DEFAULT_LIMIT},
+    options,
   );
-  return {value: jsonBody};
+  if (typeis(request, textOptions.type)) {
+    await parse(text(textOptions), request);
+    return {value: request.body};
+  }
+  return undefined;
 }
 
 function buildOperationArguments(
diff --git a/packages/rest/src/types.ts b/packages/rest/src/types.ts
@@ -6,6 +6,7 @@
 import {Binding, BoundValue} from '@loopback/context';
 import {ResolvedRoute, RouteEntry} from './router/routing-table';
 import {Request, Response} from 'express';
+import {OptionsJson, OptionsUrlencoded, OptionsText} from 'body-parser';
 
 export {Request, Response};
 
@@ -81,11 +82,9 @@ export type LogError = (
 /**
  * Options for request body parsing
  */
-export type RequestBodyParserOptions = {
-  limit?: number;
-  encoding?: string;
-  [property: string]: any;
-};
+export type RequestBodyParserOptions = OptionsJson &
+  OptionsUrlencoded &
+  OptionsText;
 
 export type PathParameterValues = {[key: string]: any};
 export type OperationArgs = any[];
diff --git a/packages/rest/test/acceptance/validation/validation.acceptance.ts b/packages/rest/test/acceptance/validation/validation.acceptance.ts
@@ -65,7 +65,10 @@ describe('Validation at REST level', () => {
     it('rejects requests with no (empty) body', async () => {
       // NOTE(bajtos) An empty body cannot be parsed as a JSON,
       // therefore this test request does not even reach the validation logic.
-      await client.post('/products').expect(400);
+      await client
+        .post('/products')
+        .type('json')
+        .expect(422);
     });
 
     it('rejects requests with null body', async () => {
diff --git a/packages/rest/test/integration/http-handler.integration.ts b/packages/rest/test/integration/http-handler.integration.ts
@@ -287,9 +287,9 @@ describe('HttpHandler', () => {
         .send('key=' + givenLargeRequest())
         .expect(413, {
           error: {
-            message: 'request entity too large',
-            name: 'Error',
             statusCode: 413,
+            name: 'PayloadTooLargeError',
+            message: 'request entity too large',
           },
         })
         .catch(err => {
@@ -307,9 +307,9 @@ describe('HttpHandler', () => {
         .send({key: givenLargeRequest()})
         .expect(413, {
           error: {
-            message: 'request entity too large',
-            name: 'Error',
             statusCode: 413,
+            name: 'PayloadTooLargeError',
+            message: 'request entity too large',
           },
         })
         .catch(err => {
@@ -323,7 +323,7 @@ describe('HttpHandler', () => {
       const body = {key: givenLargeRequest()};
       rootContext
         .bind(RestBindings.REQUEST_BODY_PARSER_OPTIONS)
-        .to({limit: 4 * 1024 * 1024}); // Set limit to 4MB
+        .to({limit: '4mb'}); // Set limit to 4MB
       return client
         .post('/show-body')
         .set('content-type', 'application/json')
diff --git a/packages/rest/test/unit/parser.unit.ts b/packages/rest/test/unit/parser.unit.ts
@@ -169,6 +169,50 @@ describe('operationArgsParser', () => {
     expect(args).to.eql([{key1: ['value1', 'value2']}]);
   });
 
+  it('parses body parameter for text data', async () => {
+    const req = givenRequest({
+      url: '/',
+      headers: {
+        'Content-Type': 'text/plain',
+      },
+      payload: 'plain-text',
+    });
+
+    const spec = givenOperationWithRequestBody({
+      description: 'data',
+      content: {
+        'text/plain': {schema: {type: 'string'}},
+      },
+    });
+    const route = givenResolvedRoute(spec);
+
+    const args = await parseOperationArgs(req, route);
+
+    expect(args).to.eql(['plain-text']);
+  });
+
+  it('parses body parameter for html data', async () => {
+    const req = givenRequest({
+      url: '/',
+      headers: {
+        'Content-Type': 'text/html',
+      },
+      payload: '<html><body><h1>Hello</h1></body></html>',
+    });
+
+    const spec = givenOperationWithRequestBody({
+      description: 'data',
+      content: {
+        'text/html': {schema: {type: 'string'}},
+      },
+    });
+    const route = givenResolvedRoute(spec);
+
+    const args = await parseOperationArgs(req, route);
+
+    expect(args).to.eql(['<html><body><h1>Hello</h1></body></html>']);
+  });
+
   context('in:query style:deepObject', () => {
     it('parses JSON-encoded string value', async () => {
       const req = givenRequest({
