fixed content_length and status always ints (#402)

Author: willarmiros

packages/core/lib/middleware/incoming_request_data.js

@@ -1,3 +1,4 @@
+var { getHttpResponseData } = require('../segments/segment_utils');
 
 /**
  * Represents an incoming HTTP/HTTPS call.
@@ -49,12 +50,7 @@ var getClientIp = function getClientIp(req) {
  */
 
 IncomingRequestData.prototype.close = function close(res) {
-  this.response = {
-    status: res.statusCode || ''
-  };
-
-  if (res.headers && res.headers['content-length'])
-    this.response.content_length = res.headers['content-length'];
+  this.response = getHttpResponseData(res);
 };
 
 module.exports = IncomingRequestData;

packages/core/lib/segments/attributes/remote_request_data.js

@@ -1,3 +1,4 @@
+const { getHttpResponseData } = require('../segment_utils');
 var { stripQueryStringFromPath } = require('../../utils');
 
 /**
@@ -23,10 +24,7 @@ RemoteRequestData.prototype.init = function init(req, res, downstreamXRayEnabled
   }
 
   if (res) {
-    this.response = {
-      status: res.statusCode || '',
-      content_length: (res.headers && res.headers['content-length']) ? res.headers['content-length'] : 0
-    };
+    this.response = getHttpResponseData(res);
   }
 };
 

packages/core/lib/segments/segment_utils.d.ts

@@ -1,3 +1,5 @@
+import * as http from 'http';
+
 export const streamingThreshold: number;
 
 export function getCurrentTime(): number;
@@ -13,3 +15,5 @@ export function setServiceData(serviceData: any): void;
 export function setStreamingThreshold(threshold: number): void;
 
 export function getStreamingThreshold(): number;
+
+export function getHttpResponseData(res: http.ServerResponse): object;

packages/core/lib/segments/segment_utils.js

@@ -1,3 +1,4 @@
+const { safeParseInt } = require('../utils');
 var logger = require('../logger');
 
 var DEFAULT_STREAMING_THRESHOLD = 100;
@@ -45,6 +46,27 @@ var utils = {
 
   getStreamingThreshold: function getStreamingThreshold() {
     return utils.streamingThreshold;
+  },
+
+  /**
+   * Parses an HTTP response object to return an X-Ray compliant HTTP response object.
+   * @param {http.ServerResponse} res
+   * @returns {Object} - X-Ray response object to be added to (sub)segment
+   */
+  getHttpResponseData: (res) => {
+    const ret = {};
+    if (!res) {
+      return ret;
+    }
+    
+    const status = safeParseInt(res.statusCode);
+    if (status !== 0) {
+      ret.status = status;
+    }
+    if (res.headers && res.headers['content-length']) {
+      ret.content_length = safeParseInt(res.headers['content-length']);
+    }
+    return ret;
   }
 };
 

packages/core/lib/utils.d.ts

@@ -19,3 +19,5 @@ export function objectWithoutProperties<T extends object, K extends keyof T>(
   keys: K[],
   preservePrototype?: boolean
 ): Omit<T, K>;
+
+export function safeParseInt(val: number | string): number;

packages/core/lib/utils.js

@@ -240,6 +240,18 @@ var utils = {
       target[property] = obj[property];
     }
     return target;
+  },
+
+  /**
+   * Safely gets an integer from a string or number
+   * @param {String | Number} - input to cast to integer
+   * @returns {Number} - Integer representation of input, or 0 if input is not castable to int
+   */
+  safeParseInt: (val) => {
+    if (!val || isNaN(val)) {
+      return 0;
+    }
+    return parseInt(val);
   }
 };
 

packages/core/test/unit/segments/segment_utils.test.js

@@ -14,4 +14,33 @@ describe('SegmentUtils', function() {
       assert.equal(SegmentUtils.streamingThreshold, 10);
     });
   });
+
+  describe('#getHttpResponseData', () => {
+    it('should populate attributes as integers', () => {
+      const responseWithStrings = {statusCode: '200', headers: {'content-length': '42'}};
+      const res = SegmentUtils.getHttpResponseData(responseWithStrings);
+      assert.deepEqual(res, {
+        'content_length': 42,
+        'status': 200
+      });
+    });
+
+    it('should omit missing properties', () => {
+      const responseWithStatus = {statusCode: 200};
+      const responseWithLength = {headers: {'content-length': 42}};
+      const emptyResponse = {};
+
+      const statusRes = SegmentUtils.getHttpResponseData(responseWithStatus);
+      const lengthRes = SegmentUtils.getHttpResponseData(responseWithLength);
+      const emptyRes = SegmentUtils.getHttpResponseData(emptyResponse);
+      
+      assert.deepEqual(statusRes, {
+        'status': 200
+      });
+      assert.deepEqual(lengthRes, {
+        'content_length': 42
+      });
+      assert.deepEqual(emptyRes, {});
+    });
+  });
 });

packages/core/test/unit/utils.test.js

@@ -368,4 +368,21 @@ describe('Utils', function() {
       assert.equal(Utils.wildcardMatch('?a?b*b?c?.?*.com', '1a2bjkdwfjkewb3c4.myelasticbeanstalkenv.org'), false);
     });
   });
+
+  describe('#safeParseInt', () => {
+    it('should cast string to int', () => {
+      assert.equal(Utils.safeParseInt('42'), 42);
+      assert.equal(Utils.safeParseInt('15.0'), 15);
+    });
+
+    it('should cast numbers to int', () => {
+      assert.equal(Utils.safeParseInt(42), 42);
+      assert.equal(Utils.safeParseInt(15.0), 15);
+    });
+
+    it('should coerce NaN to zero', () => {
+      assert.equal(Utils.safeParseInt('not a number'), 0);
+      assert.equal(Utils.safeParseInt(NaN), 0);
+    });
+  });
 });