Fix thrift negative integer handling.

lib/column_family.js

@@ -368,7 +368,12 @@ ColumnFamily.prototype.get = function(key, options, callback){
     if(options.count === true){
       callback(null, val);
     } else {
-      callback(null, Row.fromThrift(key, val, self));
+      try {
+        var row = Row.fromThrift(key, val, self);
+      } catch(e) {
+        return callback(e);
+      }
+      callback(null, row);
     }
   }
 

lib/marshal/deserializers.js

@@ -1,5 +1,7 @@
 var UUID = require('../uuid').UUID,
-    TimeUUID = require('../uuid').TimeUUID;
+    TimeUUID = require('../uuid').TimeUUID,
+    Int64 = require('node-int64'),
+    util = require('util');
 
 /**
  * Deserializers for various types
@@ -8,14 +10,13 @@ var UUID = require('../uuid').UUID,
 var Deserializers = {};
 
 var decodeLongFromBuffer = function(buf) {
-    var negative = buf.readInt8(0) < 0;
-    if(negative) {
-      //TODO: replace this with a better negative reading function that does up to 53 bytes
-      return buf.readInt32BE(4);
-    } else {
-      return parseInt(buf.toString('hex'), 16);
-    }
-}
+  var int64 = new Int64(buf);
+  var ret = int64.valueOf();
+  if (ret === Infinity || ret === -Infinity) {
+    throw new Error("Integer value out of range for Javascript (" + util.inspect(buf) + ")");
+  }
+  return ret;
+};
 
 Deserializers.decodeInteger = function(bits, val, signed){
   if(signed === null || signed === undefined){

lib/marshal/serializers.js

@@ -1,4 +1,5 @@
-var UUID = require('../uuid');
+var UUID = require('../uuid'),
+    Int64 = require('node-int64');
 
 /**
  * Serializers for various types
@@ -14,22 +15,19 @@ var Serializers = {};
  * @static
  */
 Serializers.encodeInteger = function(bits, num){
-  var buf = new Buffer(bits/8), func;
-
   if(typeof num !== 'number'){
     num = parseInt(num, 10);
   }
 
   switch(bits){
-    case 32: buf.writeUInt32BE(num, 0); break;
-    case 64:
-      var hex = num.toString(16);
-      hex = new Array(17 - hex.length).join('0') + hex;
-      buf.writeUInt32BE(parseInt(hex.slice( 0, 8), 16), 0);
-      buf.writeUInt32BE(parseInt(hex.slice( 8, 16), 16), 4);
+  case 32:
+    var buf = new Buffer(4);
+    buf.writeInt32BE(num, 0);
+    return buf;
+  case 64:
+    var int64 = new Int64(num);
+    return int64.buffer;
   }
-
-  return buf;
 };
 
 /**

lib/row.js

@@ -47,7 +47,12 @@ var Row = function(data, schema){
       if(schema.noDeserialize) {
         this.push(new Column(name,item.value, item.timestamp, item.ttl));
       } else {
-        this.push(new Column(name,deserializeValue(item.value), item.timestamp, item.ttl));
+        try {
+          var val = deserializeValue(item.value);
+        } catch(e) {
+          throw new Error("Error deserializing value for column " + name + ": " + e.message);
+        }
+        this.push(new Column(name, val, item.timestamp, item.ttl));
       }
 
       this._map[name] = this.length - 1;

package.json

@@ -15,6 +15,7 @@
   , "dependencies": {
       "helenus-thrift": "0.7.4"
     , "node-uuid": "1.3.3"
+    , "node-int64": "0.3.0"
     }
   , "optionalDependencies": {
       "microtime": "0.4.0"

test/thrift.js

@@ -493,6 +493,54 @@ module.exports = {
     });
   },
 
+  'test standard cf with LongType 32bit-signed':function(test, assert){
+    var key = config.standard_row_key + '-long',
+        opts = { 'long-test' : -707 };
+
+    cf_standard.insert(key, opts, function(err){
+      assert.ifError(err);
+      cf_standard.get(key, function(err, row){
+        assert.ifError(err);
+        var col = row.get('long-test');
+        assert.ok(typeof col.value === 'number');
+        assert.ok(col.value === -707 );
+        test.finish();
+      });
+    });
+  },
+
+  'test standard cf with LongType 64bit-signed':function(test, assert){
+    var key = config.standard_row_key + '-long',
+        opts = { 'long-test' : -8589934592 };
+
+    cf_standard.insert(key, opts, function(err){
+      assert.ifError(err);
+      cf_standard.get(key, function(err, row){
+        assert.ifError(err);
+        var col = row.get('long-test');
+        assert.ok(typeof col.value === 'number');
+        assert.ok(col.value === -8589934592 );
+        test.finish();
+      });
+    });
+  },
+
+  'test standard cf with LongType out of range error':function(test, assert){
+    // this number happens to be representable so we can insert it,
+    // but it is > 2**53 which should trigger our check when reading
+    var key = config.standard_row_key + '-long',
+        opts = { 'long-test' : 1152921504606847000 };
+
+    cf_standard.insert(key, opts, function(err){
+      assert.ifError(err);
+      cf_standard.get(key, function(err, row){
+        assert.ok(err);
+        assert.ok(err.message.match(/long-test.*out of range/));
+        test.finish();
+      });
+    });
+  },
+
   'test standard cf with IntegerType':function(test, assert){
     var key = config.standard_row_key + '-integer',
         opts = { 'integer-test' : 1234 };
@@ -509,6 +557,22 @@ module.exports = {
     });
   },
 
+  'test standard cf with IntegerType 32bit-signed':function(test, assert){
+    var key = config.standard_row_key + '-integer',
+        opts = { 'integer-test' : -1234 };
+
+    cf_standard.insert(key, opts, function(err){
+      assert.ifError(err);
+      cf_standard.get(key, function(err, row){
+        assert.ifError(err);
+        var col = row.get('integer-test');
+        assert.ok(typeof col.value === 'number');
+        assert.ok(col.value === -1234);
+        test.finish();
+      });
+    });
+  },
+
   'test standard cf with UTF8Type':function(test, assert){
     var key = config.standard_row_key + '-utf8',
         opts = { 'utf8-test' : 'åbcd' };