SPY-63 Keys can be anything over binary protocol

Change-Id: I8fcd92bac797f7bb610fd6bc42a8b78d27700785
Reviewed-on: http://review.couchbase.org/21323
Reviewed-by: Matt Ingenthron <[email protected]>
Tested-by: Matt Ingenthron <[email protected]>
Reviewed-by: Michael Nitschinger <[email protected]>

Author: apage43

src/main/java/net/spy/memcached/MemcachedClient.java

@@ -69,6 +69,7 @@
 import net.spy.memcached.ops.StoreOperation;
 import net.spy.memcached.ops.StoreType;
 import net.spy.memcached.ops.TimedOutOperationStatus;
+import net.spy.memcached.protocol.binary.BinaryOperationFactory;
 import net.spy.memcached.transcoders.TranscodeService;
 import net.spy.memcached.transcoders.Transcoder;
 import net.spy.memcached.util.StringUtils;
@@ -1061,7 +1062,7 @@ public <T> BulkFuture<Map<String, T>> asyncGetBulk(Iterator<String> keyIter,
     while (keyIter.hasNext() && tcIter.hasNext()) {
       String key = keyIter.next();
       tcMap.put(key, tcIter.next());
-      StringUtils.validateKey(key);
+      StringUtils.validateKey(key, opFact instanceof BinaryOperationFactory);
       final MemcachedNode primaryNode = locator.getPrimary(key);
       MemcachedNode node = null;
       if (primaryNode.isActive()) {

src/main/java/net/spy/memcached/MemcachedConnection.java

@@ -56,6 +56,7 @@
 import net.spy.memcached.ops.OperationState;
 import net.spy.memcached.ops.TapOperation;
 import net.spy.memcached.ops.VBucketAware;
+import net.spy.memcached.protocol.binary.BinaryOperationFactory;
 import net.spy.memcached.protocol.binary.TapAckOperationImpl;
 import net.spy.memcached.util.StringUtils;
 
@@ -636,7 +637,7 @@ public NodeLocator getLocator() {
   }
 
   public void enqueueOperation(String key, Operation o) {
-    StringUtils.validateKey(key);
+    StringUtils.validateKey(key, opFact instanceof BinaryOperationFactory);
     checkState();
     addOperation(key, o);
   }

src/main/java/net/spy/memcached/util/StringUtils.java

@@ -63,7 +63,7 @@ public static boolean isJsonObject(String s) {
     }
   }
 
-  public static void validateKey(String key) {
+  public static void validateKey(String key, boolean binary) {
     byte[] keyBytes = KeyUtil.getKeyBytes(key);
     if (keyBytes.length > MemcachedClientIF.MAX_KEY_LENGTH) {
       throw new IllegalArgumentException("Key is too long (maxlen = "
@@ -73,11 +73,13 @@ public static void validateKey(String key) {
       throw new IllegalArgumentException(
           "Key must contain at least one character.");
     }
-    // Validate the key
-    for (byte b : keyBytes) {
-      if (b == ' ' || b == '\n' || b == '\r' || b == 0) {
-        throw new IllegalArgumentException(
-            "Key contains invalid characters:  ``" + key + "''");
+    if(!binary) {
+      // Validate the key
+      for (byte b : keyBytes) {
+        if (b == ' ' || b == '\n' || b == '\r' || b == 0) {
+          throw new IllegalArgumentException(
+              "Key contains invalid characters:  ``" + key + "''");
+        }
       }
     }
   }

src/test/java/net/spy/memcached/BinaryClientTest.java

@@ -24,6 +24,7 @@
 package net.spy.memcached;
 
 import java.net.InetSocketAddress;
+import java.util.Map;
 import java.util.concurrent.ExecutionException;
 import net.spy.memcached.internal.OperationFuture;
 
@@ -103,6 +104,42 @@ public void testAsyncCASResponse() {
     assertNotNull(casRes.getCas());
   }
 
+  @Override
+  public void testKeyWithSpaces() throws Exception {
+    String key = "key with spaces";
+    client.set(key, 0, "");
+    assertNotNull("Couldn't get the key with spaces in it.", client.get(key));
+  }
+
+  @Override
+  public void testKeyWithNewline() throws Exception {
+    String key = "Key\n";
+    client.set(key, 0, "");
+    assertNotNull(client.get(key));
+  }
+
+  @Override
+  public void testKeyWithReturn() throws Exception {
+    String key = "Key\r";
+    client.set(key, 0, "");
+    assertNotNull(client.get(key));
+  }
+
+  @Override
+  public void testKeyWithASCIINull() throws Exception {
+    String key = "Key\0";
+    client.set(key, 0, "");
+    assertNotNull(client.get(key));
+  }
+
+  @Override
+  public void testGetBulkKeyWSpaces() throws Exception {
+    String key = "Key key2";
+    client.set(key, 0, "");
+    Map<String, Object> bulkReturn = client.getBulk(key);
+    assertTrue(bulkReturn.size() >= 1);
+  }
+
   @Override
   protected void syncGetTimeoutsInitClient() throws Exception {
     initClient(new BinaryConnectionFactory() {

src/test/java/net/spy/memcached/ProtocolBaseCase.java

@@ -178,7 +178,7 @@ public void testExtendedUTF8Key() throws Exception {
     assertEquals("test1value", client.get(key));
   }
 
-  public void testInvalidKey1() throws Exception {
+  public void testKeyWithSpaces() throws Exception {
     try {
       client.get("key with spaces");
       fail("Expected IllegalArgumentException getting key with spaces");
@@ -187,7 +187,7 @@ public void testInvalidKey1() throws Exception {
     }
   }
 
-  public void testInvalidKey2() throws Exception {
+  public void testKeyLongerThan250() throws Exception {
     try {
       StringBuilder longKey = new StringBuilder();
       for (int i = 0; i < 251; i++) {
@@ -200,7 +200,7 @@ public void testInvalidKey2() throws Exception {
     }
   }
 
-  public void testInvalidKey3() throws Exception {
+  public void testKeyWithNewline() throws Exception {
     try {
       Object val = client.get("Key\n");
       fail("Expected IllegalArgumentException, got " + val);
@@ -209,7 +209,7 @@ public void testInvalidKey3() throws Exception {
     }
   }
 
-  public void testInvalidKey4() throws Exception {
+  public void testKeyWithReturn() throws Exception {
     try {
       Object val = client.get("Key\r");
       fail("Expected IllegalArgumentException, got " + val);
@@ -218,7 +218,7 @@ public void testInvalidKey4() throws Exception {
     }
   }
 
-  public void testInvalidKey5() throws Exception {
+  public void testKeyWithASCIINull() throws Exception {
     try {
       Object val = client.get("Key\0");
       fail("Expected IllegalArgumentException, got " + val);
@@ -236,7 +236,7 @@ public void testInvalidKeyBlank() throws Exception {
     }
   }
 
-  public void testInvalidKeyBulk() throws Exception {
+  public void testGetBulkKeyWSpaces() throws Exception {
     try {
       Object val = client.getBulk("Key key2");
       fail("Expected IllegalArgumentException, got " + val);