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 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/6348/3/src/kudu/tserver/tablet_server-test-base.h File src/kudu/tserver/tablet_server-test-base.h: http://gerrit.cloudera.org:8080/#/c/6348/3/src/kudu/tserver/tablet_server-test-base.h@125 PS3, Line 125: Status FillNewScanRequest(ReadMode read_mode, NewScanRequestPB* scan); Could it be a static method (or at least const)? http://gerrit.cloudera.org:8080/#/c/6348/3/src/kudu/tserver/tablet_server-test.cc File src/kudu/tserver/tablet_server-test.cc: http://gerrit.cloudera.org:8080/#/c/6348/3/src/kudu/tserver/tablet_server-test.cc@3400 PS3, Line 3400: TEST_F(TabletServerTest, TestScannerCheckMatchingUser) { Is it worth adding a higher-level test to make sure this works as expected? It seems to be too early at this phase, but after the authz token stuff is there, it might make sense. http://gerrit.cloudera.org:8080/#/c/6348/3/src/kudu/tserver/tablet_server-test.cc@3407 PS3, Line 3407: different user Does it make sense to add a scenario to verify that without user_credentials set for the scanner it's not possible to re-use the original scanner? -- 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: 3 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: Fri, 26 Oct 2018 17:28:23 +0000 Gerrit-HasComments: Yes
