From 0dd35a6c4b6e211c0fec867d279cee5172280f4b Mon Sep 17 00:00:00 2001 From: Wade Simmons Date: Tue, 1 Jul 2014 12:18:30 -0700 Subject: [PATCH] fix global leaks and copy-paste errors - TCompactProtocol.prototype.writeBool not referencing `booleanField_` on `this` correctly. Also using `NULL` instead of `null`. - TCompactProtocol.prototype.writeVarint64 had a typo for TProtocolException - TCompactProtocol.prototype.readMapBegin had a typo between `kvtype` and `kvType` - createMultiplexServer leaked a global var `processStatus` - TFramedTransport had a line that was accidently copy pasted, leading to a global leak of `frameLeft`. (I created the patch the introduced this copy paste error in Thrift-1353, so I can confirm it was a mistake). - createWebServer tried to check a non-existent var `route` and leaked a global `result` Signed-off-by: Roger Meier --- lib/nodejs/lib/thrift/protocol.js | 14 +++++++------- lib/nodejs/lib/thrift/server.js | 2 +- lib/nodejs/lib/thrift/transport.js | 1 - lib/nodejs/lib/thrift/web_server.js | 8 +++----- 4 files changed, 11 insertions(+), 14 deletions(-) diff --git a/lib/nodejs/lib/thrift/protocol.js b/lib/nodejs/lib/thrift/protocol.js index 8ccb55d34..9bfe26825 100644 --- a/lib/nodejs/lib/thrift/protocol.js +++ b/lib/nodejs/lib/thrift/protocol.js @@ -652,14 +652,14 @@ TCompactProtocol.prototype.writeSetEnd = function() { }; TCompactProtocol.prototype.writeBool = function(value) { - if (booleanField_.name != NULL) { + if (this.booleanField_.name !== null) { // we haven't written the field header yet - this.writeFieldBeginInternal(booleanField_.name, - booleanField_.fieldType, - booleanField_.fieldId, + this.writeFieldBeginInternal(this.booleanField_.name, + this.booleanField_.fieldType, + this.booleanField_.fieldId, (value ? TCompactProtocol.Types.CT_BOOLEAN_TRUE : TCompactProtocol.Types.CT_BOOLEAN_FALSE)); - booleanField_.name = NULL; + this.booleanField_.name = null; } else { // we're not part of a field, so just write the value this.writeByte((value ? TCompactProtocol.Types.CT_BOOLEAN_TRUE @@ -765,7 +765,7 @@ TCompactProtocol.prototype.writeVarint64 = function(n) { n = new Int64(n); } if (! (n instanceof Int64)) { - throw new TProtocolError(INVALID_DATA, "Expected Int64 or Number, found: " + n); + throw new TProtocolException(INVALID_DATA, "Expected Int64 or Number, found: " + n); } var buf = new Buffer(10); @@ -903,7 +903,7 @@ TCompactProtocol.prototype.readMapBegin = function() { throw new TProtocolException(NEGATIVE_SIZE, "Negative map size"); } - var kvtype = 0; + var kvType = 0; if (msize !== 0) { kvType = this.trans.readByte(); } diff --git a/lib/nodejs/lib/thrift/server.js b/lib/nodejs/lib/thrift/server.js index 383d1f5af..313a80040 100644 --- a/lib/nodejs/lib/thrift/server.js +++ b/lib/nodejs/lib/thrift/server.js @@ -52,7 +52,7 @@ exports.createMultiplexServer = function(processor, options) { try { do { - processStatus = processor.process(input, output); + processor.process(input, output); transportWithData.commitPosition(); } while (true); } catch (err) { diff --git a/lib/nodejs/lib/thrift/transport.js b/lib/nodejs/lib/thrift/transport.js index 900472c2f..6d4224acb 100644 --- a/lib/nodejs/lib/thrift/transport.js +++ b/lib/nodejs/lib/thrift/transport.js @@ -157,7 +157,6 @@ TFramedTransport.prototype = { // TODO: optimize this better, allocate one buffer instead of both: var msg = new Buffer(out.length + 4); binary.writeI32(msg, out.length); - frameLeft = binary.readI32(this.inBuf, 0); out.copy(msg, 4, 0, out.length); this.onFlush(msg); } diff --git a/lib/nodejs/lib/thrift/web_server.js b/lib/nodejs/lib/thrift/web_server.js index dd7ad5f33..e6f6f9792 100644 --- a/lib/nodejs/lib/thrift/web_server.js +++ b/lib/nodejs/lib/thrift/web_server.js @@ -506,10 +506,8 @@ exports.createWebServer = function(options) { try { svc = services[Object.keys(services)[0]]; } catch(e) { - if (!route) { - socket.write("HTTP/1.1 403 No Apache Thrift Service availible\r\n\r\n"); - return; - } + socket.write("HTTP/1.1 403 No Apache Thrift Service availible\r\n\r\n"); + return; } //Perform upgrade var hash = crypto.createHash("sha1"); @@ -524,7 +522,7 @@ exports.createWebServer = function(options) { socket.on('data', function(frame) { try { while (frame) { - result = wsFrame.decode(frame); + var result = wsFrame.decode(frame); //Prepend any existing decoded data if (data) { if (result.data) {