diff options
author | Mohammad Arshad <arshad@apache.org> | 2022-04-16 15:37:42 +0530 |
---|---|---|
committer | Mohammad Arshad <arshad@apache.org> | 2022-04-16 15:37:42 +0530 |
commit | 86690ff40cb5c9f5782f0971db16e8fd1c3528e6 (patch) | |
tree | 921b210c1e0e01c51b1b295add2491cb24ffb844 /zookeeper-server | |
parent | e5f84f46283d70b618ce00f2df51f4bfe31d9a44 (diff) | |
download | zookeeper-86690ff40cb5c9f5782f0971db16e8fd1c3528e6.tar.gz |
ZOOKEEPER-1875: NullPointerException in ClientCnxn$EventThread.processEvent
Author: Mohammad Arshad <arshad@apache.org>
Reviewers: Mate Szalay-Beko <symat@apache.org>, Enrico Olivelli <eolivelli@apache.org>
Closes #1855 from arshadmohammad/ZOOKEEPER-1875-npe and squashes the following commits:
4a7d471e3 [Mohammad Arshad] Corrected impacted test cases
12f44d62e [Mohammad Arshad] ZOOKEEPER-1875: NullPointerException in ClientCnxn$EventThread.processEvent
Diffstat (limited to 'zookeeper-server')
4 files changed, 92 insertions, 13 deletions
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java index 127f5a2a3..281355b1a 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java @@ -442,7 +442,12 @@ public class ZooKeeper implements AutoCloseable { * @throws IOException * in cases of network failure * @throws IllegalArgumentException - * if an invalid chroot path is specified + * if any of the following is true: + * <ul> + * <li> if an invalid chroot path is specified + * <li> for an invalid list of ZooKeeper hosts + * <li> watcher is null + * </ul> */ public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher) throws IOException { this(connectString, sessionTimeout, watcher, false); @@ -491,7 +496,12 @@ public class ZooKeeper implements AutoCloseable { * @throws IOException * in cases of network failure * @throws IllegalArgumentException - * if an invalid chroot path is specified + * if any of the following is true: + * <ul> + * <li> if an invalid chroot path is specified + * <li> for an invalid list of ZooKeeper hosts + * <li> watcher is null + * </ul> */ public ZooKeeper( String connectString, @@ -556,7 +566,12 @@ public class ZooKeeper implements AutoCloseable { * @throws IOException * in cases of network failure * @throws IllegalArgumentException - * if an invalid chroot path is specified + * if any of the following is true: + * <ul> + * <li> if an invalid chroot path is specified + * <li> for an invalid list of ZooKeeper hosts + * <li> watcher is null + * </ul> */ public ZooKeeper( String connectString, @@ -624,7 +639,12 @@ public class ZooKeeper implements AutoCloseable { * @throws IOException * in cases of network failure * @throws IllegalArgumentException - * if an invalid chroot path is specified + * if any of the following is true: + * <ul> + * <li> if an invalid chroot path is specified + * <li> for an invalid list of ZooKeeper hosts + * <li> watcher is null + * </ul> */ public ZooKeeper( String connectString, @@ -640,6 +660,7 @@ public class ZooKeeper implements AutoCloseable { sessionTimeout, watcher); + validateWatcher(watcher); this.clientConfig = clientConfig != null ? clientConfig : new ZKClientConfig(); this.hostProvider = hostProvider; ConnectStringParser connectStringParser = new ConnectStringParser(connectString); @@ -724,7 +745,12 @@ public class ZooKeeper implements AutoCloseable { * @throws IOException * in cases of network failure * @throws IllegalArgumentException - * if an invalid chroot path is specified + * if any of the following is true: + * <ul> + * <li> if an invalid chroot path is specified + * <li> for an invalid list of ZooKeeper hosts + * <li> watcher is null + * </ul> */ public ZooKeeper( String connectString, @@ -786,7 +812,12 @@ public class ZooKeeper implements AutoCloseable { * @throws IOException * in cases of network failure * @throws IllegalArgumentException - * if an invalid chroot path is specified + * if any of the following is true: + * <ul> + * <li> if an invalid chroot path is specified + * <li> for an invalid list of ZooKeeper hosts + * <li> watcher is null + * </ul> */ public ZooKeeper( String connectString, @@ -852,8 +883,13 @@ public class ZooKeeper implements AutoCloseable { * password for this session * * @throws IOException in cases of network failure - * @throws IllegalArgumentException if an invalid chroot path is specified - * @throws IllegalArgumentException for an invalid list of ZooKeeper hosts + * @throws IllegalArgumentException + * if any of the following is true: + * <ul> + * <li> if an invalid chroot path is specified + * <li> for an invalid list of ZooKeeper hosts + * <li> watcher is null + * </ul> */ public ZooKeeper( String connectString, @@ -926,7 +962,13 @@ public class ZooKeeper implements AutoCloseable { * @param aHostProvider * use this as HostProvider to enable custom behaviour. * @throws IOException in cases of network failure - * @throws IllegalArgumentException if an invalid chroot path is specified + * @throws IllegalArgumentException + * if any of the following is true: + * <ul> + * <li> if an invalid chroot path is specified + * <li> for an invalid list of ZooKeeper hosts + * <li> watcher is null + * </ul> */ public ZooKeeper( String connectString, @@ -1013,7 +1055,12 @@ public class ZooKeeper implements AutoCloseable { * configuring properties differently compared to other instances * @throws IOException in cases of network failure * @throws IllegalArgumentException if an invalid chroot path is specified - * + * if any of the following is true: + * <ul> + * <li> if an invalid chroot path is specified + * <li> for an invalid list of ZooKeeper hosts + * <li> watcher is null + * </ul> * @since 3.5.5 */ public ZooKeeper( @@ -1034,6 +1081,7 @@ public class ZooKeeper implements AutoCloseable { Long.toHexString(sessionId), (sessionPasswd == null ? "<null>" : "<hidden>")); + validateWatcher(watcher); this.clientConfig = clientConfig != null ? clientConfig : new ZKClientConfig(); ConnectStringParser connectStringParser = new ConnectStringParser(connectString); this.hostProvider = hostProvider; @@ -1111,7 +1159,13 @@ public class ZooKeeper implements AutoCloseable { * allowed while write requests are not. It continues seeking for * majority in the background. * @throws IOException in cases of network failure - * @throws IllegalArgumentException if an invalid chroot path is specified + * @throws IllegalArgumentException + * if any of the following is true: + * <ul> + * <li> if an invalid chroot path is specified + * <li> for an invalid list of ZooKeeper hosts + * <li> watcher is null + * </ul> */ public ZooKeeper( String connectString, @@ -1194,8 +1248,10 @@ public class ZooKeeper implements AutoCloseable { /** * Specify the default watcher for the connection (overrides the one * specified during construction). + * @throws IllegalArgumentException if watcher is null */ public synchronized void register(Watcher watcher) { + validateWatcher(watcher); getWatchManager().setDefaultWatcher(watcher); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java index c829e85da..9d45802bb 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java @@ -815,4 +815,27 @@ public class ZooKeeperTest extends ClientBase { assertThrows(IllegalArgumentException.class, () -> KeeperException.create(Code.get(Integer.MAX_VALUE))); } + @Test + public void testInvalidWatcherRegistration() throws Exception { + String nullWatcherMsg = "Invalid Watcher, shouldn't be null!"; + final ZooKeeper zk = createClient(); + try { + zk.register(null); + fail("Should validate null watcher!"); + } catch (IllegalArgumentException iae) { + assertEquals(nullWatcherMsg, iae.getMessage()); + } + try { + new ZooKeeper(hostPort, 10000, null, false); + fail("Should validate null watcher!"); + } catch (IllegalArgumentException iae) { + assertEquals(nullWatcherMsg, iae.getMessage()); + } + try { + new ZooKeeper(hostPort, 10000, null, 10L, "".getBytes(), false); + fail("Should validate null watcher!"); + } catch (IllegalArgumentException iae) { + assertEquals(nullWatcherMsg, iae.getMessage()); + } + } } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumDigestTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumDigestTest.java index af2fed8cf..a9e65a445 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumDigestTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumDigestTest.java @@ -204,7 +204,7 @@ public class QuorumDigestTest extends QuorumPeerTestBase { private void triggerOps(int sid, String prefix) throws Exception { TxnLogDigestTest.performOperations(servers.zk[sid], prefix); - servers.restartClient(sid, null); + servers.restartClient(sid, event -> { }); waitForOne(servers.zk[sid], States.CONNECTED); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java index 4ba33c7a8..ab30a1de0 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ObserverMasterTest.java @@ -213,7 +213,7 @@ public class ObserverMasterTest extends ObserverMasterTestBase { public void testInOrderCommits(boolean testObserverMaster) throws Exception { setUp(-1, testObserverMaster); - zk = new ZooKeeper("127.0.0.1:" + CLIENT_PORT_QP1, ClientBase.CONNECTION_TIMEOUT, null); + zk = new ZooKeeper("127.0.0.1:" + CLIENT_PORT_QP1, ClientBase.CONNECTION_TIMEOUT, event -> { }); for (int i = 0; i < 10; i++) { zk.create("/bulk" + i, ("Initial data of some size").getBytes(), Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); |