Edward Ribeiro commented on ZOOKEEPER-2439:

Hi [~hanm]!

Should we check ACL in FinalRequestProcessor?

I hope so! :) I considered this solution too, but sidetracked because I am 
unsure about transactional guarantees, particularly the _*sync request to 
disk*_ performed by {{SyncRequestProcessor}}.

Note: by the way, my stab at a solution (just a sketch, really) most probably 
breaks on parallel unit test running due to the use of static mutable fields.

note we already check ACL in FinalRequestProcessor (example) for some ops, but 
not all, not sure why.

The *non state changing* operations (exists, getData, getChildren, etc) follow 
a slightly different, more straightforward, path down the processing pipeline 
while transactional, *state changing*, operations (setData, delete, setACL, 
etc) perform a series of extra operations, as expected. Therefore, it looks 
like {{FinalRequestProcessor}} is currently checking the ACL permissions for 
non transactional check/read operations (even tough _exists()_ operation is 
lacking ACL check, a bug, imho) while the transactional operations are handled 
by {{PrepRequestProcessor.pRequest2Txn}} nowadays.

> The order of asynchronous setACL is not correct.
> ------------------------------------------------
>                 Key: ZOOKEEPER-2439
>                 URL: https://issues.apache.org/jira/browse/ZOOKEEPER-2439
>             Project: ZooKeeper
>          Issue Type: Bug
>    Affects Versions: 3.4.8, 3.5.1
>         Environment: Linux Ubuntu
> Mac OS X
>            Reporter: Kazuaki Banzai
>              Labels: acl
>         Attachments: ZOOKEEPER-2439-WIP.patch, ZOOKEEPER-2439.patch
> Within a given client connection, the execution of commands on the ZooKeeper 
> server is always ordered, as both synchronous and asynchronous commands are 
> dispatched through queuePacket (directly or indirectly).
> In other words, Zookeeper guarantees sequential consistency: updates from a 
> client will be applied in the order that they were sent.
> However, the order of asynchronous setACL is not correct on Ubuntu.
> When asynchronous setACL is called BEFORE another API is called, asynchronous 
> setACL is applied AFTER another API.
> For example, if a client calls
> (1) asynchronous setACL to remove all permissions of node "/" and
> (2) synchronous create to create node "/a",
> synchronous create should fail, but it succeeds on Ubuntu.
> (We can see all permissions of node "/" are removed when the client calls 
> getACL to node "/" after (2), so (1) is applied AFTER (2). If we call getACL 
> between (1) and (2), the synchronous case works correctly but the 
> asynchronous case still produces the bug.)
> The attached unit test reproduces this scenario. It fails on Linux Ubuntu but 
> succeeds on Mac OS X. If used on a heavily loaded server on Mac OS, the test 
> sometimes fails as well but only rarely.

This message was sent by Atlassian JIRA

Reply via email to