You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ha...@apache.org on 2017/06/01 03:45:53 UTC

zookeeper git commit: ZOOKEEPER-2797: Defend against bad TTLs from misbehaving clients

Repository: zookeeper
Updated Branches:
  refs/heads/master e358a808d -> 3824da0b6


ZOOKEEPER-2797: Defend against bad TTLs from misbehaving clients

Validate the TTL before it makes it to the commit processor to prevent blowing up ZK

Author: Patrick White <pa...@patrickwhite.org>

Reviewers: Jordan Zimmerman <jo...@jordanzimmerman.com>, Abraham Fine <af...@apache.org>, Michael Han <ha...@apache.org>

Closes #267 from packysauce/protect_from_bad_ttl


Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/3824da0b
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/3824da0b
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/3824da0b

Branch: refs/heads/master
Commit: 3824da0b649c6a6a698cfe6c79ca3d44a9e94f29
Parents: e358a80
Author: Patrick White <pa...@patrickwhite.org>
Authored: Wed May 31 20:45:49 2017 -0700
Committer: Michael Han <ha...@apache.org>
Committed: Wed May 31 20:45:49 2017 -0700

----------------------------------------------------------------------
 .../zookeeper/server/PrepRequestProcessor.java  | 12 +++++++---
 .../apache/zookeeper/server/CreateTTLTest.java  | 23 +++++++++++++++++++-
 2 files changed, 31 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/3824da0b/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java b/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
index 9ad4eea..a4fc7d3 100644
--- a/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
+++ b/src/java/main/org/apache/zookeeper/server/PrepRequestProcessor.java
@@ -658,10 +658,10 @@ public class PrepRequestProcessor extends ZooKeeperCriticalThread implements
             path = createRequest.getPath();
             acl = createRequest.getAcl();
             data = createRequest.getData();
-            ttl = 0;
+            ttl = -1;
         }
         CreateMode createMode = CreateMode.fromFlag(flags);
-        validateCreateRequest(createMode, request);
+        validateCreateRequest(path, createMode, request, ttl);
         String parentPath = validatePathForCreate(path, request.sessionId);
 
         List<ACL> listACL = fixupACL(path, request.authInfo, acl);
@@ -925,8 +925,14 @@ public class PrepRequestProcessor extends ZooKeeperCriticalThread implements
         return retval;
     }
     
-    private void validateCreateRequest(CreateMode createMode, Request request)
+    private void validateCreateRequest(String path, CreateMode createMode, Request request, long ttl)
             throws KeeperException {
+        try {
+            EphemeralType.validateTTL(createMode, ttl);
+        } catch (IllegalArgumentException e) {
+            BadArgumentsException bae = new BadArgumentsException(path);
+            throw bae;
+        }
         if (createMode.isEphemeral()) {
             // Exception is set when local session failed to upgrade
             // so we just need to report the error

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/3824da0b/src/java/test/org/apache/zookeeper/server/CreateTTLTest.java
----------------------------------------------------------------------
diff --git a/src/java/test/org/apache/zookeeper/server/CreateTTLTest.java b/src/java/test/org/apache/zookeeper/server/CreateTTLTest.java
index 66d17eb..70fa223 100644
--- a/src/java/test/org/apache/zookeeper/server/CreateTTLTest.java
+++ b/src/java/test/org/apache/zookeeper/server/CreateTTLTest.java
@@ -23,9 +23,15 @@ import org.apache.zookeeper.CreateMode;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.Op;
 import org.apache.zookeeper.OpResult;
+import org.apache.zookeeper.TestableZooKeeper;
 import org.apache.zookeeper.ZooDefs;
 import org.apache.zookeeper.ZooKeeper;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.data.Stat;
+import org.apache.zookeeper.proto.CreateResponse;
+import org.apache.zookeeper.proto.CreateTTLRequest;
+import org.apache.zookeeper.proto.ReplyHeader;
+import org.apache.zookeeper.proto.RequestHeader;
 import org.apache.zookeeper.test.ClientBase;
 import org.junit.Assert;
 import org.junit.Test;
@@ -37,7 +43,7 @@ import java.util.List;
 import java.util.concurrent.atomic.AtomicLong;
 
 public class CreateTTLTest extends ClientBase {
-    private ZooKeeper zk;
+    private TestableZooKeeper zk;
 
     @Override
     public void setUp() throws Exception {
@@ -69,6 +75,21 @@ public class CreateTTLTest extends ClientBase {
     }
 
     @Test
+    public void testBadTTLs() throws InterruptedException, KeeperException {
+        RequestHeader h = new RequestHeader(1, ZooDefs.OpCode.createTTL);
+
+        String path = "/bad_ttl";
+        CreateTTLRequest request = new CreateTTLRequest(path, new byte[0], ZooDefs.Ids.OPEN_ACL_UNSAFE,
+                                                        CreateMode.PERSISTENT_WITH_TTL.toFlag(), -100);
+        CreateResponse response = new CreateResponse();
+        ReplyHeader r = zk.submitRequest(h, request, response, null);
+        Assert.assertEquals("An invalid CreateTTLRequest should throw BadArguments",
+                            r.getErr(), Code.BADARGUMENTS.intValue());
+        Assert.assertNull("An invalid CreateTTLRequest should not result in znode creation",
+                          zk.exists(path, false));
+    }
+
+    @Test
     public void testCreateSequential()
             throws IOException, KeeperException, InterruptedException {
         Stat stat = new Stat();