Skip to content

Commit

Permalink
ZOOKEEPER-1875: NullPointerException in ClientCnxn$EventThread.proces…
Browse files Browse the repository at this point in the history
…sEvent
  • Loading branch information
arshadmohammad committed Apr 9, 2022
1 parent 54cb5c3 commit 12f44d6
Show file tree
Hide file tree
Showing 2 changed files with 90 additions and 11 deletions.
78 changes: 67 additions & 11 deletions zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,12 @@ public boolean isConnected() {
* @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);
Expand Down Expand Up @@ -491,7 +496,12 @@ public ZooKeeper(String connectString, int sessionTimeout, Watcher watcher) thro
* @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,
Expand Down Expand Up @@ -556,7 +566,12 @@ public ZooKeeper(
* @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,
Expand Down Expand Up @@ -624,7 +639,12 @@ public ZooKeeper(
* @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,
Expand All @@ -640,6 +660,7 @@ public ZooKeeper(
sessionTimeout,
watcher);

validateWatcher(watcher);
this.clientConfig = clientConfig != null ? clientConfig : new ZKClientConfig();
this.hostProvider = hostProvider;
ConnectStringParser connectStringParser = new ConnectStringParser(connectString);
Expand Down Expand Up @@ -724,7 +745,12 @@ ClientCnxn createConnection(
* @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,
Expand Down Expand Up @@ -786,7 +812,12 @@ public ZooKeeper(
* @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,
Expand Down Expand Up @@ -852,8 +883,13 @@ public ZooKeeper(
* 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,
Expand Down Expand Up @@ -926,7 +962,13 @@ public ZooKeeper(
* @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,
Expand Down Expand Up @@ -1013,7 +1055,12 @@ public ZooKeeper(
* 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(
Expand All @@ -1034,6 +1081,7 @@ public ZooKeeper(
Long.toHexString(sessionId),
(sessionPasswd == null ? "<null>" : "<hidden>"));

validateWatcher(watcher);
this.clientConfig = clientConfig != null ? clientConfig : new ZKClientConfig();
ConnectStringParser connectStringParser = new ConnectStringParser(connectString);
this.hostProvider = hostProvider;
Expand Down Expand Up @@ -1111,7 +1159,13 @@ public ZooKeeper(
* 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,
Expand Down Expand Up @@ -1194,8 +1248,10 @@ public void addAuthInfo(String scheme, byte[] auth) {
/**
* 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -815,4 +815,27 @@ public void testKeeperExceptionCreateNPE() {
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());
}
}
}

0 comments on commit 12f44d6

Please sign in to comment.