Deprecate MEMCACHED_NO_KEY_PROVIDED, and fixed key validation tests for the binary...

diff --git a/libmemcached/common.h b/libmemcached/common.h
index ab61a22c424a37db9263e08f11da6edbf8756ae9..760fb04241c5a3ac77a28eee0ba97fdc89bb2e65 100644 (file)
--- a/libmemcached/common.h
+++ b/libmemcached/common.h
@@ -120,4 +120,23 @@ extern uint64_t htonll(uint64_t);
 
 memcached_return memcached_purge(memcached_server_st *ptr);
 
+static inline memcached_return memcached_validate_key_length(size_t key_length, 
+                                                             bool binary) {
+  unlikely (key_length == 0)
+    return MEMCACHED_BAD_KEY_PROVIDED;
+  
+  if (binary)
+  {
+    unlikely (key_length > 0xffff)
+      return MEMCACHED_BAD_KEY_PROVIDED;
+  }
+  else
+  {
+    unlikely (key_length > 250) 
+      return MEMCACHED_BAD_KEY_PROVIDED;
+  }
+
+  return MEMCACHED_SUCCESS;
+}
+
 #endif /* __COMMON_H__ */

diff --git a/libmemcached/memcached_auto.c b/libmemcached/memcached_auto.c
index b0bd05b0df29617507049cd517590a48cca45f71..9808ab65e8e98f815f5f3218d4f2b91f76d1d652 100644 (file)
--- a/libmemcached/memcached_auto.c
+++ b/libmemcached/memcached_auto.c
@@ -11,9 +11,6 @@ static memcached_return memcached_auto(memcached_st *ptr,
   char buffer[MEMCACHED_DEFAULT_COMMAND_SIZE];
   unsigned int server_key;
 
-  unlikely (key_length == 0)
-    return MEMCACHED_NO_KEY_PROVIDED;
-
   unlikely (ptr->hosts == NULL || ptr->number_of_hosts == 0)
     return MEMCACHED_NO_SERVERS;
 
@@ -68,15 +65,9 @@ static memcached_return binary_incr_decr(memcached_st *ptr, uint8_t cmd,
 {
   unsigned int server_key;
 
-  unlikely (key_length == 0)
-    return MEMCACHED_NO_KEY_PROVIDED;
-
   unlikely (ptr->hosts == NULL || ptr->number_of_hosts == 0)
     return MEMCACHED_NO_SERVERS;
 
-  if ((ptr->flags & MEM_VERIFY_KEY) && (memcachd_key_test((char **)&key, &key_length, 1) == MEMCACHED_BAD_KEY_PROVIDED))
-    return MEMCACHED_BAD_KEY_PROVIDED;
-
   server_key= memcached_generate_hash(ptr, key, key_length);
 
   protocol_binary_request_incr request= {.bytes= {0}};
@@ -106,7 +97,9 @@ memcached_return memcached_increment(memcached_st *ptr,
                                      uint32_t offset,
                                      uint64_t *value)
 {
-  memcached_return rc;
+  memcached_return rc= memcached_validate_key_length(key_length, ptr->flags & MEM_BINARY_PROTOCOL);
+  unlikely (rc != MEMCACHED_SUCCESS)
+    return rc;
 
   LIBMEMCACHED_MEMCACHED_INCREMENT_START();
   if (ptr->flags & MEM_BINARY_PROTOCOL) 
@@ -125,7 +118,9 @@ memcached_return memcached_decrement(memcached_st *ptr,
                                      uint32_t offset,
                                      uint64_t *value)
 {
-  memcached_return rc;
+  memcached_return rc= memcached_validate_key_length(key_length, ptr->flags & MEM_BINARY_PROTOCOL);
+  unlikely (rc != MEMCACHED_SUCCESS)
+    return rc;
 
   LIBMEMCACHED_MEMCACHED_DECREMENT_START();
   if (ptr->flags & MEM_BINARY_PROTOCOL) 

diff --git a/libmemcached/memcached_constants.h b/libmemcached/memcached_constants.h
index 67a0044584f22d38c251027d3e7c359774adfcfe..7f5bbdabff66f4a690799c0a511a93da5ce5a319 100644 (file)
--- a/libmemcached/memcached_constants.h
+++ b/libmemcached/memcached_constants.h
@@ -56,7 +56,7 @@ typedef enum {
   MEMCACHED_ERRNO,
   MEMCACHED_FAIL_UNIX_SOCKET,
   MEMCACHED_NOT_SUPPORTED,
-  MEMCACHED_NO_KEY_PROVIDED,
+  MEMCACHED_NO_KEY_PROVIDED, /* Deprecated. Use MEMCACHED_BAD_KEY_PROVIDED! */
   MEMCACHED_FETCH_NOTFINISHED,
   MEMCACHED_TIMEOUT,
   MEMCACHED_BUFFERED,

diff --git a/libmemcached/memcached_delete.c b/libmemcached/memcached_delete.c
index c1c1267c9b8653ca0eae7700b529a416e320ac7e..84b6160c2fe00a4058aed9013df56fc981d2f741 100644 (file)
--- a/libmemcached/memcached_delete.c
+++ b/libmemcached/memcached_delete.c
@@ -26,8 +26,10 @@ memcached_return memcached_delete_by_key(memcached_st *ptr,
 
   LIBMEMCACHED_MEMCACHED_DELETE_START();
 
-  unlikely (key_length == 0)
-    return MEMCACHED_NO_KEY_PROVIDED;
+  rc= memcached_validate_key_length(key_length, 
+                                    ptr->flags & MEM_BINARY_PROTOCOL);
+  unlikely (rc != MEMCACHED_SUCCESS)
+    return rc;
 
   unlikely (ptr->hosts == NULL || ptr->number_of_hosts == 0)
     return MEMCACHED_NO_SERVERS;

diff --git a/libmemcached/memcached_get.c b/libmemcached/memcached_get.c
index dd02cf29c7151362fd87f3ea47a40fd3d7b11b2f..a7bf3a963b8f4b422f38595176321a85bf8bab58 100644 (file)
--- a/libmemcached/memcached_get.c
+++ b/libmemcached/memcached_get.c
@@ -281,6 +281,15 @@ static memcached_return binary_mget_by_key(memcached_st *ptr,
     else
       request.message.header.request.opcode= PROTOCOL_BINARY_CMD_GETKQ;
 
+    memcached_return vk;
+    vk= memcached_validate_key_length(key_length[x],
+                                      ptr->flags & MEM_BINARY_PROTOCOL);
+    unlikely (vk != MEMCACHED_SUCCESS) {
+      if (x > 0)
+        memcached_io_reset(ptr);
+      return vk;
+    }
+
     request.message.header.request.keylen= htons((uint16_t)key_length[x]);
     request.message.header.request.datatype= PROTOCOL_BINARY_RAW_BYTES;
     request.message.header.request.bodylen= htonl(key_length[x]);

diff --git a/libmemcached/memcached_key.c b/libmemcached/memcached_key.c
index b8291ed77b31b1f77c3b38e420093cbdde1243bd..738dff8787d4e96739d29e468be2f311e6da8b8a 100644 (file)
--- a/libmemcached/memcached_key.c
+++ b/libmemcached/memcached_key.c
@@ -4,13 +4,17 @@ memcached_return memcachd_key_test(char **keys, size_t *key_length,
                                    unsigned int number_of_keys)
 {
   uint32_t x;
+  memcached_return rc;
 
   for (x= 0; x < number_of_keys; x++)
   {
     size_t y;
 
-    if (*(key_length + x) == 0)
-        return MEMCACHED_BAD_KEY_PROVIDED;
+    rc= memcached_validate_key_length(*(key_length + x), false);
+    if (rc != MEMCACHED_SUCCESS)
+      return rc;
+    
+
 
     for (y= 0; y < *(key_length + x); y++)
     {

diff --git a/libmemcached/memcached_server.c b/libmemcached/memcached_server.c
index 75e44d0ca76450d62bd0cc8e2a93dd25aa129452..b7dd8a94700eb309b8d54dcbf385ea2b5ec8b722 100644 (file)
--- a/libmemcached/memcached_server.c
+++ b/libmemcached/memcached_server.c
@@ -110,11 +110,10 @@ memcached_server_st *memcached_server_by_key(memcached_st *ptr,  const char *key
 {
   uint32_t server_key;
 
-  unlikely (key_length == 0)
-  {
-    *error= MEMCACHED_NO_KEY_PROVIDED;
+  *error= memcached_validate_key_length(key_length, 
+                                        ptr->flags & MEM_BINARY_PROTOCOL);
+  unlikely (*error != MEMCACHED_SUCCESS)
     return NULL;
-  }
 
   unlikely (ptr->number_of_hosts == 0)
   {

diff --git a/libmemcached/memcached_stats.c b/libmemcached/memcached_stats.c
index 87bc3ace254455680d813f7640ee6cedf2d22ad7..d489052c6ecd5f2e551569bf5c672ab43d8172f3 100644 (file)
--- a/libmemcached/memcached_stats.c
+++ b/libmemcached/memcached_stats.c
@@ -227,6 +227,11 @@ static memcached_return binary_stats_fetch(memcached_st *ptr,
   if (args != NULL) 
   {
     int len= strlen(args);
+
+    rc= memcached_validate_key_length(len, true);
+    unlikely (rc != MEMCACHED_SUCCESS)
+      return rc;
+
     request.message.header.request.keylen= htons((uint16_t)len);
     request.message.header.request.bodylen= htonl(len);
       

diff --git a/libmemcached/memcached_storage.c b/libmemcached/memcached_storage.c
index 770b27d4006a2a46137f89501b642750bce4643e..1ac70d146c9971c839fa6e0a59694dee81937686 100644 (file)
--- a/libmemcached/memcached_storage.c
+++ b/libmemcached/memcached_storage.c
@@ -70,9 +70,10 @@ static inline memcached_return memcached_send(memcached_st *ptr,
 
   WATCHPOINT_ASSERT(!(value == NULL && value_length > 0));
 
-  unlikely (key_length == 0)
-    return MEMCACHED_NO_KEY_PROVIDED;
-
+  rc= memcached_validate_key_length(key_length, ptr->flags & MEM_BINARY_PROTOCOL);
+  unlikely (rc != MEMCACHED_SUCCESS)
+    return rc;
+  
   unlikely (ptr->number_of_hosts == 0)
     return MEMCACHED_NO_SERVERS;
 

diff --git a/tests/function.c b/tests/function.c
index 5164b828270295c7a723f108697e264eb3354fac..4bc0be4ef9392c7c0f0e2531bd7a417541a5ec77 100644 (file)
--- a/tests/function.c
+++ b/tests/function.c
@@ -603,6 +603,7 @@ static test_return  bad_key_test(memcached_st *memc)
   uint32_t flags;
   memcached_st *clone;
   unsigned int set= 1;
+  size_t max_keylen= 0xffff;
 
   clone= memcached_clone(NULL, memc);
   assert(clone);
@@ -610,23 +611,25 @@ static test_return  bad_key_test(memcached_st *memc)
   rc= memcached_behavior_set(clone, MEMCACHED_BEHAVIOR_VERIFY_KEY, set);
   assert(rc == MEMCACHED_SUCCESS);
 
-  string= memcached_get(clone, key, strlen(key),
-                        &string_length, &flags, &rc);
-  assert(rc == MEMCACHED_BAD_KEY_PROVIDED);
-  assert(string_length ==  0);
-  assert(!string);
+  /* All keys are valid in the binary protocol (except for length) */
+  if (memcached_behavior_get(clone, MEMCACHED_BEHAVIOR_BINARY_PROTOCOL) == 0) 
+  {
+    string= memcached_get(clone, key, strlen(key),
+                          &string_length, &flags, &rc);
+    assert(rc == MEMCACHED_BAD_KEY_PROVIDED);
+    assert(string_length ==  0);
+    assert(!string);
 
-  set= 0;
-  rc= memcached_behavior_set(clone, MEMCACHED_BEHAVIOR_VERIFY_KEY, set);
-  assert(rc == MEMCACHED_SUCCESS);
-  string= memcached_get(clone, key, strlen(key),
-                        &string_length, &flags, &rc);
-  assert(rc == MEMCACHED_NOTFOUND);
-  assert(string_length ==  0);
-  assert(!string);
+    set= 0;
+    rc= memcached_behavior_set(clone, MEMCACHED_BEHAVIOR_VERIFY_KEY, set);
+    assert(rc == MEMCACHED_SUCCESS);
+    string= memcached_get(clone, key, strlen(key),
+                          &string_length, &flags, &rc);
+    assert(rc == MEMCACHED_NOTFOUND);
+    assert(string_length ==  0);
+    assert(!string);
 
-  /* Test multi key for bad keys */
-  {
+    /* Test multi key for bad keys */
     char *keys[] = { "GoodKey", "Bad Key", "NotMine" };
     size_t key_lengths[] = { 7, 7, 7 };
     set= 1;
@@ -638,6 +641,34 @@ static test_return  bad_key_test(memcached_st *memc)
 
     rc= memcached_mget_by_key(clone, "foo daddy", 9, keys, key_lengths, 1);
     assert(rc == MEMCACHED_BAD_KEY_PROVIDED);
+
+    max_keylen= 250;
+
+    /* The following test should be moved to the end of this function when the
+       memcached server is updated to allow max size length of the keys in the
+       binary protocol
+    */
+    rc= memcached_callback_set(clone, MEMCACHED_CALLBACK_PREFIX_KEY, NULL);
+    assert(rc == MEMCACHED_SUCCESS);
+
+    char *longkey= malloc(max_keylen + 1);
+    if (longkey != NULL) 
+    {
+      memset(longkey, 'a', max_keylen + 1);
+      string= memcached_get(clone, longkey, max_keylen,
+                            &string_length, &flags, &rc);
+      assert(rc == MEMCACHED_NOTFOUND);
+      assert(string_length ==  0);
+      assert(!string);
+
+      string= memcached_get(clone, longkey, max_keylen + 1,
+                            &string_length, &flags, &rc);
+      assert(rc == MEMCACHED_BAD_KEY_PROVIDED);
+      assert(string_length ==  0);
+      assert(!string);
+
+      free(longkey);
+    }
   }
 
   /* Make sure zero length keys are marked as bad */