[
https://issues.apache.org/jira/browse/ZOOKEEPER-1437?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13406135#comment-13406135
]
Eugene Koontz commented on ZOOKEEPER-1437:
------------------------------------------
Hi Mahadev,
Thanks for looking at the patch and your comments. Responding to your comments:
bq. You added
bq. public boolean readOnly;
bq. In ClientCnxn.java Packet class which doesnt seem to be used anywhere, am I
correct?
This was added so that packet-serialization can be deferred until the
packet is actually sent: we need to save the readOnly parameter,
passed as an argument to the packet constructor, until it is actually
used in the serialization of the packet. In this patch this
serialization is now done in a newly-added method,
ClientCnxnSocketNIO::sendPacket(), which serializes the input packet
'p' by calling p.createBB(). createBB() uses the packet's 'readOnly'
member variable in order to create the serialization of the packet.
bq. I think we are a little weak on our synchronizations in the
bq. patch. I will take a look again but looks like its using a lot of
bq. member variables which could get changed by various threads.
bq. I think there is a race condition in
bq. ClientCnxnSocketNIO:findSendablePacket() wherein how do you make
bq. sure that the sasl packets (without header) are present in the
bq. queue before we start running the thread.
I don't think there is a race condition possible. My reasoning is that
the sendThread both sends packets from the outgoingQueue and manages
the {{zooKeeperSaslClient}} object. Therefore it is not possible to send
packets from the outgoing queue (besides the initial priming packet) without
the {{zooKeeperSaslClient}} being
finished with authentication (whether successfully or not).
In the code from the patch that you
cite, {{clientTunneledAuthenticationInProgress()}} must have returned
false in order to get inside the {{ else{} }}. For
{{clientTunneledAuthenticationInProgress()}} is false, then at least one
of the following must be true:
1. saslAuthFailed is true, or
2. zooKeeperSaslClient's clientTunneledAuthenticationInProgress() is false.
With regard to 1., saslAuthFailed can only be true if a {{LoginException}} was
caught within
{{startConnect()}}, but {{startConnect()}} can only be run by the sendThread.
With regard to 2., this can only be false if {{gotLastPacket}} is true,
and either:
2.1. saslState is COMPLETE, or
2.2. saslState is FAILED.
But {{gotLastPacket}} is initialized to false when the sendThread creates it,
and can only be set to true by code run by the sendThread
(specifically by {{ZooKeeperSaslClient::respondToServer()}}).
> Client uses session before SASL authentication complete
> -------------------------------------------------------
>
> Key: ZOOKEEPER-1437
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1437
> Project: ZooKeeper
> Issue Type: Bug
> Components: java client
> Affects Versions: 3.4.3
> Reporter: Thomas Weise
> Assignee: Eugene Koontz
> Fix For: 3.4.4, 3.5.0
>
> Attachments: ZOOKEEPER-1437.patch, ZOOKEEPER-1437.patch,
> ZOOKEEPER-1437.patch, ZOOKEEPER-1437.patch, ZOOKEEPER-1437.patch,
> ZOOKEEPER-1437.patch, ZOOKEEPER-1437.patch, ZOOKEEPER-1437.patch,
> ZOOKEEPER-1437.patch, ZOOKEEPER-1437.patch, ZOOKEEPER-1437.patch,
> ZOOKEEPER-1437.patch, ZOOKEEPER-1437.patch, ZOOKEEPER-1437.patch,
> ZOOKEEPER-1437.patch, getXidCallHierarchy.png
>
>
> Found issue in the context of hbase region server startup, but can be
> reproduced w/ zkCli alone.
> getData may occur prior to SaslAuthenticated and fail with NoAuth. This is
> not expected behavior when the client is configured to use SASL.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
