You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@thrift.apache.org by ns...@apache.org on 2015/11/06 13:26:56 UTC
[4/4] thrift git commit: THRIFT-3409 NodeJS binary field issues
Client: Node.js Patch: Nobuaki Sukegawa
THRIFT-3409 NodeJS binary field issues
Client: Node.js
Patch: Nobuaki Sukegawa
This closes #681
Project: http://git-wip-us.apache.org/repos/asf/thrift/repo
Commit: http://git-wip-us.apache.org/repos/asf/thrift/commit/8a4d06fe
Tree: http://git-wip-us.apache.org/repos/asf/thrift/tree/8a4d06fe
Diff: http://git-wip-us.apache.org/repos/asf/thrift/diff/8a4d06fe
Branch: refs/heads/master
Commit: 8a4d06febe8bc2e1bd84f955b1c2f0149665a0be
Parents: a185d7e
Author: Nobuaki Sukegawa <ns...@apache.org>
Authored: Fri Nov 6 21:24:26 2015 +0900
Committer: Nobuaki Sukegawa <ns...@apache.org>
Committed: Fri Nov 6 21:25:25 2015 +0900
----------------------------------------------------------------------
lib/nodejs/lib/thrift/binary_protocol.js | 42 +++++++++++++---------
lib/nodejs/lib/thrift/compact_protocol.js | 48 +++++++++++++++++---------
lib/nodejs/lib/thrift/json_protocol.js | 24 ++++++++++---
lib/nodejs/test/test_driver.js | 17 +++++++++
test/known_failures_Linux.json | 44 ++++-------------------
5 files changed, 101 insertions(+), 74 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/thrift/blob/8a4d06fe/lib/nodejs/lib/thrift/binary_protocol.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/binary_protocol.js b/lib/nodejs/lib/thrift/binary_protocol.js
index a230291..25e5634 100644
--- a/lib/nodejs/lib/thrift/binary_protocol.js
+++ b/lib/nodejs/lib/thrift/binary_protocol.js
@@ -146,22 +146,10 @@ TBinaryProtocol.prototype.writeDouble = function(dub) {
this.trans.write(binary.writeDouble(new Buffer(8), dub));
};
-TBinaryProtocol.prototype.writeString = function(arg) {
+TBinaryProtocol.prototype.writeStringOrBinary = function(name, encoding, arg) {
if (typeof(arg) === 'string') {
- this.writeI32(Buffer.byteLength(arg, 'utf8'));
- this.trans.write(arg, 'utf8');
- } else if (arg instanceof Buffer) {
- this.writeI32(arg.length);
- this.trans.write(arg);
- } else {
- throw new Error('writeString called without a string/Buffer argument: ' + arg);
- }
-};
-
-TBinaryProtocol.prototype.writeBinary = function(arg) {
- if (typeof(arg) === 'string') {
- this.writeI32(Buffer.byteLength(arg, 'utf8'));
- this.trans.write(arg, 'utf8');
+ this.writeI32(Buffer.byteLength(arg, encoding));
+ this.trans.write(new Buffer(arg, encoding));
} else if ((arg instanceof Buffer) ||
(Object.prototype.toString.call(arg) == '[object Uint8Array]')) {
// Buffers in Node.js under Browserify may extend UInt8Array instead of
@@ -170,10 +158,18 @@ TBinaryProtocol.prototype.writeBinary = function(arg) {
this.writeI32(arg.length);
this.trans.write(arg);
} else {
- throw new Error('writeBinary called without a string/Buffer argument: ' + arg);
+ throw new Error(name + ' called without a string/Buffer argument: ' + arg);
}
};
+TBinaryProtocol.prototype.writeString = function(arg) {
+ this.writeStringOrBinary('writeString', 'utf8', arg);
+};
+
+TBinaryProtocol.prototype.writeBinary = function(arg) {
+ this.writeStringOrBinary('writeBinary', 'binary', arg);
+};
+
TBinaryProtocol.prototype.readMessageBegin = function() {
var sz = this.readI32();
var type, name, seqid;
@@ -279,11 +275,25 @@ TBinaryProtocol.prototype.readDouble = function() {
TBinaryProtocol.prototype.readBinary = function() {
var len = this.readI32();
+ if (len === 0) {
+ return new Buffer();
+ }
+
+ if (len < 0) {
+ throw new Thrift.TProtocolException(Thrift.TProtocolExceptionType.NEGATIVE_SIZE, "Negative binary size");
+ }
return this.trans.read(len);
};
TBinaryProtocol.prototype.readString = function() {
var len = this.readI32();
+ if (len === 0) {
+ return "";
+ }
+
+ if (len < 0) {
+ throw new Thrift.TProtocolException(Thrift.TProtocolExceptionType.NEGATIVE_SIZE, "Negative string size");
+ }
return this.trans.readString(len);
};
http://git-wip-us.apache.org/repos/asf/thrift/blob/8a4d06fe/lib/nodejs/lib/thrift/compact_protocol.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/compact_protocol.js b/lib/nodejs/lib/thrift/compact_protocol.js
index 8aee808..deecf48 100644
--- a/lib/nodejs/lib/thrift/compact_protocol.js
+++ b/lib/nodejs/lib/thrift/compact_protocol.js
@@ -429,22 +429,30 @@ TCompactProtocol.prototype.writeDouble = function(v) {
this.trans.write(buff);
};
-TCompactProtocol.prototype.writeString = function(arg) {
- this.writeBinary(arg);
-};
-
-TCompactProtocol.prototype.writeBinary = function(arg) {
+TCompactProtocol.prototype.writeStringOrBinary = function(name, encoding, arg) {
if (typeof arg === 'string') {
- this.writeVarint32(Buffer.byteLength(arg, 'utf8')) ;
- this.trans.write(arg, 'utf8');
- } else if (arg instanceof Buffer) {
+ this.writeVarint32(Buffer.byteLength(arg, encoding)) ;
+ this.trans.write(new Buffer(arg, encoding));
+ } else if (arg instanceof Buffer ||
+ Object.prototype.toString.call(arg) == '[object Uint8Array]') {
+ // Buffers in Node.js under Browserify may extend UInt8Array instead of
+ // defining a new object. We detect them here so we can write them
+ // correctly
this.writeVarint32(arg.length);
this.trans.write(arg);
} else {
- throw new Error('writeString/writeBinary called without a string/Buffer argument: ' + arg);
+ throw new Error(name + ' called without a string/Buffer argument: ' + arg);
}
};
+TCompactProtocol.prototype.writeString = function(arg) {
+ this.writeStringOrBinary('writeString', 'utf8', arg);
+};
+
+TCompactProtocol.prototype.writeBinary = function(arg) {
+ this.writeStringOrBinary('writeBinary', 'binary', arg);
+};
+
//
// Compact Protocol internal write methods
@@ -752,22 +760,28 @@ TCompactProtocol.prototype.readDouble = function() {
TCompactProtocol.prototype.readBinary = function() {
var size = this.readVarint32();
- // Catch empty string case
if (size === 0) {
- return "";
+ return new Buffer();
}
- // Catch error cases
if (size < 0) {
- throw new Thrift.TProtocolException(Thrift.TProtocolExceptionType.NEGATIVE_SIZE, "Negative binary/string size");
+ throw new Thrift.TProtocolException(Thrift.TProtocolExceptionType.NEGATIVE_SIZE, "Negative binary size");
}
- var value = this.trans.readString(size);
-
- return value;
+ return this.trans.read(size);
};
TCompactProtocol.prototype.readString = function() {
- return this.readBinary();
+ var size = this.readVarint32();
+ // Catch empty string case
+ if (size === 0) {
+ return "";
+ }
+
+ // Catch error cases
+ if (size < 0) {
+ throw new Thrift.TProtocolException(Thrift.TProtocolExceptionType.NEGATIVE_SIZE, "Negative string size");
+ }
+ return this.trans.readString(size);
};
http://git-wip-us.apache.org/repos/asf/thrift/blob/8a4d06fe/lib/nodejs/lib/thrift/json_protocol.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/lib/thrift/json_protocol.js b/lib/nodejs/lib/thrift/json_protocol.js
index 18fb012..436709c 100644
--- a/lib/nodejs/lib/thrift/json_protocol.js
+++ b/lib/nodejs/lib/thrift/json_protocol.js
@@ -317,11 +317,19 @@ TJSONProtocol.prototype.writeDouble = function(dub) {
};
/** Serializes a string */
-TJSONProtocol.prototype.writeString = function(str) {
+TJSONProtocol.prototype.writeString = function(arg) {
// We do not encode uri components for wire transfer:
- if (str === null) {
+ if (arg === null) {
this.tstack.push(null);
} else {
+ if (typeof arg === 'string') {
+ var str = arg;
+ } else if (arg instanceof Buffer) {
+ var str = arg.toString('utf8');
+ } else {
+ throw new Error('writeString called without a string/Buffer argument: ' + arg);
+ }
+
// concat may be slower than building a byte buffer
var escapedString = '';
for (var i = 0; i < str.length; i++) {
@@ -358,7 +366,15 @@ TJSONProtocol.prototype.writeString = function(str) {
/** Serializes a string */
TJSONProtocol.prototype.writeBinary = function(arg) {
- this.writeString(arg);
+ if (typeof arg === 'string') {
+ var buf = new Buffer(arg, 'binary');
+ } else if (arg instanceof Buffer ||
+ Object.prototype.toString.call(arg) == '[object Uint8Array]') {
+ var buf = arg;
+ } else {
+ throw new Error('writeBinary called without a string/Buffer argument: ' + arg);
+ }
+ this.tstack.push('"' + buf.toString('base64') + '"');
};
/**
@@ -680,7 +696,7 @@ TJSONProtocol.prototype.readDouble = function() {
/** Returns the an object with a value property set to the
next value found in the protocol buffer */
TJSONProtocol.prototype.readBinary = function() {
- return this.readString();
+ return new Buffer(this.readString(), 'base64');
};
/** Returns the an object with a value property set to the
http://git-wip-us.apache.org/repos/asf/thrift/blob/8a4d06fe/lib/nodejs/test/test_driver.js
----------------------------------------------------------------------
diff --git a/lib/nodejs/test/test_driver.js b/lib/nodejs/test/test_driver.js
index 09439e5..590d583 100644
--- a/lib/nodejs/test/test_driver.js
+++ b/lib/nodejs/test/test_driver.js
@@ -57,6 +57,23 @@ exports.ThriftTestDriver = function(client, callback) {
testCases.deep.forEach(makeAsserter(assert.deepEqual));
testCases.deepUnordered.forEach(makeAsserter(makeUnorderedDeepEqual(assert)));
+ var arr = [];
+ for (var i = 0; i < 256; ++i) {
+ arr[i] = 255 - i;
+ }
+ var buf = new Buffer(arr);
+ client.testBinary(buf, function(err, response) {
+ assert.error(err, 'testBinary: no callback error');
+ assert.equal(response.length, 256, 'testBinary');
+ assert.deepEqual(response, buf, 'testBinary(Buffer)');
+ });
+ var buf = new Buffer(arr);
+ client.testBinary(buf.toString('binary'), function(err, response) {
+ assert.error(err, 'testBinary: no callback error');
+ assert.equal(response.length, 256, 'testBinary');
+ assert.deepEqual(response, buf, 'testBinary(string)');
+ });
+
client.testMapMap(42, function(err, response) {
var expected = {
"4": {"1":1, "2":2, "3":3, "4":4},
http://git-wip-us.apache.org/repos/asf/thrift/blob/8a4d06fe/test/known_failures_Linux.json
----------------------------------------------------------------------
diff --git a/test/known_failures_Linux.json b/test/known_failures_Linux.json
index 3625fa9..d326c59 100644
--- a/test/known_failures_Linux.json
+++ b/test/known_failures_Linux.json
@@ -1,6 +1,4 @@
[
- "c_glib-nodejs_binary_buffered-ip",
- "c_glib-nodejs_binary_framed-ip",
"c_glib-py_binary-accel_buffered-ip",
"c_glib-py_binary-accel_framed-ip",
"c_glib-py_binary_buffered-ip",
@@ -9,6 +7,7 @@
"c_glib-rb_binary-accel_framed-ip",
"c_glib-rb_binary_buffered-ip",
"c_glib-rb_binary_framed-ip",
+ "cpp-cpp_binary_http-domain",
"cpp-cpp_binary_http-ip",
"cpp-cpp_compact_http-domain",
"cpp-cpp_header_http-domain",
@@ -23,6 +22,7 @@
"cpp-java_compact_http-ip-ssl",
"cpp-java_json_http-ip",
"cpp-java_json_http-ip-ssl",
+ "cpp-nodejs_json_buffered-ip-ssl",
"cpp-rb_json_buffered-ip",
"csharp-cpp_binary_buffered-ip-ssl",
"csharp-cpp_binary_framed-ip-ssl",
@@ -48,7 +48,6 @@
"csharp-nodejs_compact_framed-ip-ssl",
"csharp-nodejs_json_buffered-ip",
"csharp-nodejs_json_buffered-ip-ssl",
- "csharp-nodejs_json_framed-ip",
"csharp-nodejs_json_framed-ip-ssl",
"erl-cpp_compact_buffered-ip",
"erl-cpp_compact_buffered-ip-ssl",
@@ -97,15 +96,7 @@
"java-hs_json_buffered-ip",
"java-hs_json_fastframed-framed-ip",
"java-hs_json_framed-ip",
- "java-nodejs_json_buffered-ip",
- "java-nodejs_json_buffered-ip-ssl",
- "nodejs-cpp_compact_buffered-ip",
- "nodejs-cpp_compact_buffered-ip-ssl",
- "nodejs-cpp_compact_framed-ip",
- "nodejs-cpp_compact_framed-ip-ssl",
- "nodejs-csharp_compact_buffered-ip",
"nodejs-csharp_compact_buffered-ip-ssl",
- "nodejs-csharp_compact_framed-ip",
"nodejs-csharp_compact_framed-ip-ssl",
"nodejs-csharp_json_buffered-ip",
"nodejs-csharp_json_buffered-ip-ssl",
@@ -113,48 +104,27 @@
"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-java_compact_buffered-ip",
- "nodejs-java_compact_buffered-ip-ssl",
- "nodejs-java_compact_framed-fastframed-ip",
- "nodejs-java_compact_framed-fastframed-ip-ssl",
- "nodejs-java_compact_framed-ip",
- "nodejs-java_compact_framed-ip-ssl",
- "nodejs-java_json_buffered-ip-ssl",
- "nodejs-py3_compact_buffered-ip",
- "nodejs-py3_compact_buffered-ip-ssl",
- "nodejs-py3_compact_framed-ip",
- "nodejs-py3_compact_framed-ip-ssl",
"nodejs-py3_json_buffered-ip",
"nodejs-py3_json_buffered-ip-ssl",
"nodejs-py3_json_framed-ip",
"nodejs-py3_json_framed-ip-ssl",
- "nodejs-py_compact_buffered-ip",
- "nodejs-py_compact_buffered-ip-ssl",
- "nodejs-py_compact_framed-ip",
- "nodejs-py_compact_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_compact_buffered-ip",
- "nodejs-rb_compact_framed-ip",
"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",
- "py-nodejs_json_buffered-ip",
- "py-nodejs_json_buffered-ip-ssl",
- "py-nodejs_json_framed-ip",
- "py-nodejs_json_framed-ip-ssl",
"py3-hs_json_buffered-ip",
"py3-hs_json_framed-ip",
- "py3-perl_binary_buffered-ip-ssl",
- "py3-perl_binary_framed-ip-ssl",
"rb-hs_json_buffered-ip",
- "rb-hs_json_framed-ip",
- "rb-nodejs_json_buffered-ip",
- "rb-nodejs_json_framed-ip"
+ "rb-hs_json_framed-ip"
]
\ No newline at end of file