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

[sy...@apache.org]

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

symat pushed a commit to branch branch-3.5
in repository https://gitbox.apache.org/repos/asf/zookeeper.git

commit 071e93f9c940c5fd8be3269f32aeac674d4e508d
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
    
    (cherry picked from commit 9f355f5a57f35d3760f8e669696622135c457938)
    Signed-off-by: maoling <ma...@apache.org>
    (cherry picked from commit 86c12634d22bbfee8cf9f5434c49eebe3ffa84c0)
---
 .../main/java/org/apache/zookeeper/KeeperException.java    | 10 +++++++---
 .../src/test/java/org/apache/zookeeper/ZooKeeperTest.java  | 14 ++++++++++++++
 2 files changed, 21 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 42f7a3392..ed7c7af03 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/KeeperException.java
@@ -150,7 +150,7 @@ public abstract class KeeperException extends Exception {
                 return new RequestTimeoutException();
             case OK:
             default:
-                throw new IllegalArgumentException("Invalid exception code");
+                throw new IllegalArgumentException("Invalid exception code:" + code.code);
         }
     }
 
@@ -427,10 +427,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 00c4125e7..d8bbf38d6 100644
--- a/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java
+++ b/zookeeper-server/src/test/java/org/apache/zookeeper/ZooKeeperTest.java
@@ -17,6 +17,7 @@
  */
 package org.apache.zookeeper;
 
+import static org.apache.zookeeper.KeeperException.Code.NOAUTH;
 import static org.junit.Assert.*;
 
 import java.io.ByteArrayOutputStream;
@@ -27,6 +28,7 @@ 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.*;
 import org.apache.zookeeper.client.ConnectStringParser;
@@ -630,4 +632,16 @@ public class ZooKeeperTest extends ClientBase {
 
         runCommandExpect(cmd, expected);
     }
+
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testKeeperExceptionCreateNPE() {
+        // One existing code
+        KeeperException k1 = KeeperException.create(Code.get(NOAUTH.intValue()));
+        assertTrue(k1 instanceof KeeperException.NoAuthException);
+
+        // One impossible code (should throw IllegalArgumentException)
+        KeeperException.create(Code.get(Integer.MAX_VALUE));
+    }
+
 }