joshelser commented on code in PR #123:
URL:
https://github.com/apache/phoenix-queryserver/pull/123#discussion_r1138900115
##########
phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java:
##########
@@ -633,6 +625,26 @@ SimpleLRUCache<String,UserGroupInformation> getCache() {
}
}
+ /**
+ * The new Jetty kerberos implementation that we use strips the realm from
the principal, which
+ * and Hadoop cannot process that.
+ * This strips the hostname part as well, so that only the username remains.
+
+ * @param remoteUserName the principal as received from Jetty
+ * @return the principal without the hostname part
+ */
+ //FIXME We are probably compensating for a Jetty a bug, which should really
be fixed in Jetty
Review Comment:
Is there still a latent Jetty bug? I thought updating to the "new" SPNEGO
API in Jetty would have addressed any lingering issues around parsing.
##########
phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java:
##########
@@ -633,6 +625,26 @@ SimpleLRUCache<String,UserGroupInformation> getCache() {
}
}
+ /**
+ * The new Jetty kerberos implementation that we use strips the realm from
the principal, which
+ * and Hadoop cannot process that.
+ * This strips the hostname part as well, so that only the username remains.
+
+ * @param remoteUserName the principal as received from Jetty
+ * @return the principal without the hostname part
+ */
+ //FIXME We are probably compensating for a Jetty a bug, which should really
be fixed in Jetty
+ private static String stripHostNameFromPrincipal(String remoteUserName) {
+ // realm got removed from remoteUserName in CALCITE-4152
+ // so we remove the instance name to avoid geting
KerberosName$NoMatchingRule exception
+ int atSignIndex = remoteUserName.indexOf('@');
+ int separatorIndex = remoteUserName.indexOf('/');
+ if (atSignIndex == -1 && separatorIndex > 0) {
+ remoteUserName = remoteUserName.substring(0, separatorIndex);
+ }
Review Comment:
Getting a principal `phoenix/pqs.apache.org` is plausible, but so would
`phoenix/[email protected]` is too. I think your code would only strip
the principal instance if the realm were not provided. I think you could remove
the `atSignIndex` logic from this method.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
