From: Brian Aker <brian@gaz>
Date: Tue, 19 Jan 2010 00:39:46 +0000 (-0800)
Subject: We now check return key size for memcached_fetch() to make sure it is not
X-Git-Tag: 0.40~86
X-Git-Url: https://git.m6w6.name/?a=commitdiff_plain;h=7e592f0e6938506a8f9e228b40557c5bb8a10a0b;p=m6w6%2Flibmemcached

We now check return key size for memcached_fetch() to make sure it is not
too big (compared to original ascii server). result_st is now initialized
differently. This should make it a bit faster.
---

diff --git a/docs/memcached_get.pod b/docs/memcached_get.pod
index ced98f1c..5faa2db9 100644
--- a/docs/memcached_get.pod
+++ b/docs/memcached_get.pod
@@ -109,7 +109,9 @@ memcached_return_t pointer to hold any error. The object will be returned
 upon success and NULL will be returned on failure. MEMCACHD_END is returned
 by the *error value when all objects that have been found are returned.
 The final value upon MEMCACHED_END is null. Values returned by
-memcached_fetch() musted be free'ed by the caller.
+memcached_fetch() musted be free'ed by the caller. memcached_fetch() will
+be DEPRECATED in the near future, memcached_fetch_result() should be used
+instead.
 
 memcached_fetch_result() is used to return a memcached_result_st(3) structure
 from a memcached server. The result object is forward compatible with changes
@@ -154,6 +156,10 @@ All objects returned must be freed by the calling application.
 memcached_get() and memcached_fetch() will return NULL on error. You must
 look at the value of error to determine what the actual error was.
 
+MEMCACHED_KEY_TOO_BIG is set to error whenever memcached_fetch() was used
+and the key was set larger then MEMCACHED_MAX_KEY, which was the largest
+key allowed for the original memcached ascii server.
+
 =head1 HOME
 
 To find out more information please check:
diff --git a/libmemcached/constants.h b/libmemcached/constants.h
index 11b2f677..3446c9cc 100644
--- a/libmemcached/constants.h
+++ b/libmemcached/constants.h
@@ -69,6 +69,7 @@ typedef enum {
   MEMCACHED_UNKNOWN_STAT_KEY,
   MEMCACHED_E2BIG,
   MEMCACHED_INVALID_ARGUMENTS,
+  MEMCACHED_KEY_TOO_BIG,
   MEMCACHED_MAXIMUM_RETURN /* Always add new error code before */
 } memcached_return_t;
 
