From dev-return-52632-archive-asf-public=cust-asf.ponee.io@thrift.apache.org Fri May 4 18:39:04 2018 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by mx-eu-01.ponee.io (Postfix) with SMTP id E7214180634 for ; Fri, 4 May 2018 18:39:03 +0200 (CEST) Received: (qmail 9900 invoked by uid 500); 4 May 2018 16:39:03 -0000 Mailing-List: contact dev-help@thrift.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@thrift.apache.org Delivered-To: mailing list dev@thrift.apache.org Received: (qmail 9889 invoked by uid 99); 4 May 2018 16:39:02 -0000 Received: from pnap-us-west-generic-nat.apache.org (HELO spamd3-us-west.apache.org) (209.188.14.142) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 04 May 2018 16:39:02 +0000 Received: from localhost (localhost [127.0.0.1]) by spamd3-us-west.apache.org (ASF Mail Server at spamd3-us-west.apache.org) with ESMTP id 7D4FF1807EC for ; Fri, 4 May 2018 16:39:02 +0000 (UTC) X-Virus-Scanned: Debian amavisd-new at spamd3-us-west.apache.org X-Spam-Flag: NO X-Spam-Score: -110.311 X-Spam-Level: X-Spam-Status: No, score=-110.311 tagged_above=-999 required=6.31 tests=[ENV_AND_HDR_SPF_MATCH=-0.5, RCVD_IN_DNSWL_MED=-2.3, SPF_PASS=-0.001, T_RP_MATCHES_RCVD=-0.01, USER_IN_DEF_SPF_WL=-7.5, USER_IN_WHITELIST=-100] autolearn=disabled Received: from mx1-lw-us.apache.org ([10.40.0.8]) by localhost (spamd3-us-west.apache.org [10.40.0.10]) (amavisd-new, port 10024) with ESMTP id adn-DjIkbaTt for ; Fri, 4 May 2018 16:39:01 +0000 (UTC) Received: from mailrelay1-us-west.apache.org (mailrelay1-us-west.apache.org [209.188.14.139]) by mx1-lw-us.apache.org (ASF Mail Server at mx1-lw-us.apache.org) with ESMTP id 1275A5F4E4 for ; Fri, 4 May 2018 16:39:01 +0000 (UTC) Received: from jira-lw-us.apache.org (unknown [207.244.88.139]) by mailrelay1-us-west.apache.org (ASF Mail Server at mailrelay1-us-west.apache.org) with ESMTP id 7F201E12CE for ; Fri, 4 May 2018 16:39:00 +0000 (UTC) Received: from jira-lw-us.apache.org (localhost [127.0.0.1]) by jira-lw-us.apache.org (ASF Mail Server at jira-lw-us.apache.org) with ESMTP id 14B6B21297 for ; Fri, 4 May 2018 16:39:00 +0000 (UTC) Date: Fri, 4 May 2018 16:39:00 +0000 (UTC) From: "Brian Forbis (JIRA)" To: dev@thrift.apache.org Message-ID: In-Reply-To: References: Subject: [jira] [Created] (THRIFT-4564) TBufferedTransport can leave corrupt data in the buffer MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-JIRA-FingerPrint: 30527f35849b9dde25b450d4833f0394 Brian Forbis created THRIFT-4564: ------------------------------------ Summary: TBufferedTransport can leave corrupt data in the buff= er Key: THRIFT-4564 URL: https://issues.apache.org/jira/browse/THRIFT-4564 Project: Thrift Issue Type: Bug Components: Node.js - Library Affects Versions: 0.11.0, 0.10.0 Reporter: Brian Forbis I ran into the following issue on Thrift 0.10.0 using Buffered Transport an= d Binary Protocol IDL File: {code:java} struct Foo { 1: string name } service SocketTestService { i16 countFoos(1: list foos) } {code} Client Call: {code:java} async function main() { numFoos =3D await client.countFoos([ new thriftTypes.Foo(), null, new thriftTypes.Foo(), ]); } {code} The issue I ran into was a client-side thrown exception: {code:java} TypeError: Cannot read property 'write' of null at SocketTestService_countFoos_args.write (/Users/bforbis/git/bforbis/thrif= t-socket-test/gen10/SocketTestService.js:81:15) at exports.Client.SocketTestServiceClient.send_countFoos (/Users/bforbis/gi= t/bforbis/thrift-socket-test/gen10/SocketTestService.js:178:8) at exports.Client.SocketTestServiceClient.countFoos (/Users/bforbis/git/bfo= rbis/thrift-socket-test/gen10/SocketTestService.js:165:10) at main (/Users/bforbis/git/bforbis/thrift-socket-test/client.js:110:28) at at process._tickCallback (internal/process/next_tick.js:188:7) {code} The cause of the issue is because I pass in *null*=C2=A0as a parameter to m= y Foo list, which does not have a write() method to serialize itself when w= riting to the buffer. That's okay on its own, but what ends up happening he= re is we have *half* of a message still written to our buffered transport, = meaning that a subsequent RPC call on that same transport will fail to be p= rocessed on the server side. {code:java} Error: Invalid type: -128 at TBinaryProtocol.skip (/Users/bforbis/git/bforbis/thrift-socket-test/node= _modules/thrift/lib/nodejs/lib/thrift/binary_protocol.js:364:13) at module.exports.Foo.Foo.read (/Users/bforbis/git/bforbis/thrift-socket-te= st/gen11/socket-test_types.js:47:15) at SocketTestService_countFoos_args.read (/Users/bforbis/git/bforbis/thrift= -socket-test/gen11/SocketTestService.js:51:17) at exports.Processor.SocketTestServiceProcessor.process_countFoos (/Users/b= forbis/git/bforbis/thrift-socket-test/gen11/SocketTestService.js:224:8) at exports.Processor.SocketTestServiceProcessor.process (/Users/bforbis/git= /bforbis/thrift-socket-test/gen11/SocketTestService.js:210:39) at /Users/bforbis/git/bforbis/thrift-socket-test/node_modules/thrift/lib/no= dejs/lib/thrift/server.js:55:21 at Socket. (/Users/bforbis/git/bforbis/thrift-socket-test/node_m= odules/thrift/lib/nodejs/lib/thrift/buffered_transport.js:48:5) at emitOne (events.js:121:20) at Socket.emit (events.js:211:7) at addChunk (_stream_readable.js:263:12) {code} *Proposal*: We should allow applications to have global persistent socket c= onnections that don't corrupt=C2=A0their buffer if there is a serialization= error when doing an RPC all on the client. Calls to send_${RPC_NAME} shoul= d be wrapped in a try / catch in case there are client side errors in the p= rocessing code. In the case that an exception is thrown, the buffer in buff= ered transport should be emptied rather than leaving it full. *Side Note*:=C2=A0The cause of the client-side exception has been fixed in = Thrift 0.11.0 in this [Pull Request|https://github.com/apache/thrift/commit= /de112fbb0d7f2139ef107211e82e03b574f890d0#diff-a686530169a8a14044fae0f95d54= 578c], which casts undef to a new struct. But I fear that this issue where = hanging data on the buffered transport isn't cleaned up could come up again= . -- This message was sent by Atlassian JIRA (v7.6.3#76005)