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

ASF GitHub Bot commented on THRIFT-4214:
----------------------------------------

GitHub user thexeos opened a pull request:

    https://github.com/apache/thrift/pull/1519

    THRIFT-4214: map<i64,value> key treated as hex value in JavaScript

    All strings passed to [node-int64 
constructor](https://github.com/broofa/node-int64/blob/c1567475712cb1cfe100c96813c2a2a92e2b42ce/Int64.js#L49)
 are treated as hex strings. We need to convert all decimal strings to hex 
representation before passing them to the constructor.
    
    We are making an assumption that no hex string will be passed to 
`writeI64()` without 0x prefix.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/thexeos/thrift master

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/thrift/pull/1519.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1519
    
----
commit 8ef113fd814ce195d06ee9c935ff702d3053c2b5
Author: thexeos <teodor.atroshenko@...>
Date:   2018-03-23T22:06:16Z

    THRIFT-4214: map<i64,value> key treated as hex value in JavaScript

----


> map<i64,value> key treated as hex value in JavaScript
> -----------------------------------------------------
>
>                 Key: THRIFT-4214
>                 URL: https://issues.apache.org/jira/browse/THRIFT-4214
>             Project: Thrift
>          Issue Type: Bug
>          Components: JavaScript - Compiler, JavaScript - Library
>    Affects Versions: 0.10.0
>            Reporter: Teodor Atroshenko
>            Priority: Critical
>              Labels: easyfix
>
> The service is defined with the following function:
> {code:javascript}
> map<i64,double> someFunctionName()
> {code}
> The --gen js:node code:
> {code:javascript}
> output.writeFieldBegin('success', Thrift.Type.MAP, 0);
> output.writeMapBegin(Thrift.Type.I64, Thrift.Type.DOUBLE, 
> Thrift.objectLength(this.success));
> for (var kiter18 in this.success)
> {
>   if (this.success.hasOwnProperty(kiter18))
>   {
>     var viter19 = this.success[kiter18];
>     output.writeI64(kiter18);
>     output.writeDouble(viter19);
>   }
> }
> output.writeMapEnd();
> output.writeFieldEnd();
> {code}
> String {{kiter18}} is passed to {{output.writeI64}}. Only *TBinaryProtocol* 
> is affected by this. The function 
> [defenition|https://github.com/apache/thrift/blob/19baeefd8c38d62085891d7956349601f79448b3/lib/nodejs/lib/thrift/binary_protocol.js#L137]:
> {code:javascript}
> TBinaryProtocol.prototype.writeI64 = function(i64) {
>   if (i64.buffer) {
>     this.trans.write(i64.buffer);
>   } else {
>     this.trans.write(new Int64(i64).buffer);
>   }
> };
> {code}
> *Int64* 
> [constructor|https://github.com/broofa/node-int64/blob/master/Int64.js#L49] 
> accepts the following arguments:
>  * new Int64(buffer\[, offset=0]) - Existing Buffer with byte offset
>  * new Int64(Uint8Array\[, offset=0]) - Existing Uint8Array with a byte offset
>  * new Int64(string)             - Hex string (throws if n is outside int64 
> range)
>  * new Int64(number)             - Number (throws if n is outside int64 range)
>  * new Int64(hi, lo)             - Raw bits as two 32-bit values
> What happens, is that JavaScript object keys are always Strings, so the 
> generated function passes the object key as-is (in String representation) and 
> *Int64* library assumes it's a hex string and converts it to a different 
> 64-bit integer.
> For example {{"4398046511580"}} becomes {{\[Int64 value:1189123005158784 
> octets:00 04 39 80 46 51 15 80]}}
> This also happens when returning {{list<i64>}}, however in lists it can be 
> bypassed by calling {{Number()}} on all Array elements before passing it to 
> Thrift's {{result()}} function.
> To solve this, the compiler can be adding {{Number()}} around everything that 
> is a decimal number that is passed to {{writeI64}}; and if {{writeI64}} will 
> never be called with an actual hex string, then js library can be updated to 
> include the {{Number()}} call around *Int64* constructor argument.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to