Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/6348 )
Change subject: KUDU-1918 Prevent hijacking of scanner IDs ...................................................................... Patch Set 8: Code-Review+2 (2 comments) http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/client/client-test.cc File src/kudu/client/client-test.cc: http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/client/client-test.cc@5765 PS7, Line 5765: ASSERT_OK(bad_guy_scanner.Open()); > Hrm, I'm not sure what exactly it would be testing, but I can tell you what Hrm, SGTM: I just wanted to make sure the client code behaves as expected, i.e. at some point it would return Status::RemoteError(). After some consideration, I think that's not crucial to test since the whole point of this test is to make sure the server-side behavior (not the client-side) is correct. http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/tserver/scanners.cc File src/kudu/tserver/scanners.cc: http://gerrit.cloudera.org:8080/#/c/6348/7/src/kudu/tserver/scanners.cc@168 PS7, Line 168: if (username != ret->remote_user().username()) { : *error_code = TabletServerErrorPB::NOT_AUTHORIZED; : return Status::NotAuthorized(Substitute("User $0 doesn't own scanner $1", : username, scanner_id)); : } > Interesting question; I don't think so because scanner ids AFAICT aren't tr SGTM -- To view, visit http://gerrit.cloudera.org:8080/6348 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic91fa0ca471bd674e35aa2f8de3806b88ad4b3b4 Gerrit-Change-Number: 6348 Gerrit-PatchSet: 8 Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Andrew Wong <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: David Ribeiro Alves <[email protected]> Gerrit-Reviewer: Hao Hao <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Tidy Bot (241) Gerrit-Comment-Date: Tue, 06 Nov 2018 19:15:33 +0000 Gerrit-HasComments: Yes
