From: Trond Norbye Date: Sat, 10 Jan 2009 18:11:17 +0000 (+0100) Subject: Deprecate MEMCACHED_NO_KEY_PROVIDED, and fixed key validation tests for the binary... X-Git-Tag: 0.26~20 X-Git-Url: https://git.m6w6.name/?a=commitdiff_plain;h=5dc8e84eaf174fe510c2d253271215cd724be718;p=m6w6%2Flibmemcached Deprecate MEMCACHED_NO_KEY_PROVIDED, and fixed key validation tests for the binary protocol --- diff --git a/libmemcached/common.h b/libmemcached/common.h index ab61a22c..760fb042 100644 --- 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 b0bd05b0..9808ab65 100644 --- 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 67a00445..7f5bbdab 100644 --- 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 c1c1267c..84b6160c 100644 --- 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 dd02cf29..a7bf3a96 100644 --- 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 b8291ed7..738dff87 100644 --- 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 75e44d0c..b7dd8a94 100644 --- 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 87bc3ace..d489052c 100644 --- 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 770b27d4..1ac70d14 100644 --- 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 5164b828..4bc0be4e 100644 --- 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 */