Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/9601 )
Change subject: KUDU-2305: Limit sidecars to INT_MAX and fortify socket code ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/9601/3/src/kudu/rpc/rpc-test.cc File src/kudu/rpc/rpc-test.cc: http://gerrit.cloudera.org:8080/#/c/9601/3/src/kudu/rpc/rpc-test.cc@724 PS3, Line 724: s.resize(TransferLimits::kMaxTotalSidecarBytes, 'a'); > I'm afraid about fixing hypotehtical issues with a non-hypothetical test. H The receiver protects itself by closing the connection when it sees a message larger than rpc_max_message_size. The question is whether the sender can ever keeping sending and overflow its own variables, particularly in TlsSocket::Writev(). That is why this is hypothetical. I don't think it is truly possible or at least it is extremely hard to hit, but changing some variables to a larger size can't really hurt. I played around with writing a test for it. The problem is that testing the sender code requires that the receiver keep the connection open, so the receiver needs to support messages larger than INT_MAX. It doesn't today. If it ignores the rpc_max_message_size, then runs into the fact that Socket::Recv() doesn't support more than INT_MAX at a time. When you get past that, serialization::ParseMessage() also doesn't support more than INT_MAX, because CodedInputStream takes an int for the buffer size. Long story short, this is a tough test to write. If the client knew the header and protobuf sizes in the right places, it could make the client and server match up. It doesn't at the moment. -- To view, visit http://gerrit.cloudera.org:8080/9601 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I2d041e214b15d9c22b810588643798e2b3bc5c24 Gerrit-Change-Number: 9601 Gerrit-PatchSet: 3 Gerrit-Owner: Joe McDonnell <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Michael Ho <[email protected]> Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-Comment-Date: Sat, 17 Mar 2018 00:36:12 +0000 Gerrit-HasComments: Yes
