Fix for case where key may not be checked.

10 files changed, 198 insertions, 167 deletions

diff --git a/libmemcached/connect.cc b/libmemcached/connect.cc
index b19ba716..b60bbd8c 100644
--- a/libmemcached/connect.cc
+++ b/libmemcached/connect.cc
@@ -76,7 +76,7 @@ static memcached_return_t connect_poll(memcached_instance_st* server, const int
   if (server->root->poll_timeout == 0)
   {
     return memcached_set_error(*server, MEMCACHED_TIMEOUT, MEMCACHED_AT,
-                               memcached_literal_param("The time to wait for a connection to be established was set to zero, which means it will always timeout (MEMCACHED_TIMEOUT)."));
+                               memcached_literal_param("The time to wait for a connection to be established was set to zero which produces a timeout to every call to poll()."));
   }
 
   while (--loop_max) // Should only loop on cases of ERESTART or EINTR
diff --git a/libmemcached/error.cc b/libmemcached/error.cc
index 266f78b5..2f16cbce 100644
--- a/libmemcached/error.cc
+++ b/libmemcached/error.cc
@@ -36,6 +36,9 @@
  */
 
 #include <libmemcached/common.h>
+
+#include "libmemcached/assert.hpp"
+
 #include <cerrno>
 #include <cstdarg>
 #include <cstdio>
@@ -87,111 +90,119 @@ static void _set(Memcached& memc, memcached_string_t *str, memcached_return_t &r
     memcached_error_free(memc);
   }
 
-  // For memory allocation we use our error since it is a bit more specific
-  if (local_errno == ENOMEM and rc == MEMCACHED_ERRNO)
+  if (memcached_fatal(rc))
   {
-    rc= MEMCACHED_MEMORY_ALLOCATION_FAILURE;
-  }
+    // For memory allocation we use our error since it is a bit more specific
+    if (local_errno == ENOMEM and rc == MEMCACHED_ERRNO)
+    {
+      rc= MEMCACHED_MEMORY_ALLOCATION_FAILURE;
+    }
 
-  if (rc == MEMCACHED_MEMORY_ALLOCATION_FAILURE)
-  {
-    local_errno= ENOMEM;
-  }
+    if (rc == MEMCACHED_MEMORY_ALLOCATION_FAILURE)
+    {
+      local_errno= ENOMEM;
+    }
 
-  if (rc == MEMCACHED_ERRNO and not local_errno)
-  {
-    local_errno= errno;
-    rc= MEMCACHED_ERRNO;
-  }
+    if (rc == MEMCACHED_ERRNO and not local_errno)
+    {
+      local_errno= errno;
+      rc= MEMCACHED_ERRNO;
+    }
 
-  if (rc == MEMCACHED_ERRNO and local_errno == ENOTCONN)
-  {
-    rc= MEMCACHED_CONNECTION_FAILURE;
-  }
+    if (rc == MEMCACHED_ERRNO and local_errno == ENOTCONN)
+    {
+      rc= MEMCACHED_CONNECTION_FAILURE;
+    }
 
-  if (rc == MEMCACHED_ERRNO and local_errno == ECONNRESET)
-  {
-    rc= MEMCACHED_CONNECTION_FAILURE;
-  }
+    if (rc == MEMCACHED_ERRNO and local_errno == ECONNRESET)
+    {
+      rc= MEMCACHED_CONNECTION_FAILURE;
+    }
 
-  if (local_errno == EINVAL)
-  {
-    rc= MEMCACHED_INVALID_ARGUMENTS;
-  }
+    if (local_errno == EINVAL)
+    {
+      rc= MEMCACHED_INVALID_ARGUMENTS;
+    }
 
-  if (local_errno == ECONNREFUSED)
-  {
-    rc= MEMCACHED_CONNECTION_FAILURE;
-  }
+    if (local_errno == ECONNREFUSED)
+    {
+      rc= MEMCACHED_CONNECTION_FAILURE;
+    }
 
-  memcached_error_t *error= libmemcached_xmalloc(&memc, memcached_error_t);
-  if (error == NULL) // Bad business if this happens
-  {
-    return;
-  }
+    if (rc == MEMCACHED_TIMEOUT)
+    {
+    }
 
