delete_multi breaks if one of the key parts is empty string

Bug #832131 reported by Pete-alex-harris
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Python Memcached
Incomplete
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.

Revision history for this message
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
Revision history for this message
Pete-alex-harris (pete-alex-harris) wrote :

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  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.