[GitHub] [hbase] joshelser commented on a change in pull request #1260: HBASE-23881 Ensure Netty client receives at least one response before…

[GitBox <gi...@apache.org>]

joshelser commented on a change in pull request #1260: HBASE-23881 Ensure Netty client receives at least one response before…
URL: https://github.com/apache/hbase/pull/1260#discussion_r390518113
 
 

 ##########
 File path: hbase-client/src/main/java/org/apache/hadoop/hbase/security/NettyHBaseSaslRpcClientHandler.java
 ##########
 @@ -114,7 +122,10 @@ public void handlerAdded(ChannelHandlerContext ctx) {
       if (initialResponse != null) {
         writeResponse(ctx, initialResponse);
       }
-      tryComplete(ctx);
+      // HBASE-23881 We do not want to check if the SaslClient thinks the handshake is
 
 Review comment:
   > It does not say that we 'must' call it after we receive the reponse from server.
   
   I was initially thinking the same thing: "why the heck does this implementation say 'i'm done'??". I can read it both ways -- reading the JDK-provided implementations, the "you should wait for a response from the server" was the only interpretation which made any sense to me.
   
   > what is the sasl client implementation
   
   I looked at what was provided in openjdk8 because I had the same question about which mechanisms actually acted this way :). Surveying all mechanisms included, we have two categories. One category is that the mechanism's impl for `isComplete()` returns true after a single call to `evaluateChallenge`, and the other category is that it takes multiple calls to evaluateChallenge to eventually have `isComplete` return true. You can also think of these as "challenge is exactly one step" and "challenge is multiple steps", respectively
   
   * Plain - one step
   * DigestMD5 - multiple steps
   * NTLM - multiple steps
   * GSSAPI/KRB5 -- multiple steps
   * CramMD5 - one step
   * External - one step
   
   This test is using Plain which is in the "one step" category, while all over sasl-based mechanisms previously in HBase are in the "multiple steps" bucket :).
   
   I completely understand your initial reaction that this mechanism is "broken", but, after doing the digging I have, I believe our interpretation of Sasl was just unaware of how some other implementations work in this regard.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services