From 7e592f0e6938506a8f9e228b40557c5bb8a10a0b Mon Sep 17 00:00:00 2001 From: Brian Aker Date: Mon, 18 Jan 2010 16:39:46 -0800 Subject: [PATCH] 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. --- docs/memcached_get.pod | 8 +++++++- libmemcached/constants.h | 1 + libmemcached/fetch.c | 13 ++++++++++--- libmemcached/response.c | 18 +++++++++--------- libmemcached/result.c | 26 ++++++++++++++++++++------ libmemcached/result.h | 28 +++++++++++++++------------- libmemcached/stats.c | 9 ++++----- libmemcached/strerror.c | 2 ++ libmemcached/string.c | 2 +- libmemcached/string.h | 2 +- tests/mem_functions.c | 11 ++++++++--- 11 files changed, 78 insertions(+), 42 deletions(-) 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)); -- 2.30.2