zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From h...@apache.org
Subject zookeeper git commit: ZOOKEEPER-2757: Incorrect path crashes zkCli
Date Fri, 26 May 2017 22:52:40 GMT
Repository: zookeeper
Updated Branches:
  refs/heads/master 0a3e2d1d4 -> b0b194533


ZOOKEEPER-2757: Incorrect path crashes zkCli

This issue is caused by ZooKeeper relying on `IllegalArgumentException` in `PathUtils#validatePath`.
`IllegalArgumentException` is an unchecked exception and we never catch it within each individual
*Command.java, so it bubbles up and kills the CLI.

Given that throwing `IllegalArgumentException` is part of ZooKeeper's API, I believe that
unfortunately we can not change this behavior at this time. This patch catches `IllegalArgumentException`
and wraps it, so the CLI prints an error but does not quit. I believe I handled all of the
relevant commands, please check to make sure I am not missing any.

Author: Abraham Fine <afine@apache.org>

Reviewers: Michael Han <hanm@apache.org>

Closes #240 from afine/ZOOKEEPER-2757

(cherry picked from commit ac544de9b63f536d4a38fc9413782d52a0accf75)
Signed-off-by: Michael Han <hanm@apache.org>


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

Branch: refs/heads/master
Commit: b0b194533c08c8a111497e853668f751b28ae4e9
Parents: 0a3e2d1
Author: Abraham Fine <afine@apache.org>
Authored: Fri May 26 15:52:28 2017 -0700
Committer: Michael Han <hanm@apache.org>
Committed: Fri May 26 15:52:37 2017 -0700

----------------------------------------------------------------------
 .../org/apache/zookeeper/cli/CreateCommand.java |  2 ++
 .../apache/zookeeper/cli/DelQuotaCommand.java   |  4 +++-
 .../apache/zookeeper/cli/DeleteAllCommand.java  |  2 ++
 .../org/apache/zookeeper/cli/DeleteCommand.java |  4 +++-
 .../org/apache/zookeeper/cli/GetAclCommand.java |  2 ++
 .../org/apache/zookeeper/cli/GetCommand.java    |  2 ++
 .../apache/zookeeper/cli/ListQuotaCommand.java  |  2 ++
 .../org/apache/zookeeper/cli/Ls2Command.java    |  2 ++
 .../org/apache/zookeeper/cli/LsCommand.java     |  2 ++
 .../zookeeper/cli/MalformedPathException.java   | 25 ++++++++++++++++++++
 .../zookeeper/cli/RemoveWatchesCommand.java     |  4 +++-
 .../org/apache/zookeeper/cli/SetAclCommand.java |  2 ++
 .../org/apache/zookeeper/cli/SetCommand.java    |  2 ++
 .../apache/zookeeper/cli/SetQuotaCommand.java   |  9 +++++--
 .../org/apache/zookeeper/cli/StatCommand.java   |  2 ++
 .../org/apache/zookeeper/cli/SyncCommand.java   | 15 ++++++++----
 16 files changed, 71 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/CreateCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/CreateCommand.java b/src/java/main/org/apache/zookeeper/cli/CreateCommand.java
