Author: mahadev
Date: Mon Aug 15 00:35:18 2011
New Revision: 1157685
URL: http://svn.apache.org/viewvc?rev=1157685&view=rev
Log:
ZOOKEEPER-1055. check for duplicate ACLs in addACL() and create(). (Eugene
Koontz via mahadev)
Added:
zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ACLCountTest.java
Modified:
zookeeper/trunk/CHANGES.txt
zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxn.java
Modified: zookeeper/trunk/CHANGES.txt
URL:
http://svn.apache.org/viewvc/zookeeper/trunk/CHANGES.txt?rev=1157685&r1=1157684&r2=1157685&view=diff
==============================================================================
--- zookeeper/trunk/CHANGES.txt (original)
+++ zookeeper/trunk/CHANGES.txt Mon Aug 15 00:35:18 2011
@@ -285,6 +285,9 @@ BUGFIXES:
ZOOKEEPER-1150. fix for this patch to compile on windows. (dheeraj
via mahadev)
+ ZOOKEEPER-1055. check for duplicate ACLs in addACL() and create().
+ (Eugene Koontz via mahadev)
+
IMPROVEMENTS:
ZOOKEEPER-724. Improve junit test integration - log harness information
(phunt via mahadev)
Modified:
zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
URL:
http://svn.apache.org/viewvc/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java?rev=1157685&r1=1157684&r2=1157685&view=diff
==============================================================================
---
zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
(original)
+++
zookeeper/trunk/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
Mon Aug 15 00:35:18 2011
@@ -305,7 +305,8 @@ public class PrepRequestProcessor extend
Long.toHexString(request.sessionId));
throw new KeeperException.BadArgumentsException(path);
}
- if (!fixupACL(request.authInfo, createRequest.getAcl())) {
+ List<ACL> listACL = removeDuplicates(createRequest.getAcl());
+ if (!fixupACL(request.authInfo, listACL)) {
throw new KeeperException.InvalidACLException(path);
}
String parentPath = path.substring(0, lastSlash);
@@ -339,7 +340,7 @@ public class PrepRequestProcessor extend
}
int newCversion = parentRecord.stat.getCversion()+1;
request.txn = new CreateTxn(path, createRequest.getData(),
- createRequest.getAcl(),
+ listACL,
createMode.isEphemeral(), newCversion);
StatPersisted s = new StatPersisted();
if (createMode.isEphemeral()) {
@@ -350,8 +351,7 @@ public class PrepRequestProcessor extend
parentRecord.stat.setCversion(newCversion);
addChangeRecord(parentRecord);
addChangeRecord(new ChangeRecord(request.hdr.getZxid(), path,
s,
- 0, createRequest.getAcl()));
-
+ 0, listACL));
break;
case OpCode.delete:
zks.sessionTracker.checkSession(request.sessionId,
request.getOwner());
@@ -403,7 +403,8 @@ public class PrepRequestProcessor extend
zks.sessionTracker.checkSession(request.sessionId,
request.getOwner());
SetACLRequest setAclRequest = (SetACLRequest)record;
path = setAclRequest.getPath();
- if (!fixupACL(request.authInfo, setAclRequest.getAcl())) {
+ listACL = removeDuplicates(setAclRequest.getAcl());
+ if (!fixupACL(request.authInfo, listACL)) {
throw new KeeperException.InvalidACLException(path);
}
nodeRecord = getRecordForPath(path);
@@ -415,7 +416,7 @@ public class PrepRequestProcessor extend
throw new KeeperException.BadVersionException(path);
}
version = currentVersion + 1;
- request.txn = new SetACLTxn(path, setAclRequest.getAcl(),
version);
+ request.txn = new SetACLTxn(path, listACL, version);
nodeRecord = nodeRecord.duplicate(request.hdr.getZxid());
nodeRecord.stat.setAversion(version);
addChangeRecord(nodeRecord);
@@ -629,6 +630,20 @@ public class PrepRequestProcessor extend
nextProcessor.processRequest(request);
}
+ private List<ACL> removeDuplicates(List<ACL> acl) {
+
+ ArrayList<ACL> retval = new ArrayList<ACL>();
+ Iterator<ACL> it = acl.iterator();
+ while (it.hasNext()) {
+ ACL a = it.next();
+ if (retval.contains(a) == false) {
+ retval.add(a);
+ }
+ }
+ return retval;
+ }
+
+
/**
* This method checks out the acl making sure it isn't null or empty,
* it has valid schemes and ids, and expanding any relative ids that
@@ -645,6 +660,7 @@ public class PrepRequestProcessor extend
if (acl == null || acl.size() == 0) {
return false;
}
+
Iterator<ACL> it = acl.iterator();
LinkedList<ACL> toAdd = null;
while (it.hasNext()) {
Modified:
zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxn.java
URL:
http://svn.apache.org/viewvc/zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxn.java?rev=1157685&r1=1157684&r2=1157685&view=diff
==============================================================================
--- zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxn.java
(original)
+++ zookeeper/trunk/src/java/main/org/apache/zookeeper/server/ServerCnxn.java
Mon Aug 15 00:35:18 2011
@@ -77,7 +77,9 @@ public abstract class ServerCnxn impleme
}
public void addAuthInfo(Id id) {
- authInfo.add(id);
+ if (authInfo.contains(id) == false) {
+ authInfo.add(id);
+ }
}
public boolean removeAuthInfo(Id id) {
Added: zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ACLCountTest.java
URL:
http://svn.apache.org/viewvc/zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ACLCountTest.java?rev=1157685&view=auto
==============================================================================
--- zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ACLCountTest.java
(added)
+++ zookeeper/trunk/src/java/test/org/apache/zookeeper/test/ACLCountTest.java
Mon Aug 15 00:35:18 2011
@@ -0,0 +1,136 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.zookeeper.test;
+
+import static org.apache.zookeeper.test.ClientBase.CONNECTION_TIMEOUT;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.concurrent.CountDownLatch;
+import java.util.List;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.apache.zookeeper.CreateMode;
+import org.apache.zookeeper.PortAssignment;
+import org.apache.zookeeper.WatchedEvent;
+import org.apache.zookeeper.Watcher;
+import org.apache.zookeeper.ZKTestCase;
+import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.Watcher.Event.KeeperState;
+import org.apache.zookeeper.ZooDefs;
+import org.apache.zookeeper.ZooDefs.Ids;
+import org.apache.zookeeper.data.ACL;
+import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.server.ServerCnxnFactory;
+import org.apache.zookeeper.server.SyncRequestProcessor;
+import org.apache.zookeeper.server.ZooKeeperServer;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ACLCountTest extends ZKTestCase implements Watcher {
+ private static final Logger LOG = LoggerFactory.getLogger(ACLTest.class);
+ private static final String HOSTPORT =
+ "127.0.0.1:" + PortAssignment.unique();
+ private volatile CountDownLatch startSignal;
+
+ /**
+ *
+ * Create a node and add 4 ACL values to it, but there are only 2 unique
ACL values,
+ * and each is repeated once:
+ *
+ * ACL(ZooDefs.Perms.READ,ZooDefs.Ids.ANYONE_ID_UNSAFE);
+ * ACL(ZooDefs.Perms.ALL,ZooDefs.Ids.AUTH_IDS);
+ * ACL(ZooDefs.Perms.READ,ZooDefs.Ids.ANYONE_ID_UNSAFE);
+ * ACL(ZooDefs.Perms.ALL,ZooDefs.Ids.AUTH_IDS);
+ *
+ * Even though we've added 4 ACL values, there should only be 2 ACLs for
that node,
+ * since there are only 2 *unique* ACL values.
+ */
+ @Test
+ public void testAclCount() throws Exception {
+ File tmpDir = ClientBase.createTmpDir();
+ ClientBase.setupTestEnv();
+ ZooKeeperServer zks = new ZooKeeperServer(tmpDir, tmpDir, 3000);
+ SyncRequestProcessor.setSnapCount(1000);
+ final int PORT = Integer.parseInt(HOSTPORT.split(":")[1]);
+ ServerCnxnFactory f = ServerCnxnFactory.createFactory(PORT, -1);
+ f.startup(zks);
+ ZooKeeper zk;
+
+ final ArrayList<ACL> CREATOR_ALL_AND_WORLD_READABLE =
+ new ArrayList<ACL>() { {
+ add(new ACL(ZooDefs.Perms.READ,ZooDefs.Ids.ANYONE_ID_UNSAFE));
+ add(new ACL(ZooDefs.Perms.ALL,ZooDefs.Ids.AUTH_IDS));
+ add(new ACL(ZooDefs.Perms.READ,ZooDefs.Ids.ANYONE_ID_UNSAFE));
+ add(new ACL(ZooDefs.Perms.ALL,ZooDefs.Ids.AUTH_IDS));
+ }};
+
+ try {
+ LOG.info("starting up the zookeeper server .. waiting");
+ Assert.assertTrue("waiting for server being up",
+ ClientBase.waitForServerUp(HOSTPORT, CONNECTION_TIMEOUT));
+ zk = new ZooKeeper(HOSTPORT, CONNECTION_TIMEOUT, this);
+
+ zk.addAuthInfo("digest", "pat:test".getBytes());
+ zk.setACL("/", Ids.CREATOR_ALL_ACL, -1);
+
+ String path = "/path";
+
+ try {
+ Assert.assertEquals(4,CREATOR_ALL_AND_WORLD_READABLE.size());
+ }
+ catch (Exception e) {
+ LOG.error("Something is fundamentally wrong with ArrayList's
add() method. add()ing four times to an empty ArrayList should result in an
ArrayList with 4 members.");
+ throw e;
+ }
+
+
zk.create(path,path.getBytes(),CREATOR_ALL_AND_WORLD_READABLE,CreateMode.PERSISTENT);
+ List<ACL> acls = zk.getACL("/path", new Stat());
+ Assert.assertEquals(2,acls.size());
+ }
+ catch (Exception e) {
+ // test failed somehow.
+ Assert.assertTrue(false);
+ }
+
+ Assert.assertTrue(true);
+
+ }
+
+
+ /*
+ * (non-Javadoc)
+ *
+ * @see
org.apache.zookeeper.Watcher#process(org.apache.zookeeper.WatcherEvent)
+ */
+ public void process(WatchedEvent event) {
+ LOG.info("Event:" + event.getState() + " " + event.getType() + " "
+ + event.getPath());
+ if (event.getState() == KeeperState.SyncConnected) {
+ if (startSignal != null && startSignal.getCount() > 0) {
+ LOG.info("startsignal.countDown()");
+ startSignal.countDown();
+ } else {
+ LOG.warn("startsignal " + startSignal);
+ }
+ }
+ }
+
+}
