ref: Skip body parsing for GET/HEAD requests (#2504)

getsentry · Mar 19, 2020 · c7fb6e1 · c7fb6e1
1 parent dea97aa
commit c7fb6e1
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 15 deletions.
diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts
@@ -148,19 +148,14 @@ function extractRequestData(req: { [key: string]: any }, keys: boolean | string[
         request.query_string = url.parse(originalUrl || '', false).query;
         break;
       case 'data':
-        // body data:
-        //   node, express, koa: req.body
-        let data = req.body;
         if (method === 'GET' || method === 'HEAD') {
-          if (typeof data === 'undefined') {
-            data = '<unavailable>';
-          }
+          break;
         }
-        if (data && !isString(data)) {
-          // Make sure the request body is a string
-          data = JSON.stringify(normalize(data));
+        // body data:
+        //   node, express, koa: req.body
+        if (req.body !== undefined) {
+          request.data = isString(req.body) ? req.body : JSON.stringify(normalize(req.body));
         }
-        request.data = data;
         break;
       default:
         if ({}.hasOwnProperty.call(req, key)) {

diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts
@@ -3,12 +3,12 @@ import { parseRequest } from '../src/handlers';
 
 describe('parseRequest', () => {
   const mockReq = {
-    body: '',
+    body: 'foo',
     cookies: { test: 'test' },
     headers: {
       host: 'mattrobenolt.com',
     },
-    method: 'GET',
+    method: 'POST',
     url: '/some/path?key=value',
     user: {
       custom_property: 'foo',
@@ -72,16 +72,22 @@ describe('parseRequest', () => {
   describe('parseRequest.request properties', () => {
     test('parseRequest.request only contains the default set of properties from the request', () => {
       const DEFAULT_REQUEST_PROPERTIES = ['cookies', 'data', 'headers', 'method', 'query_string', 'url'];
-      const parsedRequest: Event = parseRequest({}, mockReq, {});
-      expect(Object.keys(parsedRequest.request as any[])).toEqual(DEFAULT_REQUEST_PROPERTIES);
+      const parsedRequest: Event = parseRequest({}, mockReq);
+      expect(Object.keys(parsedRequest.request as {})).toEqual(DEFAULT_REQUEST_PROPERTIES);
     });
 
     test('parseRequest.request only contains the specified properties in the options.request array', () => {
       const INCLUDED_PROPERTIES = ['data', 'headers', 'query_string', 'url'];
       const parsedRequest: Event = parseRequest({}, mockReq, {
         request: INCLUDED_PROPERTIES,
       });
-      expect(Object.keys(parsedRequest.request as any[])).toEqual(['data', 'headers', 'query_string', 'url']);
+      expect(Object.keys(parsedRequest.request as {})).toEqual(INCLUDED_PROPERTIES);
+    });
+
+    test('parseRequest.request skips `body` property for GET and HEAD requests', () => {
+      expect(parseRequest({}, mockReq, {}).request).toHaveProperty('data');
+      expect(parseRequest({}, { ...mockReq, method: 'GET' }, {}).request).not.toHaveProperty('data');
+      expect(parseRequest({}, { ...mockReq, method: 'HEAD' }, {}).request).not.toHaveProperty('data');
     });
   });
 });