index ee6f58a..46ffc17 100644
--- a/src/java/main/org/apache/zookeeper/cli/CreateCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/CreateCommand.java
@@ -118,6 +118,8 @@ public class CreateCommand extends CliCommand {
         try {
             String newPath = hasT ? zk.create(path, data, acl, flags, new Stat(), ttl) :
zk.create(path, data, acl, flags);
             err.println("Created " + newPath);
+        } catch(IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch(KeeperException.EphemeralOnLocalSessionException e) {
             err.println("Unable to create ephemeral node on a local session");
             throw new CliWrapperException(e);

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/DelQuotaCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/DelQuotaCommand.java b/src/java/main/org/apache/zookeeper/cli/DelQuotaCommand.java
index 31a0546..8005406 100644
--- a/src/java/main/org/apache/zookeeper/cli/DelQuotaCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/DelQuotaCommand.java
@@ -95,7 +95,7 @@ public class DelQuotaCommand extends CliCommand {
      */
     public static boolean delQuota(ZooKeeper zk, String path,
             boolean bytes, boolean numNodes)
-            throws KeeperException, IOException, InterruptedException {
+            throws KeeperException, IOException, InterruptedException, MalformedPathException
{
         String parentPath = Quotas.quotaZookeeper + path;
         String quotaPath = Quotas.quotaZookeeper + path + "/" + 
                 Quotas.limitNode;
@@ -106,6 +106,8 @@ public class DelQuotaCommand extends CliCommand {
         byte[] data = null;
         try {
             data = zk.getData(quotaPath, false, new Stat());
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException.NoNodeException ne) {
             System.err.println("quota does not exist for " + path);
             return true;

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/DeleteAllCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/DeleteAllCommand.java b/src/java/main/org/apache/zookeeper/cli/DeleteAllCommand.java
index 4d1b749..a3f7853 100644
--- a/src/java/main/org/apache/zookeeper/cli/DeleteAllCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/DeleteAllCommand.java
@@ -65,6 +65,8 @@ public class DeleteAllCommand extends CliCommand {
         String path = args[1];
         try {
             ZKUtil.deleteRecursive(zk, path);
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException|InterruptedException ex) {
             throw new CliWrapperException(ex);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/DeleteCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/DeleteCommand.java b/src/java/main/org/apache/zookeeper/cli/DeleteCommand.java
index e2be214..d3c67b6 100644
--- a/src/java/main/org/apache/zookeeper/cli/DeleteCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/DeleteCommand.java
@@ -80,7 +80,9 @@ public class DeleteCommand extends CliCommand {
         }
         
         try {
-        zk.delete(path, version);
+            zk.delete(path, version);
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch(KeeperException|InterruptedException ex) {
             throw new CliWrapperException(ex);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/GetAclCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/GetAclCommand.java b/src/java/main/org/apache/zookeeper/cli/GetAclCommand.java
index 3f9e22b..b5feb60 100644
--- a/src/java/main/org/apache/zookeeper/cli/GetAclCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/GetAclCommand.java
@@ -67,6 +67,8 @@ public class GetAclCommand extends CliCommand {
         List<ACL> acl;
         try {
            acl = zk.getACL(path, stat);
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException|InterruptedException ex) {
             throw new CliWrapperException(ex);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/GetCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/GetCommand.java b/src/java/main/org/apache/zookeeper/cli/GetCommand.java
index bbc63e6..6e58a5e 100644
--- a/src/java/main/org/apache/zookeeper/cli/GetCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/GetCommand.java
@@ -83,6 +83,8 @@ public class GetCommand extends CliCommand {
         byte data[];
         try {
             data = zk.getData(path, watch, stat);
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException|InterruptedException ex) {
             throw new CliException(ex);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/ListQuotaCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/ListQuotaCommand.java b/src/java/main/org/apache/zookeeper/cli/ListQuotaCommand.java
index e3b9083..8c51c26 100644
--- a/src/java/main/org/apache/zookeeper/cli/ListQuotaCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/ListQuotaCommand.java
@@ -69,6 +69,8 @@ public class ListQuotaCommand extends CliCommand {
                     + Quotas.statNode, false, stat);
             out.println("Output stat for " + path + " "
                     + new StatsTrack(new String(data)).toString());
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException.NoNodeException ne) {
             err.println("quota for " + path + " does not exist.");
         } catch (KeeperException|InterruptedException ex) {

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/Ls2Command.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/Ls2Command.java b/src/java/main/org/apache/zookeeper/cli/Ls2Command.java
index 43f5d0e..aed1b0e 100644
--- a/src/java/main/org/apache/zookeeper/cli/Ls2Command.java
+++ b/src/java/main/org/apache/zookeeper/cli/Ls2Command.java
@@ -60,6 +60,8 @@ public class Ls2Command extends CliCommand {
         List<String> children;
         try {
             children = zk.getChildren(path, watch, stat);
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException|InterruptedException ex) {
             throw new CliWrapperException(ex);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/LsCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/LsCommand.java b/src/java/main/org/apache/zookeeper/cli/LsCommand.java
index ebb1cdd..9e53d5d 100644
--- a/src/java/main/org/apache/zookeeper/cli/LsCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/LsCommand.java
@@ -108,6 +108,8 @@ public class LsCommand extends CliCommand {
                 List<String> children = zk.getChildren(path, watch, stat);
                 printChildren(children, stat);
             }
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException|InterruptedException ex) {
             throw new CliWrapperException(ex);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/MalformedPathException.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/MalformedPathException.java b/src/java/main/org/apache/zookeeper/cli/MalformedPathException.java
new file mode 100644
index 0000000..e65765b
--- /dev/null
+++ b/src/java/main/org/apache/zookeeper/cli/MalformedPathException.java
@@ -0,0 +1,25 @@
+/**
+ * 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.cli;
+
+@SuppressWarnings("serial")
+public class MalformedPathException extends CliException {
+    public MalformedPathException(String message) {
+        super(message);
+    }
+}

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/RemoveWatchesCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/RemoveWatchesCommand.java b/src/java/main/org/apache/zookeeper/cli/RemoveWatchesCommand.java
index 604cd4b..2863443 100644
--- a/src/java/main/org/apache/zookeeper/cli/RemoveWatchesCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/RemoveWatchesCommand.java
@@ -62,7 +62,7 @@ public class RemoveWatchesCommand extends CliCommand {
     }
 
     @Override
-    public boolean exec() throws CliWrapperException {
+    public boolean exec() throws CliWrapperException, MalformedPathException {
         String path = args[1];
         WatcherType wtype = WatcherType.Any;
         // if no matching option -c or -d or -a is specified, we remove
@@ -79,6 +79,8 @@ public class RemoveWatchesCommand extends CliCommand {
 
         try {
             zk.removeAllWatches(path, wtype, local);
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException|InterruptedException ex) {
             throw new CliWrapperException(ex);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/SetAclCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/SetAclCommand.java b/src/java/main/org/apache/zookeeper/cli/SetAclCommand.java
index 7fb4f0f..d2cfc0d 100644
--- a/src/java/main/org/apache/zookeeper/cli/SetAclCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/SetAclCommand.java
@@ -73,6 +73,8 @@ public class SetAclCommand extends CliCommand {
             if (cl.hasOption("s")) {
                 new StatPrinter(out).print(stat);
             }
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException|InterruptedException ex) {
             throw new CliWrapperException(ex);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/SetCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/SetCommand.java b/src/java/main/org/apache/zookeeper/cli/SetCommand.java
index 10bb129..43ca2e1 100644
--- a/src/java/main/org/apache/zookeeper/cli/SetCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/SetCommand.java
@@ -71,6 +71,8 @@ public class SetCommand extends CliCommand {
             if (cl.hasOption("s")) {
                 new StatPrinter(out).print(stat);
             }
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException|InterruptedException ex) {
             throw new CliWrapperException(ex);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/SetQuotaCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/SetQuotaCommand.java b/src/java/main/org/apache/zookeeper/cli/SetQuotaCommand.java
index cd670a2..7df5667 100644
--- a/src/java/main/org/apache/zookeeper/cli/SetQuotaCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/SetQuotaCommand.java
@@ -91,11 +91,16 @@ public class SetQuotaCommand extends CliCommand {
 
     public static boolean createQuota(ZooKeeper zk, String path,
             long bytes, int numNodes)
-            throws KeeperException, IOException, InterruptedException {
+            throws KeeperException, IOException, InterruptedException, MalformedPathException
{
         // check if the path exists. We cannot create
         // quota for a path that already exists in zookeeper
         // for now.
-        Stat initStat = zk.exists(path, false);
+        Stat initStat;
+        try {
+            initStat = zk.exists(path, false);
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
+        }
         if (initStat == null) {
             throw new IllegalArgumentException(path + " does not exist.");
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/StatCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/StatCommand.java b/src/java/main/org/apache/zookeeper/cli/StatCommand.java
index b37071b..33d8e87 100644
--- a/src/java/main/org/apache/zookeeper/cli/StatCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/StatCommand.java
@@ -81,6 +81,8 @@ public class StatCommand extends CliCommand {
         Stat stat;
         try {
             stat = zk.exists(path, watch);
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
         } catch (KeeperException|InterruptedException ex) {
             throw new CliWrapperException(ex);
         }

http://git-wip-us.apache.org/repos/asf/zookeeper/blob/b0b19453/src/java/main/org/apache/zookeeper/cli/SyncCommand.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/cli/SyncCommand.java b/src/java/main/org/apache/zookeeper/cli/SyncCommand.java
index f79db88..74affd2 100644
--- a/src/java/main/org/apache/zookeeper/cli/SyncCommand.java
+++ b/src/java/main/org/apache/zookeeper/cli/SyncCommand.java
@@ -57,12 +57,17 @@ public class SyncCommand extends CliCommand {
     @Override
     public boolean exec() throws CliException {
         String path = args[1];
-        zk.sync(path, new AsyncCallback.VoidCallback() {
+        try {
+            zk.sync(path, new AsyncCallback.VoidCallback() {
+
+                public void processResult(int rc, String path, Object ctx) {
+                    out.println("Sync returned " + rc);
+                }
+            }, null);
+        } catch (IllegalArgumentException ex) {
+            throw new MalformedPathException(ex.getMessage());
+        }
 
-            public void processResult(int rc, String path, Object ctx) {
-                out.println("Sync returned " + rc);
-            }
-        }, null);
 
         return false;
     }


Mime
View raw message