[ https://issues.apache.org/jira/browse/ZOOKEEPER-2186?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15083614#comment-15083614 ]
Powell Molleti commented on ZOOKEEPER-2186: ------------------------------------------- I agree with Markus here, I noticed this issue but assumed that there is no expectation perhaps to add to the HDR anymore with respect to this protocol version -65536L . Otherwise its best to do the following as Markus requested. {code:java} dout.writeLong(PROTOCOL_VERSION); String addr = self.getElectionAddress().getHostString() + ":" + self.getElectionAddress().getPort(); byte[] addr_bytes = addr.getBytes(); // After version write the total length of msg sent by sender. dout.writeInt(Long.BYTES + addr_bytes.length); // Write sid afterwards dout.writeLong(self.getId()); // Write length of host/port string dout.writeInt(addr_bytes.length); // Write host/port string dout.write(addr_bytes); {code} This helps older revisions to ignore msg past host/port since it will read only what it understands and discard rest of the bytes. I have another proposal: Once the RX side when a compatible peer sees new protocol version i.e -65536L it could reply with -65536L too (this wont happen with older peer since it will send notification with state, which cannot be -65536L) and both of them could negotiate in protobufs, and this once and for all solving this issue. Also I prefer both channels be symmetric with respect to header negotiation just like the data exchange which makes implementation of channels much easier. I am not quite sure if this can be done now that 3.5.1 is out there and Protocol version bump happened sometime ago in ZOOKEEPER-1633. > QuorumCnxManager#receiveConnection may crash with random input > -------------------------------------------------------------- > > Key: ZOOKEEPER-2186 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2186 > Project: ZooKeeper > Issue Type: Bug > Components: server > Affects Versions: 3.4.6, 3.5.0 > Reporter: Raul Gutierrez Segales > Assignee: Raul Gutierrez Segales > Fix For: 3.4.7, 3.5.1, 3.6.0 > > Attachments: ZOOKEEPER-2186-v3.4.patch, ZOOKEEPER-2186.patch, > ZOOKEEPER-2186.patch, ZOOKEEPER-2186.patch > > > This will allocate an arbitrarily large byte buffer (and try to read it!): > {code} > public boolean receiveConnection(Socket sock) { > Long sid = null; > ... > sid = din.readLong(); > // next comes the #bytes in the remainder of the message > > int num_remaining_bytes = din.readInt(); > byte[] b = new byte[num_remaining_bytes]; > // remove the remainder of the message from din > > int num_read = din.read(b); > {code} > This will crash the QuorumCnxManager thread, so the cluster will keep going > but future elections might fail to converge (ditto for leaving/joining > members). > Patch coming up in a bit. -- This message was sent by Atlassian JIRA (v6.3.4#6332)