[GitHub] [tinkerpop] spmallette commented on a change in pull request #1289: TINKERPOP-2374 fix missing authorization with SaslAndHttpBasicAuthenticationHandler

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

spmallette commented on a change in pull request #1289:
URL: https://github.com/apache/tinkerpop/pull/1289#discussion_r432432931



##########
File path: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/SaslAndHttpBasicAuthenticationHandler.java
##########
@@ -47,9 +47,11 @@ public SaslAndHttpBasicAuthenticationHandler(final Authenticator authenticator,
     @Override
     public void channelRead(final ChannelHandlerContext ctx, final Object obj) throws Exception {
         if (obj instanceof HttpMessage && !WebSocketHandlerUtil.isWebSocket((HttpMessage)obj)) {
-            if (null == ctx.pipeline().get(HTTP_AUTH)) {
-                ctx.pipeline().addAfter(PIPELINE_AUTHENTICATOR, HTTP_AUTH, new HttpBasicAuthenticationHandler(authenticator, this.authenticationSettings));
+            ChannelPipeline pipeline = ctx.pipeline();
+            if (null != pipeline.get(HTTP_AUTH)) {
+                pipeline.remove(HTTP_AUTH);
             }

Review comment:
       I'm fine with not merging a workaround. i trust your judgement with the Java driver - if you'd prefer a root cause fix, then let's shoot for that.
   
   @javeme do you have any thoughts on the theory that @divijvaidya proposed?




----------------------------------------------------------------
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