Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/16584 )
Change subject: IMPALA-9232 Potential overflow in serializeThriftMsg ...................................................................... Patch Set 1: (1 comment) Thanks for catching this, these overflows can be a headache to find otherwise. I have a nit about how to to the string substitution, but after that I'll +2 http://gerrit.cloudera.org:8080/#/c/16584/1/be/src/rpc/jni-thrift-util.h File be/src/rpc/jni-thrift-util.h: http://gerrit.cloudera.org:8080/#/c/16584/1/be/src/rpc/jni-thrift-util.h@40 PS1, Line 40: if (size > INT_MAX) { This is a good catch. One nit: we try to use strings::Substitute() from gutil/strings/substitute.h instead of stringstream where possible, i.e. when substituting values into a fixed string template. -- To view, visit http://gerrit.cloudera.org:8080/16584 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie76028acea84dbe0e88518dae60aaf7e7ca55e9e Gerrit-Change-Number: 16584 Gerrit-PatchSet: 1 Gerrit-Owner: Qifan Chen <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Mon, 12 Oct 2020 16:32:51 +0000 Gerrit-HasComments: Yes
