Jason Fehr has posted comments on this change. ( http://gerrit.cloudera.org:8080/22986 )
Change subject: IMPALA-14083: Connected user and session user mismatch when cookie based authentication is used with SPNEGO ...................................................................... Patch Set 3: (3 comments) http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java File fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java: http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java@249 PS3, Line 249: System.out.println("cookie " + cookie); Nit: debugging line that should be removed. http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/SpnegoAuthTest.java@401 PS3, Line 401: String[] cookieValueFields = cookieFields[0].trim().split("&"); Since Impala expects the cookie value to be in a specific format, it would make the test assertions more robust to check a specific index: // pseudocode assertEquals(4, cookieValueFields.length) String[] authMech = cookieValueFields[3].trim().split("=") assertEquals(2, authMech.length) assertEquals("a", authMech[0]) return authMech[1] http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/THttpClientWithHeaders.java File fe/src/test/java/org/apache/impala/customcluster/THttpClientWithHeaders.java: http://gerrit.cloudera.org:8080/#/c/22986/3/fe/src/test/java/org/apache/impala/customcluster/THttpClientWithHeaders.java@77 PS3, Line 77: THttpClientWithHeaders > This class seems only used in custom cluster tests, maybe rename it to Test I'm not in favor of adding "Test" to the class name since having that word in the class name indicates the class contains JUnit tests. To me, having this class in the "src/test/java" folder is enough to indicate this class supports tests. This class won't be available to main code since it's in the "src/test/java" folder. -- To view, visit http://gerrit.cloudera.org:8080/22986 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id7223e449c32484bfd2295f7a9e728b7c02637e9 Gerrit-Change-Number: 22986 Gerrit-PatchSet: 3 Gerrit-Owner: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Jason Fehr <[email protected]> Gerrit-Reviewer: Yida Wu <[email protected]> Gerrit-Comment-Date: Tue, 10 Jun 2025 16:09:50 +0000 Gerrit-HasComments: Yes
