[
https://issues.apache.org/jira/browse/THRIFT-2933?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14274600#comment-14274600
]
Randy Abernethy commented on THRIFT-2933:
-----------------------------------------
Hey Dan,
Sorry for the confusion, my fault actually. Because the Apache Thrift Node JS
implementation has evolved from a handy tidbit to a fairly full implementation
in 0.9.2, much of the internals and file layout are still distinct from the
rest of Apache Thrift. When I add bits I try to massage things in the direction
of the larger Apache Thrift library to improve over all consistency.
The TProtocol interface in the reference implementations (CPP/Java) defines a
set of methods which are then implemented by every protocol. Though JS doesn't
really have formal interfaces, it would be preferable to stick with the
interface driven function names (writeDouble(), readDouble(), writeI64(), etc.)
in each protocol implementation. The protocol.js and binary.js files are
artifacts from a time when there was only one protocol in this lib. I was
hoping you could provide your little endian Double implementation in the
protocol.js, "TCompactProtocol.prototype.writeDouble = function(dub)" body on
line 682. This will make it easier to keep the TBinaryProtocol code in
binary.js segregated until someone has time to sort things into proper modules
(TBinaryProtocol.js, TCompactProtocol.js, etc.).
As you have noted each protocol is responsible for defining its own wire layout
which all languages must adhere to. As you have surmized, TBinaryProtocol uses
TCP/IP network byte order (big endian) on the wire and TCompactProtocol uses
little endian on the wire for doubles. So Node Doubles are broken presently,
making this a patch we definitely need to get into the trunk.
Best,
Randy
> v0.9.2: doubles encoded in node with compact protocol cannot be decoded by
> python
> ---------------------------------------------------------------------------------
>
> Key: THRIFT-2933
> URL: https://issues.apache.org/jira/browse/THRIFT-2933
> Project: Thrift
> Issue Type: Bug
> Components: Node.js - Library
> Affects Versions: 0.9.2
> Reporter: Dan Heller
> Attachments: little-endian.patch, thrift-double.tgz
>
>
> I've noticed that with Thrift v0.9.2, using the Compact protocol, I can't
> encode doubles in node and decode them in python. It looks like in python,
> we're decoding little-endian for compact and big-endian for binary, and in
> node, writing big-endian in all cases (therefore, Binary works fine). I've
> attached a test program to show this behavior. I have not found the thrift
> binary spec, so I'm not sure which side is wrong.
> {code}
> [~/thrift-double]$ thrift --version
> Thrift version 0.9.2
> [~/thrift-double]$ grep -i thrift package.json
> "thrift": "0.9.2"
> "compile": "thrift --gen js:node --gen py test.thrift"
> [~/thrift-double]$ pip show thrift
> ---
> Name: thrift
> Version: 0.9.2
> Location: /Library/Python/2.7/site-packages
> Requires:
> [~/thrift-double]$ npm run compile
> > double-decode-bug@0.0.1 compile /Users/dh/thrift-double
> > thrift --gen js:node --gen py test.thrift
> [~/thrift-double]$ node test.js binary | python test.py binary
> Trying to send { intField: 100, doubleField: 99.88 }
> Reading with binary protocol.
> Got test object: TestType(intField=100, doubleField=99.88)
> [~/thrift-double]$ node test.js compact | python test.py compact
> Trying to send { intField: 100, doubleField: 99.88 }
> Reading with compact protocol.
> Got test object: TestType(intField=100, doubleField=-2.24248483894553e-38)
> [~/thrift-double]$
> {code}
> I notice the following snippets.
> * TBinaryProtocol.py: network format (big-endian)
> {code}
> def readDouble(self):
> buff = self.trans.readAll(8)
> val, = unpack('!d', buff)
> return val
> {code}
> * TCompactProtocol.py: little-endian
> {code}
> def readDouble(self):
> buff = self.trans.readAll(8)
> val, = unpack('<d', buff)
> return val
> {code}
> * binary.js: my best IEEE floating point days are behind me but this looks
> big-endian. In any case it's used for both compact and binary.
> {code}
> exports.writeDouble = function(buff, v) {
> var m, e, c;
> buff[0] = (v < 0 ? 0x80 : 0x00);
> v = Math.abs(v);
> if (v !== v) {
> // NaN, use QNaN IEEE format
> m = 2251799813685248;
> e = 2047;
> } else if (v === Infinity) {
> m = 0;
> e = 2047;
> } else {
> e = Math.floor(Math.log(v) / Math.LN2);
> c = Math.pow(2, -e);
> if (v * c < 1) {
> e--;
> c *= 2;
> }
> if (e + 1023 >= 2047)
> {
> // Overflow
> m = 0;
> e = 2047;
> }
> else if (e + 1023 >= 1)
> {
> // Normalized - term order matters, as Math.pow(2, 52-e) and
> v*Math.pow(2, 52) can overflow
> m = (v*c-1) * POW_52;
> e += 1023;
> }
> else
> {
> // Denormalized - also catches the '0' case, somewhat by chance
> m = (v * POW_1022) * POW_52;
> e = 0;
> }
> }
> buff[1] = (e << 4) & 0xf0;
> buff[0] |= (e >> 4) & 0x7f;
> buff[7] = m & 0xff;
> m = Math.floor(m / POW_8);
> buff[6] = m & 0xff;
> m = Math.floor(m / POW_8);
> buff[5] = m & 0xff;
> m = Math.floor(m / POW_8);
> buff[4] = m & 0xff;
> m >>= 8;
> buff[3] = m & 0xff;
> m >>= 8;
> buff[2] = m & 0xff;
> m >>= 8;
> buff[1] |= m & 0x0f;
> return buff;
> };
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
