[
https://issues.apache.org/jira/browse/TINKERPOP-2374?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17120036#comment-17120036
]
ASF GitHub Bot commented on TINKERPOP-2374:
-------------------------------------------
divijvaidya commented on pull request #1289:
URL: https://github.com/apache/tinkerpop/pull/1289#issuecomment-636244153
Really appreciate the detailed explanation @javeme. We are on the same page
here. While writing my explanation, I missed that step#7 is not adding the
http-authenticator back. Your change will definitely fix this issue.
Before we push this change, can you please add a couple of more associated
changes that will ensure we catch such bugs proactively in future.
1. Add logic to validate that the pipeline has been setup as expected. You
might want to leverage the [finalize method in
AbstractChannelizer](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/AbstractChannelizer.java#L124).
Note that this wouldn't catch pipeline setup issues where it is modified
dynamically based on message information while processing the message. For
that, we will do #2
2. Add tests to validate the pipeline through which a message is executed.
If we had this test, this bug would have been caught earlier.
After you have made the above changes, we can push this change out while
separately working on the larger issue as described below.
The reason why I called this change as a workaround is because, while it
solves this particular problem of missing http-authenticator, it doesn't
address the underlying issue which is dynamic modification of pipeline when
more than one request is expected.
Each request is expected to be processed by a deterministic set of handlers.
In some cases such as WS and HTTP being served on same connection, this handler
chain is created dynamically based on the request headers or content. However,
when we execute two requests on the same pipeline (case of keepAlive), both the
requests would try to modify the same pipeline at the same time which might
lead to a case when the pipeline is misconfigured. e.g. while one message [has
removed the PIPELINE_AUTHENTICATOR
handler](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsAndHttpChannelizerHandler.java#L68)
(and has not added it back yet in the next line), in a separate thread,
another message would reach a [check for PIPELINE_AUTHENTICATOR presence in the
same
pipeline](https://github.com/apache/tinkerpop/blob/cc3c5cb83e253b9949076628a7cfaade7f86f40e/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsAndHttpChannelizerHandler.java#L65)
and finds that it isn't present, hence, not taking the actions to configure
the pipeline correctly.
Do you agree that this is still a problem even after the fix you proposed
and can lead to a similar misconfigured pipeline?
----------------------------------------------------------------
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:
[email protected]
> SaslAndHttpBasicAuthenticationHandler can't extract authorization
> -----------------------------------------------------------------
>
> Key: TINKERPOP-2374
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2374
> Project: TinkerPop
> Issue Type: Bug
> Reporter: Jermy Li
> Priority: Major
>
> When we use the following configuration and keep http connection alive, some
> requests will fail to get authorization information during consecutive
> requests.
> {code:yaml}
> channelizer: org.apache.tinkerpop.gremlin.server.channel.WsAndHttpChannelizer
> authentication: {
> authenticationHandler:
> org.apache.tinkerpop.gremlin.server.handler.SaslAndHttpBasicAuthenticationHandler,
> }
> {code}
>
> We expect the sequence in the pipeline to be:
> {code:java}
> (http-response-encoder = io.netty.handler.codec.http.HttpResponseEncoder),
> (authenticator =
> org.apache.tinkerpop.gremlin.server.handler.SaslAndHttpBasicAuthenticationHandler),
>
> (http-authentication =
> org.apache.tinkerpop.gremlin.server.handler.HttpBasicAuthenticationHandler),
> (request-handler =
> org.apache.tinkerpop.gremlin.server.handler.HttpGremlinEndpointHandler),
> {code}
> authenticator -> {color:#ff0000}http-authentication{color} -> request-handler
> But sometimes its order becomes the following, so that user information
> cannot be obtained:
> {code:java}
> (http-response-encoder = io.netty.handler.codec.http.HttpResponseEncoder),
> (authenticator =
> org.apache.tinkerpop.gremlin.server.handler.SaslAndHttpBasicAuthenticationHandler),
>
> (request-handler =
> org.apache.tinkerpop.gremlin.server.handler.HttpGremlinEndpointHandler),
> (http-authentication =
> org.apache.tinkerpop.gremlin.server.handler.HttpBasicAuthenticationHandler),
> {code}
> authenticator -> request-handler -> {color:#ff0000}http-authentication{color}
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
