[ 
https://issues.apache.org/jira/browse/THRIFT-2933?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14276041#comment-14276041
 ] 

Tom Croucher commented on THRIFT-2933:
--------------------------------------

I disagree with this approach a bit. I think that having it in binary.js is a 
good thing since JavaScript doesn't have good native support for these 
encodings we should put in one place. This would make it easy to replace with 
specific optimized implementations for Node or the browser using either 
Buffers, TypedArrays or whatever other method in the future.

I would say that if we put them into protocol.js we should at the least we 
should put the double encoding/decoding functions into helpers not directly 
into the interface methods.

> 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: double-endiannes-2.patch, 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)

Reply via email to