Todd Lipcon 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: (2 comments) 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'); can we add a test like this that actually sends and receives a valid sidecar of the maximum, after configuring rpc_max_message_size to allow it? I think that woudl be a good check for overflow conditions, etc, given our use of UBSAN. http://gerrit.cloudera.org:8080/#/c/9601/3/src/kudu/rpc/rpc_sidecar.cc File src/kudu/rpc/rpc_sidecar.cc: http://gerrit.cloudera.org:8080/#/c/9601/3/src/kudu/rpc/rpc_sidecar.cc@78 PS3, Line 78: if (buffer.size() > TransferLimits::kMaxTotalSidecarBytes) { is kMaxTotalSidecarBytes meant to limit the total size of sidecars? Or the total size of the message (sidecars PLUS pb PLUS header?) Isn't 'buffer' here the entire message including everything, not just the sidecar portion? -- 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: Fri, 16 Mar 2018 20:57:03 +0000 Gerrit-HasComments: Yes
