THRIFT-3444 Large 64 bit Integer does not preserve value through Node.js JSONProtocol
Client: Node.js
Patch: Nobuaki Sukegawa
This closes #712
diff --git a/LICENSE b/LICENSE
index 8d5e082..a8cf8cd 100644
--- a/LICENSE
+++ b/LICENSE
@@ -251,3 +251,15 @@
ghost@aladdin.com
*/
+--------------------------------------------------
+For the lib/nodejs/lib/thrift/json_parse.js:
+
+/*
+ json_parse.js
+ 2015-05-02
+ Public Domain.
+ NO WARRANTY EXPRESSED OR IMPLIED. USE AT YOUR OWN RISK.
+
+*/
+(By Douglas Crockford <douglas@crockford.com>)
+--------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/int64_util.js b/lib/nodejs/lib/thrift/int64_util.js
new file mode 100644
index 0000000..ecba439
--- /dev/null
+++ b/lib/nodejs/lib/thrift/int64_util.js
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+var Int64 = require('node-int64');
+
+var Int64Util = module.exports = {};
+
+var POW2_24 = Math.pow(2, 24);
+var POW2_31 = Math.pow(2, 31);
+var POW2_32 = Math.pow(2, 32);
+var POW10_11 = Math.pow(10, 11);
+
+Int64Util.toDecimalString = function(i64) {
+ var b = i64.buffer;
+ var o = i64.offset;
+ if ((!b[o] && !(b[o + 1] & 0xe0)) ||
+ (!~b[o] && !~(b[o + 1] & 0xe0))) {
+ // The magnitude is small enough.
+ return i64.toString();
+ } else {
+ var negative = b[o] & 0x80;
+ if (negative) {
+ // 2's complement
+ var incremented = false;
+ var buffer = new Buffer(8);
+ for (var i = 7; i >= 0; --i) {
+ buffer[i] = ~b[o + i] + (incremented ? 0 : 1);
+ incremented |= b[o + i];
+ }
+ b = buffer;
+ }
+ var high2 = b[o + 1] + (b[o] << 8);
+ // Lesser 11 digits with exceeding values but is under 53 bits capacity.
+ var low = b[o + 7] + (b[o + 6] << 8) + (b[o + 5] << 16)
+ + b[o + 4] * POW2_24 // Bit shift renders 32th bit as sign, so use multiplication
+ + (b[o + 3] + (b[o + 2] << 8)) * POW2_32 + high2 * 74976710656; // The literal is 2^48 % 10^11
+ // 12th digit and greater.
+ var high = Math.floor(low / POW10_11) + high2 * 2814; // The literal is 2^48 / 10^11
+ // Make it exactly 11 with leading zeros.
+ low = ('00000000000' + String(low % POW10_11)).slice(-11);
+ return (negative ? '-' : '') + String(high) + low;
+ }
+};
+
+Int64Util.fromDecimalString = function(text) {
+ var negative = text.charAt(0) === '-';
+ if (text.length < (negative ? 17 : 16)) {
+ // The magnitude is smaller than 2^53.
+ return new Int64(+text);
+ } else if (text.length > (negative ? 20 : 19)) {
+ throw new RangeError('Too many digits for Int64: ' + text);
+ } else {
+ // Most significant (up to 5) digits
+ var high5 = +text.slice(negative ? 1 : 0, -15);
+ var low = +text.slice(-15) + high5 * 2764472320; // The literal is 10^15 % 2^32
+ var high = Math.floor(low / POW2_32) + high5 * 232830; // The literal is 10^15 / 2^&32
+ low = low % POW2_32;
+ if (high >= POW2_31 &&
+ !(negative && high == POW2_31 && low == 0) // Allow minimum Int64
+ ) {
+ throw new RangeError('The magnitude is too large for Int64.');
+ }
+ if (negative) {
+ // 2's complement
+ high = ~high;
+ if (low === 0) {
+ high = (high + 1) & 0xffffffff;
+ } else {
+ low = ~low + 1;
+ }
+ high = 0x80000000 | high;
+ }
+ return new Int64(high, low);
+ }
+};
diff --git a/lib/nodejs/lib/thrift/json_parse.js b/lib/nodejs/lib/thrift/json_parse.js
new file mode 100644
index 0000000..93b0bf2
--- /dev/null
+++ b/lib/nodejs/lib/thrift/json_parse.js
@@ -0,0 +1,299 @@
+/*
+ * Imported from Douglas Crockford's reference implementation with minimum modification
+ * to handle Int64.
+ *
+ * https://github.com/douglascrockford/JSON-js/blob/c98948ae1944a28e2e8ebc3717894e580aeaaa05/json_parse.js
+ *
+ * Original license header:
+ *
+ * json_parse.js
+ * 2015-05-02
+ * Public Domain.
+ * NO WARRANTY EXPRESSED OR IMPLIED. USE AT YOUR OWN RISK.
+ */
+
+
+/*jslint for */
+
+/*property
+ at, b, call, charAt, f, fromCharCode, hasOwnProperty, message, n, name,
+ prototype, push, r, t, text
+*/
+
+var Int64 = require('node-int64');
+var Int64Util = require('./int64_util');
+
+var json_parse = module.exports = (function () {
+ "use strict";
+
+// This is a function that can parse a JSON text, producing a JavaScript
+// data structure. It is a simple, recursive descent parser. It does not use
+// eval or regular expressions, so it can be used as a model for implementing
+// a JSON parser in other languages.
+
+// We are defining the function inside of another function to avoid creating
+// global variables.
+
+ var at, // The index of the current character
+ ch, // The current character
+ escapee = {
+ '"': '"',
+ '\\': '\\',
+ '/': '/',
+ b: '\b',
+ f: '\f',
+ n: '\n',
+ r: '\r',
+ t: '\t'
+ },
+ text,
+
+ error = function (m) {
+
+// Call error when something is wrong.
+
+ throw new SyntaxError(m);
+ },
+
+ next = function (c) {
+
+// If a c parameter is provided, verify that it matches the current character.
+
+ if (c && c !== ch) {
+ error("Expected '" + c + "' instead of '" + ch + "'");
+ }
+
+// Get the next character. When there are no more characters,
+// return the empty string.
+
+ ch = text.charAt(at);
+ at += 1;
+ return ch;
+ },
+
+ number = function () {
+
+// Parse a number value.
+
+ var number,
+ string = '';
+
+ if (ch === '-') {
+ string = '-';
+ next('-');
+ }
+ while (ch >= '0' && ch <= '9') {
+ string += ch;
+ next();
+ }
+ if (ch === '.') {
+ string += '.';
+ while (next() && ch >= '0' && ch <= '9') {
+ string += ch;
+ }
+ }
+ if (ch === 'e' || ch === 'E') {
+ string += ch;
+ next();
+ if (ch === '-' || ch === '+') {
+ string += ch;
+ next();
+ }
+ while (ch >= '0' && ch <= '9') {
+ string += ch;
+ next();
+ }
+ }
+ number = +string;
+ if (!isFinite(number)) {
+ error("Bad number");
+ } else if (number >= Int64.MAX_INT || number <= Int64.MIN_INT) {
+ // Return raw string for further process in TJSONProtocol
+ return string;
+ } else {
+ return number;
+ }
+ },
+
+ string = function () {
+
+// Parse a string value.
+
+ var hex,
+ i,
+ string = '',
+ uffff;
+
+// When parsing for string values, we must look for " and \ characters.
+
+ if (ch === '"') {
+ while (next()) {
+ if (ch === '"') {
+ next();
+ return string;
+ }
+ if (ch === '\\') {
+ next();
+ if (ch === 'u') {
+ uffff = 0;
+ for (i = 0; i < 4; i += 1) {
+ hex = parseInt(next(), 16);
+ if (!isFinite(hex)) {
+ break;
+ }
+ uffff = uffff * 16 + hex;
+ }
+ string += String.fromCharCode(uffff);
+ } else if (typeof escapee[ch] === 'string') {
+ string += escapee[ch];
+ } else {
+ break;
+ }
+ } else {
+ string += ch;
+ }
+ }
+ }
+ error("Bad string");
+ },
+
+ white = function () {
+
+// Skip whitespace.
+
+ while (ch && ch <= ' ') {
+ next();
+ }
+ },
+
+ word = function () {
+
+// true, false, or null.
+
+ switch (ch) {
+ case 't':
+ next('t');
+ next('r');
+ next('u');
+ next('e');
+ return true;
+ case 'f':
+ next('f');
+ next('a');
+ next('l');
+ next('s');
+ next('e');
+ return false;
+ case 'n':
+ next('n');
+ next('u');
+ next('l');
+ next('l');
+ return null;
+ }
+ error("Unexpected '" + ch + "'");
+ },
+
+ value, // Place holder for the value function.
+
+ array = function () {
+
+// Parse an array value.
+
+ var array = [];
+
+ if (ch === '[') {
+ next('[');
+ white();
+ if (ch === ']') {
+ next(']');
+ return array; // empty array
+ }
+ while (ch) {
+ array.push(value());
+ white();
+ if (ch === ']') {
+ next(']');
+ return array;
+ }
+ next(',');
+ white();
+ }
+ }
+ error("Bad array");
+ },
+
+ object = function () {
+
+// Parse an object value.
+
+ var key,
+ object = {};
+
+ if (ch === '{') {
+ next('{');
+ white();
+ if (ch === '}') {
+ next('}');
+ return object; // empty object
+ }
+ while (ch) {
+ key = string();
+ white();
+ next(':');
+ if (Object.hasOwnProperty.call(object, key)) {
+ error('Duplicate key "' + key + '"');
+ }
+ object[key] = value();
+ white();
+ if (ch === '}') {
+ next('}');
+ return object;
+ }
+ next(',');
+ white();
+ }
+ }
+ error("Bad object");
+ };
+
+ value = function () {
+
+// Parse a JSON value. It could be an object, an array, a string, a number,
+// or a word.
+
+ white();
+ switch (ch) {
+ case '{':
+ return object();
+ case '[':
+ return array();
+ case '"':
+ return string();
+ case '-':
+ return number();
+ default:
+ return ch >= '0' && ch <= '9'
+ ? number()
+ : word();
+ }
+ };
+
+// Return the json_parse function. It will have access to all of the above
+// functions and variables.
+
+ return function (source) {
+ var result;
+
+ text = source;
+ at = 0;
+ ch = ' ';
+ result = value();
+ white();
+ if (ch) {
+ error("Syntax error");
+ }
+
+ return result;
+ };
+}());
diff --git a/lib/nodejs/lib/thrift/json_protocol.js b/lib/nodejs/lib/thrift/json_protocol.js
index 436709c..b98f131 100644
--- a/lib/nodejs/lib/thrift/json_protocol.js
+++ b/lib/nodejs/lib/thrift/json_protocol.js
@@ -24,6 +24,9 @@
var Type = Thrift.Type;
var util = require("util");
+var Int64Util = require('./int64_util');
+var json_parse = require('./json_parse');
+
var InputBufferUnderrunError = require('./input_buffer_underrun_error');
module.exports = TJSONProtocol;
@@ -308,7 +311,11 @@
/** Serializes a number */
TJSONProtocol.prototype.writeI64 = function(i64) {
- this.tstack.push(i64);
+ if (i64 instanceof Int64) {
+ this.tstack.push(Int64Util.toDecimalString(i64));
+ } else {
+ this.tstack.push(i64);
+ }
};
/** Serializes a number */
@@ -439,13 +446,13 @@
}
//Reconstitute the JSON object and conume the necessary bytes
- this.robj = JSON.parse(transBuf.buf.slice(transBuf.readIndex, cursor+1));
+ this.robj = json_parse(transBuf.buf.slice(transBuf.readIndex, cursor+1).toString());
this.trans.consume(cursor + 1 - transBuf.readIndex);
//Verify the protocol version
var version = this.robj.shift();
if (version != TJSONProtocol.Version) {
- throw 'Wrong thrift protocol version: ' + version;
+ throw new Error('Wrong thrift protocol version: ' + version);
}
//Objectify the thrift message {name/type/sequence-number} for return
@@ -628,28 +635,24 @@
return this.readListEnd();
};
-/** Returns an object with a value property set to
- * False unless the next number in the protocol buffer
- * is 1, in which case the value property is True */
TJSONProtocol.prototype.readBool = function() {
- return this.readI32() == '1';
+ return this.readValue() == '1';
};
-/** Returns the an object with a value property set to the
- next value found in the protocol buffer */
TJSONProtocol.prototype.readByte = function() {
return this.readI32();
};
-/** Returns the an object with a value property set to the
- next value found in the protocol buffer */
TJSONProtocol.prototype.readI16 = function() {
return this.readI32();
};
-/** Returns the an object with a value property set to the
- next value found in the protocol buffer */
TJSONProtocol.prototype.readI32 = function(f) {
+ return +this.readValue();
+}
+
+/** Returns the next value found in the protocol buffer */
+TJSONProtocol.prototype.readValue = function(f) {
if (f === undefined) {
f = this.rstack[this.rstack.length - 1];
}
@@ -662,7 +665,7 @@
} else {
r.value = f.shift();
}
- } else if (f instanceof Object) {
+ } else if (!(f instanceof Int64) && f instanceof Object) {
for (var i in f) {
if (i === null) {
continue;
@@ -681,28 +684,26 @@
return r.value;
};
-/** Returns the an object with a value property set to the
- next value found in the protocol buffer */
TJSONProtocol.prototype.readI64 = function() {
- return new Int64(this.readI32());
+ var n = this.readValue()
+ if (typeof n === 'string') {
+ // Assuming no one is sending in 1.11111e+33 format
+ return Int64Util.fromDecimalString(n);
+ } else {
+ return new Int64(n);
+ }
};
-/** Returns the an object with a value property set to the
- next value found in the protocol buffer */
TJSONProtocol.prototype.readDouble = function() {
return this.readI32();
};
-/** Returns the an object with a value property set to the
- next value found in the protocol buffer */
TJSONProtocol.prototype.readBinary = function() {
- return new Buffer(this.readString(), 'base64');
+ return new Buffer(this.readValue(), 'base64');
};
-/** Returns the an object with a value property set to the
- next value found in the protocol buffer */
TJSONProtocol.prototype.readString = function() {
- return this.readI32();
+ return this.readValue();
};
/**
@@ -718,5 +719,5 @@
* Method to arbitrarily skip over data
*/
TJSONProtocol.prototype.skip = function(type) {
- throw 'skip not supported yet';
+ throw new Error('skip not supported yet');
};
diff --git a/lib/nodejs/test/test-cases.js b/lib/nodejs/test/test-cases.js
index 6953e1a..13722be 100644
--- a/lib/nodejs/test/test-cases.js
+++ b/lib/nodejs/test/test-cases.js
@@ -1,6 +1,7 @@
'use strict';
var ttypes = require('./gen-nodejs/ThriftTest_types');
+var Int64 = require('node-int64');
//all Languages in UTF-8
/*jshint -W100 */
@@ -58,15 +59,14 @@
['testI32', -1],
['testDouble', -5.2098523],
['testDouble', 7.012052175215044],
- ['testEnum', ttypes.Numberz.ONE]
-];
-
-var simpleLoose = [
+ ['testEnum', ttypes.Numberz.ONE],
['testI64', 5],
['testI64', -5],
['testI64', 734359738368],
- ['testI64', -34359738368],
['testI64', -734359738368],
+ ['testI64', new Int64(new Buffer([0, 0x20, 0, 0, 0, 0, 0, 1]))], // 2^53+1
+ ['testI64', new Int64(
+ new Buffer([0xff, 0xdf, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff]))], // -2^53-1
['testTypedef', 69]
]
@@ -134,7 +134,6 @@
};
module.exports.simple = simple;
-module.exports.simpleLoose = simpleLoose;
module.exports.deep = deep;
module.exports.deepUnordered = deepUnordered;
diff --git a/lib/nodejs/test/test_driver.js b/lib/nodejs/test/test_driver.js
index 590d583..03ec513 100644
--- a/lib/nodejs/test/test_driver.js
+++ b/lib/nodejs/test/test_driver.js
@@ -50,9 +50,13 @@
};
}
- testCases.simple.forEach(makeAsserter(assert.equal));
- testCases.simpleLoose.forEach(makeAsserter(function(a, e, m){
- assert.ok(a == e, m);
+ testCases.simple.forEach(makeAsserter(function(a, e, m){
+ if (a instanceof Int64) {
+ var e64 = e instanceof Int64 ? e : new Int64(e);
+ assert.deepEqual(a.buffer, e64.buffer, m);
+ } else {
+ assert.equal(a, e, m);
+ }
}));
testCases.deep.forEach(makeAsserter(assert.deepEqual));
testCases.deepUnordered.forEach(makeAsserter(makeUnorderedDeepEqual(assert)));
@@ -160,9 +164,13 @@
};
}
- testCases.simple.forEach(makeAsserter(assert.equal));
- testCases.simpleLoose.forEach(makeAsserter(function(a, e, m){
- assert.ok(a == e, m);
+ testCases.simple.forEach(makeAsserter(function(a, e, m){
+ if (a instanceof Int64) {
+ var e64 = e instanceof Int64 ? e : new Int64(e);
+ assert.deepEqual(a.buffer, e64.buffer, m);
+ } else {
+ assert.equal(a, e, m);
+ }
}));
testCases.deep.forEach(makeAsserter(assert.deepEqual));
testCases.deepUnordered.forEach(makeAsserter(makeUnorderedDeepEqual(assert)));
diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json
index b98e6c1..b05f61c 100644
--- a/test/known_failures_Linux.json
+++ b/test/known_failures_Linux.json
@@ -98,24 +98,13 @@
"nodejs-csharp_json_buffered-ip-ssl",
"nodejs-csharp_json_framed-ip",
"nodejs-csharp_json_framed-ip-ssl",
- "nodejs-dart_json_buffered-ip",
- "nodejs-dart_json_framed-ip",
"nodejs-hs_binary_buffered-ip",
"nodejs-hs_binary_framed-ip",
"nodejs-hs_compact_buffered-ip",
"nodejs-hs_compact_framed-ip",
"nodejs-hs_json_buffered-ip",
"nodejs-hs_json_framed-ip",
- "nodejs-py3_json_buffered-ip",
- "nodejs-py3_json_buffered-ip-ssl",
- "nodejs-py3_json_framed-ip",
- "nodejs-py3_json_framed-ip-ssl",
- "nodejs-py_json_buffered-ip",
- "nodejs-py_json_buffered-ip-ssl",
- "nodejs-py_json_framed-ip",
- "nodejs-py_json_framed-ip-ssl",
"nodejs-rb_json_buffered-ip",
- "nodejs-rb_json_framed-ip",
"perl-php_binary_framed-ip",
"py-hs_json_buffered-ip",
"py-hs_json_framed-ip",