JedisClusterCRC16.getSlot() wraps NullPointerException

src/main/java/redis/clients/jedis/JedisClusterCommand.java

@@ -11,8 +11,6 @@
 
 public abstract class JedisClusterCommand<T> {
 
-  private static final String NO_DISPATCH_MESSAGE = "No way to dispatch this command to Redis Cluster.";
-
   private final JedisClusterConnectionHandler connectionHandler;
   private final int maxAttempts;
   private final ThreadLocal<Jedis> askConnection = new ThreadLocal<Jedis>();
@@ -25,16 +23,12 @@ public JedisClusterCommand(JedisClusterConnectionHandler connectionHandler, int
   public abstract T execute(Jedis connection);
 
   public T run(String key) {
-    if (key == null) {
-      throw new JedisClusterOperationException(NO_DISPATCH_MESSAGE);
-    }
-
     return runWithRetries(JedisClusterCRC16.getSlot(key), this.maxAttempts, false, false);
   }
 
   public T run(int keyCount, String... keys) {
     if (keys == null || keys.length == 0) {
-      throw new JedisClusterOperationException(NO_DISPATCH_MESSAGE);
+      throw new JedisClusterOperationException("No way to dispatch this command to Redis Cluster.");
     }
 
     // For multiple keys, only execute if they all share the same connection slot.
@@ -53,16 +47,12 @@ public T run(int keyCount, String... keys) {
   }
 
   public T runBinary(byte[] key) {
-    if (key == null) {
-      throw new JedisClusterOperationException(NO_DISPATCH_MESSAGE);
-    }
-
     return runWithRetries(JedisClusterCRC16.getSlot(key), this.maxAttempts, false, false);
   }
 
   public T runBinary(int keyCount, byte[]... keys) {
     if (keys == null || keys.length == 0) {
-      throw new JedisClusterOperationException(NO_DISPATCH_MESSAGE);
+      throw new JedisClusterOperationException("No way to dispatch this command to Redis Cluster.");
     }
 
     // For multiple keys, only execute if they all share the same connection slot.

src/main/java/redis/clients/jedis/util/JedisClusterCRC16.java

@@ -1,5 +1,7 @@
 package redis.clients.jedis.util;
 
+import redis.clients.jedis.exceptions.JedisClusterOperationException;
+
 /**
  * CRC16 Implementation according to CCITT standard Polynomial : 1021 (x^16 + x^12 + x^5 + 1) See <a
  * href="http://redis.io/topics/cluster-spec">Appendix A. CRC16 reference implementation in ANSI
@@ -36,13 +38,20 @@ private JedisClusterCRC16(){
   }
 
   public static int getSlot(String key) {
+    if (key == null) {
+      throw new JedisClusterOperationException("Slot calculation of null is impossible");
+    }
+
     key = JedisClusterHashTagUtil.getHashTag(key);
-    // optimization with modulo operator with power of 2
-    // equivalent to getCRC16(key) % 16384
+    // optimization with modulo operator with power of 2 equivalent to getCRC16(key) % 16384
     return getCRC16(key) & (16384 - 1);
   }
 
   public static int getSlot(byte[] key) {
+    if (key == null) {
+      throw new JedisClusterOperationException("Slot calculation of null is impossible");
+    }
+
     int s = -1;
     int e = -1;
     boolean sFound = false;

src/test/java/redis/clients/jedis/tests/JedisClusterTest.java

@@ -2,7 +2,6 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
@@ -376,16 +375,6 @@ public void testRedisClusterMaxRedirections() {
     jc.set("51", "foo");
   }
 
-  @Test
-  public void testRedisHashtag() {
-    assertEquals(JedisClusterCRC16.getSlot("{user1000}.following"),
-      JedisClusterCRC16.getSlot("{user1000}.followers"));
-    assertEquals(JedisClusterCRC16.getSlot("bar"), JedisClusterCRC16.getSlot("foo{bar}{zap}"));
-    assertNotEquals(JedisClusterCRC16.getSlot("bar"), JedisClusterCRC16.getSlot("foo{}{bar}"));
-    assertNotEquals(JedisClusterCRC16.getSlot(""), JedisClusterCRC16.getSlot("foo{}{bar}"));
-    assertEquals(JedisClusterCRC16.getSlot("{bar"), JedisClusterCRC16.getSlot("foo{{bar}}zap"));
-  }
-
   @Test
   public void testClusterForgetNode() throws InterruptedException {
     // at first, join node4 to cluster
@@ -642,6 +631,59 @@ public void testInvalidStartNodeNotAdded() {
     assertFalse(clusterNodes.containsKey(JedisClusterInfoCache.getNodeKey(invalidHost)));
   }
 
+  @Test
+  public void nullKeys() {
+    Set<HostAndPort> jedisClusterNode = new HashSet<HostAndPort>();
+    jedisClusterNode.add(new HostAndPort(nodeInfo1.getHost(), nodeInfo1.getPort()));
+    JedisCluster cluster = new JedisCluster(jedisClusterNode, DEFAULT_TIMEOUT, DEFAULT_TIMEOUT,
+        DEFAULT_REDIRECTIONS, "cluster", DEFAULT_CONFIG);
+
+    String foo = "foo";
+    byte[] bfoo = new byte[]{0x0b, 0x0f, 0x00, 0x00};
+
+    try {
+      cluster.exists((String) null);
+      fail();
+    } catch (JedisClusterOperationException coe) {
+      // expected
+    }
+
+    try {
+      cluster.exists(foo, null);
+      fail();
+    } catch (JedisClusterOperationException coe) {
+      // expected
+    }
+
+    try {
+      cluster.exists(null, foo);
+      fail();
+    } catch (JedisClusterOperationException coe) {
+      // expected
+    }
+
+    try {
+      cluster.exists((byte[]) null);
+      fail();
+    } catch (JedisClusterOperationException coe) {
+      // expected
+    }
+
+    try {
+      cluster.exists(bfoo, null);
+      fail();
+    } catch (JedisClusterOperationException coe) {
+      // expected
+    }
+
+    try {
+      cluster.exists(null, bfoo);
+      fail();
+    } catch (JedisClusterOperationException coe) {
+      // expected
+    }
+  }
+
   private static String getNodeServingSlotRange(String infoOutput) {
     // f4f3dc4befda352a4e0beccf29f5e8828438705d 127.0.0.1:7380 master - 0
     // 1394372400827 0 connected 5461-10922

src/test/java/redis/clients/jedis/tests/commands/ClusterBinaryJedisCommandsTest.java

@@ -2,7 +2,6 @@
 
 import static org.junit.Assert.assertArrayEquals;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotEquals;
 import static org.junit.Assert.assertTrue;
 
 import java.util.ArrayList;
@@ -180,20 +179,6 @@ public void failKeys() {
     jedisCluster.keys("*".getBytes());
   }
 
-  @Test
-  public void testGetSlot() {
-    assertEquals(JedisClusterCRC16.getSlot("{user1000}.following".getBytes()),
-      JedisClusterCRC16.getSlot("{user1000}.followers".getBytes()));
-    assertEquals(JedisClusterCRC16.getSlot("bar".getBytes()),
-        JedisClusterCRC16.getSlot("foo{bar}{zap}".getBytes()));
-    assertNotEquals(JedisClusterCRC16.getSlot("bar".getBytes()),
-        JedisClusterCRC16.getSlot("foo{}{bar}".getBytes()));
-    assertNotEquals(JedisClusterCRC16.getSlot(new byte[0]),
-        JedisClusterCRC16.getSlot("foo{}{bar}".getBytes()));
-    assertEquals(JedisClusterCRC16.getSlot("{bar".getBytes()),
-        JedisClusterCRC16.getSlot("foo{{bar}}zap".getBytes()));
-  }
-
   private static String getNodeId(String infoOutput) {
     for (String infoLine : infoOutput.split("\n")) {
       if (infoLine.contains("myself")) {

src/test/java/redis/clients/jedis/tests/utils/JedisClusterCRC16Test.java

@@ -1,6 +1,7 @@
 package redis.clients.jedis.tests.utils;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotEquals;
 
 import java.util.HashMap;
 import java.util.Map;
@@ -42,4 +43,25 @@ private Map<String, Integer> prepareSolutionSet() {
     solutionMap.put("Hello, World!", 0x4FD6);
     return solutionMap;
   }
+
+  @Test
+  public void testRedisHashtagGetSlot() {
+    assertEquals(JedisClusterCRC16.getSlot("{bar"), JedisClusterCRC16.getSlot("foo{{bar}}zap"));
+    assertEquals(JedisClusterCRC16.getSlot("{user1000}.following"),
+      JedisClusterCRC16.getSlot("{user1000}.followers"));
+    assertNotEquals(JedisClusterCRC16.getSlot("foo{}{bar}"), JedisClusterCRC16.getSlot("bar"));
+    assertEquals(JedisClusterCRC16.getSlot("foo{bar}{zap}"), JedisClusterCRC16.getSlot("bar"));
+  }
+
+  @Test
+  public void testBinaryHashtagGetSlot() {
+    assertEquals(JedisClusterCRC16.getSlot("{bar".getBytes()), JedisClusterCRC16.getSlot("{bar".getBytes()));
+    assertEquals(JedisClusterCRC16.getSlot("{user1000}.following".getBytes()),
+      JedisClusterCRC16.getSlot("{user1000}.followers".getBytes()));
+    assertNotEquals(JedisClusterCRC16.getSlot("foo{}{bar}".getBytes()),
+      JedisClusterCRC16.getSlot("bar".getBytes()));
+    assertEquals(JedisClusterCRC16.getSlot("foo{bar}{zap}".getBytes()),
+      JedisClusterCRC16.getSlot("bar".getBytes()));
+  }
+
 }