diff --git a/libmemcached/fetch.c b/libmemcached/fetch.c
index 7baac544..d76f19b2 100644
--- a/libmemcached/fetch.c
+++ b/libmemcached/fetch.c
@@ -26,12 +26,19 @@ char *memcached_fetch(memcached_st *ptr, char *key, size_t *key_length,
 
   if (key)
   {
-    strncpy(key, result_buffer->key, result_buffer->key_length);
+    if (result_buffer->key_length > MEMCACHED_MAX_KEY)
+    {
+      *error= MEMCACHED_KEY_TOO_BIG;
+      *value_length= 0;
+
+      return NULL;
+    }
+    strncpy(key, result_buffer->item_key, result_buffer->key_length); // For the binary protocol we will cut off the key :(
     *key_length= result_buffer->key_length;
   }
 
-  if (result_buffer->flags)
-    *flags= result_buffer->flags;
+  if (result_buffer->item_flags)
+    *flags= result_buffer->item_flags;
   else
     *flags= 0;
 
diff --git a/libmemcached/response.c b/libmemcached/response.c
index afaed844..cbb49784 100644
--- a/libmemcached/response.c
+++ b/libmemcached/response.c
@@ -110,7 +110,7 @@ static memcached_return_t textual_value_fetch(memcached_server_instance_st *ptr,
     char *key;
     size_t prefix_length;
 
-    key= result->key;
+    key= result->item_key;
     result->key_length= 0;
 
     for (prefix_length= ptr->root->prefix_key_length; !(iscntrl(*string_ptr) || isspace(*string_ptr)) ; string_ptr++)
@@ -124,7 +124,7 @@ static memcached_return_t textual_value_fetch(memcached_server_instance_st *ptr,
       else
         prefix_length--;
     }
-    result->key[result->key_length]= 0;
+    result->item_key[result->key_length]= 0;
   }
 
   if (end_ptr == string_ptr)
@@ -135,7 +135,7 @@ static memcached_return_t textual_value_fetch(memcached_server_instance_st *ptr,
   if (end_ptr == string_ptr)
     goto read_error;
   for (next_ptr= string_ptr; isdigit(*string_ptr); string_ptr++);
-  result->flags= (uint32_t) strtoul(next_ptr, &string_ptr, 10);
+  result->item_flags= (uint32_t) strtoul(next_ptr, &string_ptr, 10);
 
   if (end_ptr == string_ptr)
     goto read_error;
@@ -161,7 +161,7 @@ static memcached_return_t textual_value_fetch(memcached_server_instance_st *ptr,
   {
     string_ptr++;
     for (next_ptr= string_ptr; isdigit(*string_ptr); string_ptr++);
-    result->cas= strtoull(next_ptr, &string_ptr, 10);
+    result->item_cas= strtoull(next_ptr, &string_ptr, 10);
   }
 
   if (end_ptr < string_ptr)
@@ -364,17 +364,17 @@ static memcached_return_t binary_read_one_response(memcached_server_instance_st
       {
         uint16_t keylen= header.response.keylen;
         memcached_result_reset(result);
-        result->cas= header.response.cas;
+        result->item_cas= header.response.cas;
 
-        if (memcached_safe_read(ptr, &result->flags,
-                                sizeof (result->flags)) != MEMCACHED_SUCCESS)
+        if (memcached_safe_read(ptr, &result->item_flags,
+                                sizeof (result->item_flags)) != MEMCACHED_SUCCESS)
           return MEMCACHED_UNKNOWN_READ_FAILURE;
 
-        result->flags= ntohl(result->flags);
+        result->item_flags= ntohl(result->item_flags);
         bodylen -= header.response.extlen;
 
         result->key_length= keylen;
-        if (memcached_safe_read(ptr, result->key, keylen) != MEMCACHED_SUCCESS) 
+        if (memcached_safe_read(ptr, result->item_key, keylen) != MEMCACHED_SUCCESS) 
           return MEMCACHED_UNKNOWN_READ_FAILURE;
 
         bodylen -= keylen;
diff --git a/libmemcached/result.c b/libmemcached/result.c
index f151160e..d7c2fa58 100644
--- a/libmemcached/result.c
+++ b/libmemcached/result.c
@@ -17,7 +17,18 @@
 */
 #include "common.h"
 
-memcached_result_st *memcached_result_create(memcached_st *memc,
+static inline void _result_init(memcached_result_st *self,
+                                const memcached_st *memc)
+{
+  self->item_flags= 0;
+  self->item_expiration= 0;
+  self->key_length= 0;
+  self->item_cas= 0;
+  self->root= memc;
+  self->item_key[0]= 0;
+}
+
+memcached_result_st *memcached_result_create(const memcached_st *memc,
                                              memcached_result_st *ptr)
 {
   WATCHPOINT_ASSERT(memc);
@@ -26,19 +37,22 @@ memcached_result_st *memcached_result_create(memcached_st *memc,
   /* Saving malloc calls :) */
   if (ptr)
   {
-    memset(ptr, 0, sizeof(memcached_result_st));
+    ptr->options.is_allocated= false;
   }
   else
   {
-    ptr= libmemcached_calloc(memc, 1, sizeof(memcached_result_st));
+    ptr= libmemcached_malloc(memc, sizeof(memcached_result_st));
 
     if (ptr == NULL)
       return NULL;
+
     ptr->options.is_allocated= true;
   }
 
   ptr->options.is_initialized= true;
 
+  _result_init(ptr, memc);
+
   ptr->root= memc;
   memcached_string_create(memc, &ptr->value, 0);
   WATCHPOINT_ASSERT_INITIALIZED(&ptr->value);
@@ -51,9 +65,9 @@ void memcached_result_reset(memcached_result_st *ptr)
 {
   ptr->key_length= 0;
   memcached_string_reset(&ptr->value);
-  ptr->flags= 0;
-  ptr->cas= 0;
-  ptr->expiration= 0;
+  ptr->item_flags= 0;
+  ptr->item_cas= 0;
+  ptr->item_expiration= 0;
 }
 
 void memcached_result_free(memcached_result_st *ptr)
diff --git a/libmemcached/result.h b/libmemcached/result.h
index 3374c515..f01550e7 100644
--- a/libmemcached/result.h
+++ b/libmemcached/result.h
@@ -17,32 +17,34 @@ extern "C" {
 #endif
 
 struct memcached_result_st {
+  uint32_t item_flags;
+  time_t item_expiration;
+  size_t key_length;
+  uint64_t item_cas;
+  const memcached_st *root;
+  memcached_string_st value;
+  char item_key[MEMCACHED_MAX_KEY];
   struct {
     bool is_allocated:1;
     bool is_initialized:1;
   } options;
-  uint32_t flags;
-  time_t expiration;
-  memcached_st *root;
-  size_t key_length;
-  uint64_t cas;
-  memcached_string_st value;
-  char key[MEMCACHED_MAX_KEY];
   /* Add result callback function */
 };
 
 /* Result Struct */
 LIBMEMCACHED_API
 void memcached_result_free(memcached_result_st *result);
+
 LIBMEMCACHED_API
 void memcached_result_reset(memcached_result_st *ptr);
+
 LIBMEMCACHED_API
-memcached_result_st *memcached_result_create(memcached_st *ptr,
+memcached_result_st *memcached_result_create(const memcached_st *ptr,
                                              memcached_result_st *result);
 
 static inline const char *memcached_result_key_value(const memcached_result_st *self)
 {
-  return self->key;
+  return self->key_length ? self->item_key : NULL;
 }
 
 static inline size_t memcached_result_key_length(const memcached_result_st *self)
@@ -64,12 +66,12 @@ static inline size_t memcached_result_length(const memcached_result_st *self)
 
 static inline uint32_t memcached_result_flags(const memcached_result_st *self)
 {
-  return self->flags;
+  return self->item_flags;
 }
 
 static inline uint64_t memcached_result_cas(const memcached_result_st *self)
 {
-  return self->cas;
+  return self->item_cas;
 }
 
 static inline memcached_return_t memcached_result_set_value(memcached_result_st *ptr, const char *value, size_t length)
@@ -79,12 +81,12 @@ static inline memcached_return_t memcached_result_set_value(memcached_result_st
 
 static inline void memcached_result_set_flags(memcached_result_st *self, uint32_t flags)
 {
-  self->flags= flags;
+  self->item_flags= flags;
 }
 
 static inline void memcached_result_set_expiration(memcached_result_st *self, time_t expiration)
 {
-  self->expiration= expiration;
+  self->item_expiration= expiration;
 }
 
 #ifdef __cplusplus
diff --git a/libmemcached/stats.c b/libmemcached/stats.c
index a47b2820..6378d89e 100644
--- a/libmemcached/stats.c
+++ b/libmemcached/stats.c
@@ -444,12 +444,11 @@ char ** memcached_stat_get_keys(memcached_st *ptr,
   char **list;
   size_t length= sizeof(memcached_stat_keys);
 
-  list= libmemcached_malloc(memc_stat && memc_stat->root
-                            ? memc_stat->root
-                            : ptr,
-                            length);
+  (void)memc_stat;
 
-  if (!list)
+  list= libmemcached_malloc(ptr, length);
+
+  if (! list)
   {
     *error= MEMCACHED_MEMORY_ALLOCATION_FAILURE;
     return NULL;
diff --git a/libmemcached/strerror.c b/libmemcached/strerror.c
index f1d651c9..6698ff18 100644
--- a/libmemcached/strerror.c
+++ b/libmemcached/strerror.c
@@ -82,6 +82,8 @@ const char *memcached_strerror(memcached_st *ptr __attribute__((unused)), memcac
     return "ITEM TOO BIG";
   case MEMCACHED_INVALID_ARGUMENTS:
      return "INVALID ARGUMENTS";
+  case MEMCACHED_KEY_TOO_BIG:
+     return "KEY RETURNED FROM SERVER WAS TOO LARGE";
   case MEMCACHED_MAXIMUM_RETURN:
     return "Gibberish returned!";
   default:
diff --git a/libmemcached/string.c b/libmemcached/string.c
index 2b6f0820..464d55c0 100644
--- a/libmemcached/string.c
+++ b/libmemcached/string.c
@@ -49,7 +49,7 @@ static inline void _init_string(memcached_string_st *self)
   self->end= self->string= NULL;
 }
 
-memcached_string_st *memcached_string_create(memcached_st *memc, memcached_string_st *self, size_t initial_size)
+memcached_string_st *memcached_string_create(const memcached_st *memc, memcached_string_st *self, size_t initial_size)
 {
   memcached_return_t rc;
 
diff --git a/libmemcached/string.h b/libmemcached/string.h
index 4fc67cfc..188c6769 100644
--- a/libmemcached/string.h
+++ b/libmemcached/string.h
@@ -43,7 +43,7 @@ struct memcached_string_st {
 #define memcached_string_value(A) (A)->string
 
 LIBMEMCACHED_LOCAL
-memcached_string_st *memcached_string_create(memcached_st *ptr,
+memcached_string_st *memcached_string_create(const memcached_st *ptr,
                                              memcached_string_st *string,
                                              size_t initial_size);
 LIBMEMCACHED_LOCAL
diff --git a/tests/mem_functions.c b/tests/mem_functions.c
index 700003f3..14550dcb 100644
--- a/tests/mem_functions.c
+++ b/tests/mem_functions.c
@@ -332,16 +332,21 @@ static test_return_t error_test(memcached_st *memc)
                         4269430871U, 610793021U, 527273862U, 1437122909U,
                         2300930706U, 2943759320U, 674306647U, 2400528935U,
                         54481931U, 4186304426U, 1741088401U, 2979625118U,
-                        4159057246U, 3425930182U, 2593724503U};
+                        4159057246U, 3425930182U, 2593724503U,  1868899624U};
 
   // You have updated the memcache_error messages but not updated docs/tests.
-  test_true(MEMCACHED_MAXIMUM_RETURN == 39);
+  test_true(MEMCACHED_MAXIMUM_RETURN == 40);
   for (rc= MEMCACHED_SUCCESS; rc < MEMCACHED_MAXIMUM_RETURN; rc++)
   {
     uint32_t hash_val;
     const char *msg=  memcached_strerror(memc, rc);
     hash_val= memcached_generate_hash_value(msg, strlen(msg),
                                             MEMCACHED_HASH_JENKINS);
+    if (values[rc] != hash_val)
+    {
+      fprintf(stderr, "\n\nYou have updated memcached_return_t without updating the error_test\n");
+      fprintf(stderr, "%u, %s, (%u)\n\n", (uint32_t)rc, memcached_strerror(memc, rc), hash_val);
+    }
     test_true(values[rc] == hash_val);
   }
 
@@ -473,7 +478,7 @@ static test_return_t cas2_test(memcached_st *memc)
 
   results= memcached_fetch_result(memc, &results_obj, &rc);
   test_true(results);
-  test_true(results->cas);
+  test_true(results->item_cas);
   test_true(rc == MEMCACHED_SUCCESS);
   test_true(memcached_result_cas(results));