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/05/26 22:52:31 UTC
zookeeper git commit: ZOOKEEPER-2757: Incorrect path crashes zkCli
Repository: zookeeper
Updated Branches:
refs/heads/branch-3.5 561a45a1e -> ac544de9b
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 <af...@apache.org>
Reviewers: Michael Han <ha...@apache.org>
Closes #240 from afine/ZOOKEEPER-2757
Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/ac544de9
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/ac544de9
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/ac544de9
Branch: refs/heads/branch-3.5
Commit: ac544de9b63f536d4a38fc9413782d52a0accf75
Parents: 561a45a
Author: Abraham Fine <af...@apache.org>
Authored: Fri May 26 15:52:28 2017 -0700
Committer: Michael Han <ha...@apache.org>
Committed: Fri May 26 15:52:28 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/ac544de9/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 3868161..3d3f3ff 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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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/ac544de9/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;
}