fix(node): ensure that unknown request bodies are handled without errors (#321)

Author: Aaron Hedges

packages/node/__tests__/index.test.ts

@@ -10,6 +10,11 @@ import config from '../src/config';
 import pkg from '../package.json';
 import { expressMiddleware } from '../src';
 import { ServerResponse } from 'http';
+import FormData from 'form-data';
+import multer from 'multer';
+
+const upload = multer();
+
 
 const apiKey = 'mockReadMeApiKey';
 const incomingGroup = {
@@ -166,7 +171,7 @@ describe('#metrics', () => {
       .reply(200);
 
     const app = express();
-    app.use((req, res, next) => {
+    app.use((req: any, res: any, next) => {
       req.a = 'a';
       res.b = 'b';
       res.c = 'c';
@@ -527,4 +532,55 @@ describe('#metrics', () => {
       mock.done();
     });
   });
+
+  describe('`req.body`', () => {
+    let apiMock;
+
+    function createMock(checkLocation: 'text' | 'params', requestBody: unknown) {
+      return nock(config.host, {
+        reqheaders: {
+          'Content-Type': 'application/json',
+          'User-Agent': `${pkg.name}/${pkg.version}`,
+        },
+      })
+        .post('/v1/request', ([body]) => {
+          expect(body.request.log.entries[0].request.postData[checkLocation]).toBe(requestBody);
+          return true;
+        })
+        .reply(200);
+    }
+
+    beforeEach(() => {
+      apiMock = getReadMeApiMock(1);
+    });
+
+    afterEach(() => {
+      apiMock.done();
+    });
+
+    it('should accept multipart/form-data', async () => {
+
+      const form = new FormData();
+      form.append('password', '123456');
+      form.append('apiKey', 'abc');
+      form.append('another', 'Hello world');
+
+      // If the request body for a multipart/form-data request comes in as an object (as it does with the express middleware) we expect it to be recorded json encoded
+      const mock = createMock('text', JSON.stringify({password: '123456', apiKey: 'abc', another: 'Hello world'}));
+      const app = express();
+      app.use(upload.none());
+      app.use(expressMiddleware(apiKey, () => incomingGroup));
+      app.post('/test', (req, res) => {
+        res.status(200).end();
+      });
+
+      await request(app)
+        .post('/test')
+        .set(form.getHeaders())
+        .send(form.getBuffer().toString())
+        .expect(200);
+
+      mock.done();
+    });
+  });
 });

packages/node/__tests__/lib/process-request.test.ts

@@ -2,8 +2,9 @@ import request from 'supertest';
 import * as http from 'http';
 import processRequest from '../../src/lib/process-request';
 import { LogOptions } from 'src/lib/construct-payload';
+import FormData from 'form-data';
 
-function createApp(reqOptions?: LogOptions, shouldPreParse: boolean = false) {
+function createApp(reqOptions?: LogOptions, shouldPreParse: boolean = false, bodyOverride?) {
   const requestListener = function (req: http.IncomingMessage, res: http.ServerResponse) {
     let body = "";
 
@@ -20,7 +21,7 @@ function createApp(reqOptions?: LogOptions, shouldPreParse: boolean = false) {
         body = JSON.parse(body);
       }
 
-      res.end(JSON.stringify(processRequest(req, body, reqOptions)));
+      res.end(JSON.stringify(processRequest(req, bodyOverride ? bodyOverride : body, reqOptions)));
     });
   };
 
@@ -96,6 +97,43 @@ it('should work with *+json', () => {
     });
 });
 
+it('should work with multipart/form-data', () => {
+  const app = createApp({ denylist: ['password']});
+
+  const form = new FormData();
+  form.append('password', '123456');
+  form.append('apiKey', 'abc');
+  form.append('another', 'Hello world');
+
+  const formHeaders = form.getHeaders();
+
+  return request(app)
+    .post('/')
+    .set(formHeaders)
+    .send(form.getBuffer().toString())
+    .expect(({ body }) => {
+      // If the request body for multipart form comes in as a string, we record it as is.
+      expect(body.postData.text).toBe(form.getBuffer().toString());
+    });
+});
+
+it('should fail gracefully with circular json objects', () => {
+  const obj = { foo: null };
+  obj.foo = obj;
+
+  const app = createApp({ denylist: ['password']}, false, obj);
+
+  return request(app)
+    .post('/')
+    .set('content-type', 'text/plain')
+    .send('this isn\'t used')
+    .expect(({ body }) => {
+      console.log(body);
+      // If the request body for multipart form comes in as a string, we record it as is.
+      expect(body.postData.text).toBe('[ReadMe is unable to handle circular JSON. Please contact support if you have any questions.]');
+    });
+});
+
 describe('options', () => {
   describe('denylist/allowlist', () => {
     it('should strip denylisted json properties', () => {