delete_multi breaks if one of the key parts is empty string

Bug #832131 reported by Pete-alex-harris on 2011-08-23
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Python Memcached
Undecided
Sean Reifschneider

Bug Description

In the Client class of memcache.py, check_key() tests falsity of the key using "if not key" and if the key being checked is an empty string, it raises Client.MemcachedKeyNoneError("Key is None").

In the context of delete/get/set_multi, an empty string passed to check_key() only means nothing extra is being added to the key prefix, so it shouldn't be an error, and definitely not a *misleading* error.

The correct behaviour should be to test "if key is None", and add a check for an empty key such as "if len(key) + key_extra_len == 0", once it has been established that the key is a string.

Sean Reifschneider (jafo) wrote :

The *_multi methods take "key" and "key_prefix" arguments. From a quick review of the code, it looks like f the key gets to check_key as '', then it is indeed the key and not the key_prefix, is empty. Also see _map_and_prefix_keys() where it checks "if not key_prefix" before calling check_key(key_prefix).

Can you provide a proposed patch which fixes this? From looking at the code, it's not clear to me what change you are proposing.

Changed in python-memcached:
assignee: nobody → Sean Reifschneider (jafo)
status: New → Incomplete

Patch attached.

My use case:

  * You want to delete a load of keys, and you know they all begin with the same prefix, with various suffixes that you have in a list. You'd use delete_multi(suffixes, key_prefix=prefix).
  * What if one of the keys to delete is just the bare prefix itself? Then one of the suffixes is the empty string.

But:

  * _map_and_prefix_keys() will call check_key() with each of the suffixes as "key", and the length of the prefix as "key_extra_len".
  * check_key() will fail if "key" is an empty string, and it will fail with an error that suggests that the key is None (which it isn't).
  * check_key() only needs to fail if "key" is an empty string and "key_extra_len" == 0, because if key_extra_len > 0 then the key is not empty.

So this patch allows the case where the set of keys to be deleted with delete_multi() includes the key that is key_prefix itself (one of the key suffixes is empty).

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers