diff options
author | Nobuaki Sukegawa <nsuke@apache.org> | 2015-11-23 19:38:18 +0900 |
---|---|---|
committer | Nobuaki Sukegawa <nsuke@apache.org> | 2015-12-02 02:03:59 +0900 |
commit | f56b90772f5a60e08c88388af753a71d519591c3 (patch) | |
tree | 5c7da8f6fce2bb9ddb5789292f5004e23133c6c9 | |
parent | aede97523d1c587c2ed5832cde60f8948c68bcd5 (diff) | |
download | thrift-f56b90772f5a60e08c88388af753a71d519591c3.tar.gz |
THRIFT-3444 Large 64 bit Integer does not preserve value through Node.js JSONProtocol
Client: Node.js
Patch: Nobuaki Sukegawa
This closes #712
-rw-r--r-- | LICENSE | 12 | ||||
-rw-r--r-- | lib/nodejs/lib/thrift/int64_util.js | 91 | ||||
-rw-r--r-- | lib/nodejs/lib/thrift/json_parse.js | 299 | ||||
-rw-r--r-- | lib/nodejs/lib/thrift/json_protocol.js | 53 | ||||
-rw-r--r-- | lib/nodejs/test/test-cases.js | 11 | ||||
-rw-r--r-- | lib/nodejs/test/test_driver.js | 20 | ||||
-rw-r--r-- | test/known_failures_Linux.json | 11 |
7 files changed, 448 insertions, 49 deletions
@@ -251,3 +251,15 @@ For the compiler/cpp/src/thrift/md5.[ch] components: 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 000000000..ecba43927 --- /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 000000000..93b0bf2ab --- /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 436709c98..b98f13157 100644 --- a/lib/nodejs/lib/thrift/json_protocol.js +++ b/lib/nodejs/lib/thrift/json_protocol.js @@ -24,6 +24,9 @@ var Thrift = require('./thrift'); 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 @@ TJSONProtocol.prototype.writeI32 = function(i32) { /** 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 @@ TJSONProtocol.prototype.readMessageBegin = function() { } //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 @@ TJSONProtocol.prototype.readSetEnd = function() { 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 @@ TJSONProtocol.prototype.readI32 = function(f) { } 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 @@ TJSONProtocol.prototype.readI32 = function(f) { 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 @@ TJSONProtocol.prototype.getTransport = function() { * 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 6953e1aae..13722be8b 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 @@ var simple = [ ['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 @@ var insanity = { }; 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 590d58337..03ec5138b 100644 --- a/lib/nodejs/test/test_driver.js +++ b/lib/nodejs/test/test_driver.js @@ -50,9 +50,13 @@ exports.ThriftTestDriver = function(client, callback) { }; } - 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 @@ exports.ThriftTestDriverPromise = function(client, callback) { }; } - 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 b98e6c156..b05f61ce6 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", |