[zookeeper] branch master updated: ZOOKEEPER-4377: KeeperException.create has NullPointerException when low version client requests the high version server

[ma...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

maoling pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new 9f355f5  ZOOKEEPER-4377: KeeperException.create has NullPointerException when low version client requests the high version server
9f355f5 is described below

commit 9f355f5a57f35d3760f8e669696622135c457938
Author: maoling <ma...@apache.org>
AuthorDate: Sun Oct 17 15:07:58 2021 +0800

    ZOOKEEPER-4377: KeeperException.create has NullPointerException when low version client requests the high version server
    
    - When low version client accessed the high version server which has some new added error code, the client will get a NPE:
    ```
     java.lang.NullPointerException
    at org.apache.zookeeper.KeeperException.create(KeeperException.java:94)
    at org.apache.zookeeper.KeeperException.create(KeeperException.java:54)
    at org.apache.zookeeper.ZooKeeper.create(ZooKeeper.java:1538)
    ```
    - How to reproduce this issue？For example:
    ```
    the client version we using is 3.6.0, and server version we using is 3.7.0 which has a new added error code QUOTAEXCEEDED(-125),
    we set quota at server side and use the client to create znodes which exceeds the quota,
    the client will get a NPE
    ```
    - Apply this patch, we will get the following:
    ```
     java.lang.IllegalArgumentException: The current client version cannot lookup this code:-125
     at org.apache.zookeeper.KeeperException$Code.get(KeeperException.java:449)
     at org.apache.zookeeper.ZooKeeper.create(ZooKeeper.java:1347)
    ```
    - we should backport this PR to all branches, making the client has upward compatibility
    - more details in the [ZOOKEEPER-4377](https://issues.apache.org/jira/browse/ZOOKEEPER-4377)
    
    Author: maoling <ma...@apache.org>
    
    Reviewers: Enrico Olivelli <eo...@apache.org>, ruanwenjun <we...@apache.org>
    
    Closes #1764 from maoling/ZOOKEEPER-4377
---
 .../src/main/java/org/apache/zookeeper/KeeperException.java | 10 +++++++---
 .../src/test/java/org/apache/zookeeper/ZooKeeperTest.java   | 13 +++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java b/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
index 4590e0f..ef38763 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
@@ -154,7 +154,7 @@ public abstract class KeeperException extends Exception {
             return new ThrottledOpException();
         case OK:
         default:
-            throw new IllegalArgumentException("Invalid exception code");
+            throw new IllegalArgumentException("Invalid exception code:" + code.code);
         }
     }
 
@@ -441,10 +441,14 @@ public abstract class KeeperException extends Exception {
         /**
          * Get the Code value for a particular integer error code
          * @param code int error code
-         * @return Code value corresponding to specified int code, or null
+         * @return Code value corresponding to specified int code, if null throws IllegalArgumentException
          */
         public static Code get(int code) {
-            return lookup.get(code);
+            Code codeVal = lookup.get(code);
+            if (codeVal == null) {
+                throw new IllegalArgumentException("The current client version cannot lookup this code:" + code);
+            }
+            return codeVal;
         }
     }
 
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 47ab6c4..c829e85 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java
@@ -18,9 +18,11 @@
 
 package org.apache.zookeeper;
 
+import static org.apache.zookeeper.KeeperException.Code.NOAUTH;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 import java.io.ByteArrayOutputStream;
@@ -31,6 +33,7 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicInteger;
 import org.apache.zookeeper.AsyncCallback.VoidCallback;
+import org.apache.zookeeper.KeeperException.Code;
 import org.apache.zookeeper.ZooDefs.Ids;
 import org.apache.zookeeper.cli.CliCommand;
 import org.apache.zookeeper.cli.CliException;
@@ -802,4 +805,14 @@ public class ZooKeeperTest extends ClientBase {
 
     }
 
+    @Test
+    public void testKeeperExceptionCreateNPE() {
+        // One existing code
+        KeeperException k1 = KeeperException.create(Code.get(NOAUTH.intValue()));
+        assertTrue(k1 instanceof KeeperException.NoAuthException);
+
+        // One impossible code
+        assertThrows(IllegalArgumentException.class, () -> KeeperException.create(Code.get(Integer.MAX_VALUE)));
+    }
+
 }