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

Randy Abernethy commented on THRIFT-2205:
-----------------------------------------

Hi Pierre,

Yeah, I made some notes about this issue in there somewhere. The 
commit/rollback idiom that was in place before did not work. The Borrow/Consume 
idiom is used in C++ and other languages to provide efficient transport buffer 
access to protocols. I started down that road here. The TBufferedTransport 
implementation is good I think. The TFramedTransport implementation is not 
baked. 

The problem with inBuf.length is that it will return the buffer size (like 8K 
or whatever) rather than the size of the data. I think the actual writeIndex 
should be something like FrameSize + readIndex. The elephant in the room here 
is that we need a TJSONProtocol over TFramedTransport test (in addition to your 
one bracket string test in the generic driver). 

With this patch I think the status is:
TBinaryProtocol/TFramedTransport (Test: sever.js - client.js) works
TJSONProtocol/TBufferedTransport (Test: sever_json.js - client_json.js) works
TBinaryProtocol/TBufferedTransport (Test: server_bin.js - client_bin.js) works 
(just uploaded these two in patch 0003)
TJSONProtocol/TBufferedTransport (Test: sever_http.js - test.html) the only 
stack choice for Browser to Node, works

TJSONProtocol/TFramedTransport (Test: none) has the borrow problem you 
identified

-Randy

> Node.js Test Server to support test.js JavaScript Browser test and sundry 
> fixes
> -------------------------------------------------------------------------------
>
>                 Key: THRIFT-2205
>                 URL: https://issues.apache.org/jira/browse/THRIFT-2205
>             Project: Thrift
>          Issue Type: Improvement
>          Components: JavaScript - Library, Node.js - Library
>    Affects Versions: 1.0
>         Environment: All
>            Reporter: Randy Abernethy
>            Assignee: Randy Abernethy
>            Priority: Minor
>              Labels: node, nodejs
>         Attachments: 0001-node-test-update-with-JSON-Buf-fixes.patch, 
> 0002-node-client-server-test-update.patch, 
> 0003-node-throwed-error-should-be-instanciated.patch, 
> 0004-node-fix-TJSONProtocol-parser.patch
>
>
> Adds lib/nodejs/test/testsvr.js 
> This server depends on ThriftTest[.js] and runs clean with test.js in the 
> browser.
> Also in this patch:
> Repairs some shortfall in the Node JSON Protocol and transport. Fixes 
> overflow on Javascript I64 tests. Improves static_server header output.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

Reply via email to