From 999db7d780703cb9573791328c21716a9eb34d81 Mon Sep 17 00:00:00 2001 From: Rahul Padigela Date: Tue, 7 Jun 2016 16:14:51 -0700 Subject: [PATCH 1/2] FT: Logs should not be able to overwrite reserved fields Fix #50 --- lib/Utils.js | 16 +++++++++++++++- tests/unit/Utils.js | 27 +++++++++++++++++++++------ 2 files changed, 36 insertions(+), 7 deletions(-) diff --git a/lib/Utils.js b/lib/Utils.js index f864f3c..24ce3e7 100644 --- a/lib/Utils.js +++ b/lib/Utils.js @@ -40,6 +40,17 @@ function unserializeUids(stringdata) { return stringdata.split(':'); } +/** This function checks if a field name collides with internal data field name +* @param {string} field - field name being logged +* +* @returns {bool} - Returns true if it collides with reserved fields +* and false otherwise +*/ +const _reservedFields = new Set([ 'name', 'level', 'time', 'message', + 'req_id', 'hostname', 'elapsed_ms', 'pid', 'tags', '@timestamp' ]); +function _isReservedField(field) { + return _reservedFields.has(field); +} /** * This function copies the properties from the source object to the target object * @param {...object} target - object to be copied to @@ -49,7 +60,10 @@ function objectCopy(target) { const result = target; let source; function copy(f) { - result[f] = source[f]; + // do not copy reserved fields + if (!_isReservedField(f)) { + result[f] = source[f]; + } } for (let i = 1; i < arguments.length; i++) { source = arguments[i]; diff --git a/tests/unit/Utils.js b/tests/unit/Utils.js index 6f18185..9b246f4 100644 --- a/tests/unit/Utils.js +++ b/tests/unit/Utils.js @@ -47,8 +47,8 @@ describe('Utils: serializeUids', () => { describe('Utils: objectCopy', () => { it('copies all the properties from source to target object', (done) => { const target = { foo: 'bar' }; - const source = { id: 1, name: 'demo', value: { a: 1, b: 2, c: 3 } }; - const result = { foo: 'bar', id: 1, name: 'demo', value: { a: 1, b: 2, c: 3 } }; + const source = { id: 1, prj: 'demo', value: { a: 1, b: 2, c: 3 } }; + const result = { foo: 'bar', id: 1, prj: 'demo', value: { a: 1, b: 2, c: 3 } }; objectCopy(target, source); assert.deepStrictEqual(target, result, 'target should have the same properties as source'); done(); @@ -56,12 +56,27 @@ describe('Utils: objectCopy', () => { it('copies all the properties from multiple sources to target object', (done) => { const target = { foo: 'bar' }; - const source1 = { id: 1, name: 'demo1', value: { a: 1, b: 2, c: 3 } }; - const source2 = { req_id: 2, method: 'test', err: { code: 'error', msg: 'test' } }; - const result = { foo: 'bar', id: 1, name: 'demo1', value: { a: 1, b: 2, c: 3 }, - req_id: 2, method: 'test', err: { code: 'error', msg: 'test' }}; + const source1 = { id: 1, prj: 'demo1', value: { a: 1, b: 2, c: 3 } }; + const source2 = { uid: 2, method: 'test', err: { code: 'error', msg: 'test' } }; + const result = { foo: 'bar', id: 1, prj: 'demo1', value: { a: 1, b: 2, c: 3 }, + uid: 2, method: 'test', err: { code: 'error', msg: 'test' }}; objectCopy(target, source1, source2); assert.deepStrictEqual(target, result, 'target should have the same properties as source'); done(); }); + + it('should not copy reserved fields from source to target object', done => { + const target = { name: 'werelogs', level: 'info', message: 'hello', + time: 12345, hostname: 'host', pid: 1234, elapsed_ms: 100 }; + const source = { foo: 'bar', method: 'test', name: 'demo', pid: 41, + message: 'hello', hostname: 'machine', time: 7890, elapsed_ms: 200, + }; + const result = { name: 'werelogs', level: 'info', message: 'hello', + time: 12345, hostname: 'host', pid: 1234, elapsed_ms: 100, + foo: 'bar', method: 'test'}; + objectCopy(target, source); + assert.deepStrictEqual(target, result, 'target\'s reserved fields' + + ' should not be overwritten'); + done(); + }); }); From 6e2ae6b1d215d7c1da86eac43b77d6677870aec8 Mon Sep 17 00:00:00 2001 From: Rahul Padigela Date: Tue, 7 Jun 2016 18:39:01 -0700 Subject: [PATCH 2/2] cleanup: set request logger name --- lib/Logger.js | 4 ++-- lib/RequestLogger.js | 25 +++++++++++++++++++------ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/Logger.js b/lib/Logger.js index ffecc9e..be7d01a 100644 --- a/lib/Logger.js +++ b/lib/Logger.js @@ -61,7 +61,7 @@ class Logger { const rLog = new RequestLogger(Config.logger, Config.level, Config.dump, Config.end, uids); - rLog.addDefaultFields({name: this.name}); + rLog.setName(this.name); return rLog; } @@ -77,7 +77,7 @@ class Logger { const rLog = new RequestLogger(Config.logger, Config.level, Config.dump, Config.end, unserializeUids(serializedUids)); - rLog.addDefaultFields({name: this.name}); + rLog.setName(this.name); return rLog; } diff --git a/lib/RequestLogger.js b/lib/RequestLogger.js index 99cbdb7..3107a93 100644 --- a/lib/RequestLogger.js +++ b/lib/RequestLogger.js @@ -205,6 +205,19 @@ class RequestLogger { this.elapsedTime = null; } + /** + * This function sets the name field for the request logger. + * + * @note As this is a protected field, it will not be copied/overwritten + * while logging objects. + * @param {string} name - name to be set for the request logger + * @returns {string} - name field of the request logger + */ + setName(name) { + this.fields.name = name; + return name; + } + /** * This function returns a copy of the local uid list, in order for * requests to a sub-component to be sent with the parent request's UID @@ -434,26 +447,26 @@ class RequestLogger { }); return; } - const fields = objectCopy({}, this.fields, logFields || {}); + objectCopy(this.fields, logFields || {}); const endFlag = isEnd || false; /* * using Date.now() as it's faster than new Date(). logstash component * uses this field to generate ISO 8601 timestamp */ - if (fields.time === undefined) { - fields.time = Date.now(); + if (this.fields.time === undefined) { + this.fields.time = Date.now(); } - fields.req_id = serializeUids(this.uids); + this.fields.req_id = serializeUids(this.uids); if (endFlag) { this.elapsedTime = process.hrtime(this.startTime); - fields.elapsed_ms = this.elapsedTime[0] * 1000 + this.fields.elapsed_ms = this.elapsedTime[0] * 1000 + this.elapsedTime[1] / 1000000; } const logEntry = { level, - fields, + fields: this.fields, msg, }; this.entries.push(logEntry);