-  error->root= &memc;
-  error->query_id= memc.query_id;
-  error->rc= rc;
-  error->local_errno= local_errno;
+    memcached_error_t *error= libmemcached_xmalloc(&memc, memcached_error_t);
+    if (error == NULL) // Bad business if this happens
+    {
+      assert_msg(error, "libmemcached_xmalloc() failed to allocate a memcached_error_t");
+      return;
+    }
 
-  const char *errmsg_ptr;
-  char errmsg[MAX_ERROR_LENGTH];
-  errmsg[0]= 0;
-  errmsg_ptr= errmsg;
+    error->root= &memc;
+    error->query_id= memc.query_id;
+    error->rc= rc;
+    error->local_errno= local_errno;
 
-  if (local_errno)
-  {
+    const char *errmsg_ptr;
+    char errmsg[MAX_ERROR_LENGTH];
+    errmsg[0]= 0;
+    errmsg_ptr= errmsg;
+
+    if (local_errno)
+    {
 #if defined(STRERROR_R_CHAR_P) && STRERROR_R_CHAR_P
-    errmsg_ptr= strerror_r(local_errno, errmsg, sizeof(errmsg));
+      errmsg_ptr= strerror_r(local_errno, errmsg, sizeof(errmsg));
 #elif defined(HAVE_STRERROR_R) && HAVE_STRERROR_R
-    strerror_r(local_errno, errmsg, sizeof(errmsg));
-    errmsg_ptr= errmsg;
+      strerror_r(local_errno, errmsg, sizeof(errmsg));
+      errmsg_ptr= errmsg;
 #elif defined(HAVE_STRERROR) && HAVE_STRERROR
-    snprintf(errmsg, sizeof(errmsg), "%s", strerror(local_errno));
-    errmsg_ptr= errmsg;
+      snprintf(errmsg, sizeof(errmsg), "%s", strerror(local_errno));
+      errmsg_ptr= errmsg;
 #endif
-  }
+    }
 
 
-  if (str and str->size and local_errno)
-  {
-    error->size= (int)snprintf(error->message, MAX_ERROR_LENGTH, "(%p) %s(%s), %.*s -> %s", 
-                               error->root,
-                               memcached_strerror(&memc, rc), 
-                               errmsg_ptr,
-                               memcached_string_printf(*str), at);
-  }
-  else if (local_errno)
-  {
-    error->size= (int)snprintf(error->message, MAX_ERROR_LENGTH, "(%p) %s(%s) -> %s", 
-                               error->root,
-                               memcached_strerror(&memc, rc), 
-                               errmsg_ptr,
-                               at);
-  }
-  else if (rc == MEMCACHED_PARSE_ERROR and str and str->size)
-  {
-    error->size= (int)snprintf(error->message, MAX_ERROR_LENGTH, "(%p) %.*s -> %s", 
-                               error->root,
-                               int(str->size), str->c_str, at);
-  }
-  else if (str and str->size)
-  {
-    error->size= (int)snprintf(error->message, MAX_ERROR_LENGTH, "(%p) %s, %.*s -> %s", 
-                               error->root,
-                               memcached_strerror(&memc, rc), 
-                               int(str->size), str->c_str, at);
-  }
-  else
-  {
-    error->size= (int)snprintf(error->message, MAX_ERROR_LENGTH, "(%p) %s -> %s", 
-                               error->root,
-                               memcached_strerror(&memc, rc), at);
-  }
+    if (str and str->size and local_errno)
+    {
+      error->size= (int)snprintf(error->message, MAX_ERROR_LENGTH, "(%p) %s(%s), %.*s -> %s", 
+                                 error->root,
+                                 memcached_strerror(&memc, rc), 
+                                 errmsg_ptr,
+                                 memcached_string_printf(*str), at);
+    }
+    else if (local_errno)
+    {
+      error->size= (int)snprintf(error->message, MAX_ERROR_LENGTH, "(%p) %s(%s) -> %s", 
+                                 error->root,
+                                 memcached_strerror(&memc, rc), 
+                                 errmsg_ptr,
+                                 at);
+    }
+    else if (rc == MEMCACHED_PARSE_ERROR and str and str->size)
+    {
+      error->size= (int)snprintf(error->message, MAX_ERROR_LENGTH, "(%p) %.*s -> %s", 
+                                 error->root,
+                                 int(str->size), str->c_str, at);
+    }
+    else if (str and str->size)
+    {
+      error->size= (int)snprintf(error->message, MAX_ERROR_LENGTH, "(%p) %s, %.*s -> %s", 
+                                 error->root,
+                                 memcached_strerror(&memc, rc), 
+                                 int(str->size), str->c_str, at);
+    }
+    else
+    {
+      error->size= (int)snprintf(error->message, MAX_ERROR_LENGTH, "(%p) %s -> %s", 
+                                 error->root,
+                                 memcached_strerror(&memc, rc), at);
+    }
 
