This is an automated email from the ASF dual-hosted git repository.
arshad pushed a commit to branch branch-3.6
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/branch-3.6 by this push:
new df5ee998a ZOOKEEPER-1875: NullPointerException in
ClientCnxn$EventThread.processEvent
df5ee998a is described below
commit df5ee998a9eae6731ed48c7feb5e1136fe964748
Author: Mohammad Arshad <[email protected]>
AuthorDate: Sat Apr 16 15:37:42 2022 +0530
ZOOKEEPER-1875: NullPointerException in ClientCnxn$EventThread.processEvent
Author: Mohammad Arshad <[email protected]>
Reviewers: Mate Szalay-Beko <[email protected]>, Enrico Olivelli
<[email protected]>
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
(cherry picked from commit 86690ff40cb5c9f5782f0971db16e8fd1c3528e6)
Signed-off-by: Mohammad Arshad <[email protected]>
---
.../main/java/org/apache/zookeeper/ZooKeeper.java | 78 +++++++++++++++++++---
.../java/org/apache/zookeeper/ZooKeeperTest.java | 24 +++++++
.../zookeeper/server/quorum/QuorumDigestTest.java | 2 +-
.../apache/zookeeper/test/ObserverMasterTest.java | 2 +-
4 files changed, 93 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 096020f18..c724bc7ea 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java
@@ -812,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, int sessionTimeout, Watcher
watcher) throws IOException {
this(connectString, sessionTimeout, watcher, false);
@@ -861,7 +866,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,
@@ -926,7 +936,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,
@@ -994,7 +1009,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,
@@ -1009,6 +1029,7 @@ public class ZooKeeper implements AutoCloseable {
sessionTimeout,
watcher);
+ validateWatcher(watcher);
if (clientConfig == null) {
clientConfig = new ZKClientConfig();
}
@@ -1098,7 +1119,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,
@@ -1160,7 +1186,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,
@@ -1226,8 +1257,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,
@@ -1300,7 +1336,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,
@@ -1387,7 +1429,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(
@@ -1408,6 +1455,7 @@ public class ZooKeeper implements AutoCloseable {
Long.toHexString(sessionId),
(sessionPasswd == null ? "<null>" : "<hidden>"));
+ validateWatcher(watcher);
if (clientConfig == null) {
clientConfig = new ZKClientConfig();
}
@@ -1491,7 +1539,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,
@@ -1579,10 +1633,12 @@ public class ZooKeeper implements AutoCloseable {
/**
* Specify the default watcher for the connection (overrides the one
* specified during construction).
+ * @throws IllegalArgumentException if watcher is null
*
* @param watcher
*/
public synchronized void register(Watcher watcher) {
+ validateWatcher(watcher);
watchManager.defaultWatcher = 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 8fc3df250..48e2468d1 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java
@@ -726,4 +726,28 @@ public class ZooKeeperTest extends ClientBase {
assertTrue("ZooKeeperMain does not wait until the specified timeout",
endTime - startTime >= timeout);
}
+
+ @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 691b45513..2f7873de5 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
@@ -203,7 +203,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 ee54a7ae5..2d7735cdd 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
@@ -413,7 +413,7 @@ public class ObserverMasterTest extends QuorumPeerTestBase
implements Watcher {
public void testInOrderCommits() throws Exception {
setUp(-1);
- 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);