-  error->next= memc.error_messages;
-  memc.error_messages= error;
+    error->next= memc.error_messages;
+    memc.error_messages= error;
+  }
 
 #if 0
   if (error_log_fd == -1)
@@ -231,13 +242,11 @@ memcached_return_t memcached_set_error(memcached_instance_st& self, memcached_re
 memcached_return_t memcached_set_error(Memcached& memc, memcached_return_t rc, const char *at, memcached_string_t& str)
 {
   assert_msg(rc != MEMCACHED_ERRNO, "Programmer error, MEMCACHED_ERRNO was set to be returned to client");
-  if (memcached_fatal(rc) == false)
+  if (memcached_fatal(rc))
   {
-    return rc;
+    _set(memc, &str, rc, at);
   }
 
-  _set(memc, &str, rc, at);
-
   return rc;
 }
 
@@ -299,17 +308,15 @@ memcached_return_t memcached_set_error(memcached_instance_st& self, memcached_re
   memcached_string_t error_host= { hostname_port_message, size_t(size) };
 
   assert_msg(self.root, "Programmer error, root was not set on instance");
-  if (self.root == NULL)
+  if (self.root)
   {
-    return rc;
+    _set(*self.root, &error_host, rc, at);
+    _set(self, (*self.root));
+    assert(self.root->error_messages);
+    assert(self.error_messages);
+    assert(self.error_messages->rc == self.root->error_messages->rc);
   }
 
-  _set(*self.root, &error_host, rc, at);
-  _set(self, (*self.root));
-  assert(self.root->error_messages);
-  assert(self.error_messages);
-  assert(self.error_messages->rc == self.root->error_messages->rc);
-
   return rc;
 }
 
@@ -326,14 +333,12 @@ memcached_return_t memcached_set_error(memcached_instance_st& self, memcached_re
 
   memcached_string_t error_host= { hostname_port, size};
 
-  if (self.root == NULL)
+  if (self.root)
   {
-    return rc;
+    _set(*self.root, &error_host, rc, at);
+    _set(self, *self.root);
   }
 
-  _set(*self.root, &error_host, rc, at);
-  _set(self, *self.root);
-
   return rc;
 }
 
@@ -528,7 +533,7 @@ const char *memcached_last_error_message(const memcached_st *shell)
   {
     if (memc->error_messages)
     {
-      if (memc->error_messages->size == 0)
+      if (memc->error_messages->size and memc->error_messages->message[0])
       {
         return memc->error_messages->message;
       }
diff --git a/libmemcached/get.cc b/libmemcached/get.cc
index 1f7e775c..86d8cf2e 100644
--- a/libmemcached/get.cc
+++ b/libmemcached/get.cc
@@ -114,7 +114,6 @@ char *memcached_get_by_key(memcached_st *shell,
   {
     *error= MEMCACHED_NOTFOUND;
   }
-
   if (value == NULL)
   {
     if (ptr->get_key_failure and *error == MEMCACHED_NOTFOUND)
@@ -221,12 +220,14 @@ static memcached_return_t __mget_by_key_real(memcached_st *ptr,
 
   if (number_of_keys == 0)
   {
-    return memcached_set_error(*ptr, MEMCACHED_NOTFOUND, MEMCACHED_AT, memcached_literal_param("number_of_keys was zero"));
+    return memcached_set_error(*ptr, MEMCACHED_INVALID_ARGUMENTS, MEMCACHED_AT, memcached_literal_param("Numbers of keys provided was zero"));
   }
 
-  if (memcached_failed(memcached_key_test(*ptr, keys, key_length, number_of_keys)))
+  if (memcached_failed((rc= memcached_key_test(*ptr, keys, key_length, number_of_keys))))
   {
-    return memcached_last_error(ptr);
+    assert(memcached_last_error(ptr) == rc);
+
+    return rc;
   }
 
   bool is_group_key_set= false;
@@ -463,6 +464,11 @@ static memcached_return_t simple_binary_mget(memcached_st *ptr,
 
   bool flush= (number_of_keys == 1);
 
+  if (memcached_failed(rc= memcached_key_test(*ptr, keys, key_length, number_of_keys)))
+  {
+    return rc;
+  }
+
   /*
     If a server fails we warn about errors and start all over with sending keys
     to the server.
@@ -502,10 +508,13 @@ static memcached_return_t simple_binary_mget(memcached_st *ptr,
       request.message.header.request.opcode= PROTOCOL_BINARY_CMD_GETK;
     }
 
+#if 0
     {
       memcached_return_t vk= memcached_validate_key_length(key_length[x], ptr->flags.binary_protocol);
-      if (vk != MEMCACHED_SUCCESS)
+      if (memcached_failed(rc= memcached_key_test(*memc, (const char **)&key, &key_length, 1)))
       {
+        memcached_set_error(ptr, vk, MEMCACHED_AT, memcached_literal_param("Key was too long."));
+
         if (x > 0)
         {
           memcached_io_reset(instance);
@@ -514,6 +523,7 @@ static memcached_return_t simple_binary_mget(memcached_st *ptr,
         return vk;
       }
     }
+#endif
 
     request.message.header.request.keylen= htons((uint16_t)(key_length[x] + memcached_array_size(ptr->_namespace)));
     request.message.header.request.datatype= PROTOCOL_BINARY_RAW_BYTES;
diff --git a/libmemcached/io.cc b/libmemcached/io.cc
index 448cc325..ba4b3474 100644
--- a/libmemcached/io.cc
+++ b/libmemcached/io.cc
@@ -412,6 +412,11 @@ memcached_return_t memcached_io_wait_for_write(memcached_instance_st* instance)
   return io_wait(instance, MEM_WRITE);
 }
 
+memcached_return_t memcached_io_wait_for_read(memcached_instance_st* instance)
+{
+  return io_wait(instance, MEM_READ);
+}
+
 static memcached_return_t _io_fill(memcached_instance_st* instance)
 {
   ssize_t data_read;
diff --git a/libmemcached/io.hpp b/libmemcached/io.hpp
index f3fed345..5480239f 100644
--- a/libmemcached/io.hpp
+++ b/libmemcached/io.hpp
@@ -49,7 +49,8 @@ bool memcached_io_writev(memcached_instance_st* ptr,
                          libmemcached_io_vector_st vector[],
                          const size_t number_of, const bool with_flush);
 
-memcached_return_t memcached_io_wait_for_write(memcached_instance_st* ptr);
+memcached_return_t memcached_io_wait_for_write(memcached_instance_st*);
+memcached_return_t memcached_io_wait_for_read(memcached_instance_st*);
 
 void memcached_io_reset(memcached_instance_st* ptr);
 
diff --git a/libmemcached/key.cc b/libmemcached/key.cc
index 19d1f957..cec0eece 100644
--- a/libmemcached/key.cc
+++ b/libmemcached/key.cc
@@ -37,47 +37,71 @@
 
 #include <libmemcached/common.h>
 
+static inline memcached_return_t memcached_validate_key_length(size_t key_length, bool)
+{
+  if (key_length == 0)
+  {
+    return MEMCACHED_BAD_KEY_PROVIDED;
+  }
+
+  // No one ever reimplemented MEMCACHED to use keys longer then the original ascii length
+#if 0
+  if (binary)
+  {
+    if (key_length > 0xffff)
+    {
+      return MEMCACHED_BAD_KEY_PROVIDED;
+    }
+  }
+  else
+#endif
+  {
+    if (key_length >= MEMCACHED_MAX_KEY)
+    {
+      return MEMCACHED_BAD_KEY_PROVIDED;
+    }
+  }
+
+  return MEMCACHED_SUCCESS;
+}
+
 memcached_return_t memcached_key_test(memcached_st &memc,
                                       const char * const *keys,
                                       const size_t *key_length,
                                       size_t number_of_keys)
 {
+  if (number_of_keys == 0)
+  {
+    return memcached_set_error(memc, MEMCACHED_INVALID_ARGUMENTS, MEMCACHED_AT, memcached_literal_param("Numbers of keys provided was zero"));
+  }
+
   if (keys == NULL or key_length == NULL)
   {
     return memcached_set_error(memc, MEMCACHED_BAD_KEY_PROVIDED, MEMCACHED_AT, memcached_literal_param("Key was NULL or length of key was zero."));
   }
 
+  const bool is_binary= memcached_flag(memc, MEMCACHED_FLAG_BINARY_PROTOCOL);
+
   // If we don't need to verify the key, or we are using the binary protoocol,
   // we just check the size of the key
-  if (memc.flags.verify_key == false or memc.flags.binary_protocol == true)
-  {
-    for (size_t x= 0; x < number_of_keys; x++)
-    {
-      // We should set binary key, but the memcached server is broken for
-      // longer keys at the moment.
-      memcached_return_t rc= memcached_validate_key_length(*(key_length +x), false /* memc.flags.binary_protocol */);
-      if (memcached_failed(rc))
-      {
-        return memcached_set_error(memc, rc, MEMCACHED_AT, memcached_literal_param("Key provided was too long."));
-      }
-    }
-
-    return MEMCACHED_SUCCESS;
-  }
-
-  for (size_t x= 0; x < number_of_keys; x++)
+  for (size_t x= 0; x < number_of_keys; ++x)
   {
-    memcached_return_t rc= memcached_validate_key_length(*(key_length + x), false);
+    // We should set binary key, but the memcached server is broken for
+    // longer keys at the moment.
+    memcached_return_t rc= memcached_validate_key_length(*(key_length +x), false /* memc.flags.binary_protocol */);
     if (memcached_failed(rc))
     {
       return memcached_set_error(memc, rc, MEMCACHED_AT, memcached_literal_param("Key provided was too long."));
     }
- 
-    for (size_t y= 0; y < *(key_length + x); y++)
+
+    if (memc.flags.verify_key and is_binary == false)
     {
-      if ((isgraph(keys[x][y])) == 0)
+      for (size_t y= 0; y < *(key_length +x); ++y)
       {
-        return memcached_set_error(memc, MEMCACHED_BAD_KEY_PROVIDED, MEMCACHED_AT, memcached_literal_param("Key provided had invalid character."));
+        if ((isgraph(keys[x][y])) == 0)
+        {
+          return memcached_set_error(memc, MEMCACHED_BAD_KEY_PROVIDED, MEMCACHED_AT, memcached_literal_param("Key provided had invalid character."));
+        }
       }
     }
   }
diff --git a/libmemcached/key.hpp b/libmemcached/key.hpp
index ddd557d8..811a4b7b 100644
--- a/libmemcached/key.hpp
+++ b/libmemcached/key.hpp
@@ -41,27 +41,3 @@ memcached_return_t memcached_key_test(memcached_st& memc,
                                       const size_t *key_length,
                                       size_t number_of_keys);
 
-static inline memcached_return_t memcached_validate_key_length(size_t key_length, bool binary)
-{
-  if (key_length == 0)
-  {
-    return MEMCACHED_BAD_KEY_PROVIDED;
-  }
-
-  if (binary)
-  {
-    if (key_length > 0xffff)
-    {
-      return MEMCACHED_BAD_KEY_PROVIDED;
-    }
-  }
-  else
-  {
-    if (key_length >= MEMCACHED_MAX_KEY)
-    {
-      return MEMCACHED_BAD_KEY_PROVIDED;
-    }
-  }
-
-  return MEMCACHED_SUCCESS;
-}
diff --git a/libmemcached/response.cc b/libmemcached/response.cc
index c01d5089..ad3c0785 100644
--- a/libmemcached/response.cc
+++ b/libmemcached/response.cc
@@ -500,6 +500,8 @@ static memcached_return_t binary_read_one_response(memcached_instance_st* instan
   memcached_return_t rc;
   protocol_binary_response_header header;
 
+  assert(memcached_is_binary(instance->root));
+
   if ((rc= memcached_safe_read(instance, &header.bytes, sizeof(header.bytes))) != MEMCACHED_SUCCESS)
   {
     WATCHPOINT_ERROR(rc);
@@ -892,12 +894,19 @@ memcached_return_t memcached_response(memcached_instance_st* instance,
     return memcached_set_error(*instance, MEMCACHED_NOT_SUPPORTED, MEMCACHED_AT);
   }
 
-  /* We may have old commands in the buffer not set, first purge */
+  /* We may have old commands in the buffer not sent, first purge */
   if ((instance->root->flags.no_block) and (memcached_is_processing_input(instance->root) == false))
   {
     (void)memcached_io_write(instance);
   }
 
+  /*  Before going into loop wait to see if we have any IO waiting for us */
+  if (0)
+  {
+    memcached_return_t read_rc= memcached_io_wait_for_read(instance);
+    fprintf(stderr, "%s:%d: %s\n", __FILE__, __LINE__, memcached_strerror(NULL, read_rc));
+  }
+
   /*
    * The previous implementation purged all pending requests and just
    * returned the last one. Purge all pending messages to ensure backwards
diff --git a/libmemcached/stats.cc b/libmemcached/stats.cc
index d70972ef..180b0d98 100644
--- a/libmemcached/stats.cc
+++ b/libmemcached/stats.cc
@@ -346,7 +346,7 @@ static memcached_return_t set_data(memcached_stat_st *memc_stat, const char *key
   return MEMCACHED_SUCCESS;
 }
 
-char *memcached_stat_get_value(const memcached_st *, memcached_stat_st *memc_stat,
+char *memcached_stat_get_value(const memcached_st* shell, memcached_stat_st *memc_stat,
                                const char *key, memcached_return_t *error)
 {
   memcached_return_t not_used;
@@ -456,13 +456,15 @@ char *memcached_stat_get_value(const memcached_st *, memcached_stat_st *memc_sta
   }
   else
   {
-    *error= MEMCACHED_NOTFOUND;
+    Memcached* memc= (Memcached*)memcached2Memcached(shell);
+    *error= memcached_set_error(*memc, MEMCACHED_INVALID_ARGUMENTS, MEMCACHED_AT, memcached_literal_param("Invalid key provided"));
     return NULL;
   }
 
   if (length >= SMALL_STRING_LEN || length < 0)
   {
-    *error= MEMCACHED_FAILURE;
+    Memcached* memc= (Memcached*)memcached2Memcached(shell);
+    *error= memcached_set_error(*memc, MEMCACHED_FAILURE, MEMCACHED_AT, memcached_literal_param("Internal failure occured with buffer, please report this bug."));
     return NULL;
   }
 
@@ -658,8 +660,7 @@ memcached_stat_st *memcached_stat(memcached_st *shell, char *args, memcached_ret
   if (args)
   {
     args_length= strlen(args);
-    rc= memcached_validate_key_length(args_length, self->flags.binary_protocol);
-    if (memcached_failed(rc))
+    if (memcached_failed(rc= memcached_key_test(*self, (const char **)&args, &args_length, 1)))
     {
       *error= memcached_set_error(*self, rc, MEMCACHED_AT);
       return NULL;
@@ -746,7 +747,7 @@ memcached_return_t memcached_stat_servername(memcached_stat_st *memc_stat, char
     if (args)
     {
       args_length= strlen(args);
-      rc= memcached_validate_key_length(args_length, memc.flags.binary_protocol);
+      rc= memcached_key_test(*memc_ptr, (const char **)&args, &args_length, 1);
     }
 
     if (memcached_success(rc))
diff --git a/libmemcached/touch.cc b/libmemcached/touch.cc
index 1f793c55..91d0825f 100644
--- a/libmemcached/touch.cc
+++ b/libmemcached/touch.cc
@@ -124,9 +124,9 @@ memcached_return_t memcached_touch_by_key(memcached_st *shell,
     return rc;
   }
 
-  if (memcached_failed(rc= memcached_validate_key_length(key_length, ptr->flags.binary_protocol)))
+  if (memcached_failed(rc= memcached_key_test(*ptr, (const char **)&key, &key_length, 1)))
   {
-    return rc;
+    return memcached_set_error(*ptr, rc, MEMCACHED_AT);
   }
 
   uint32_t server_key= memcached_generate_hash_with_redistribution(ptr, group_key, group_key_length);