\x0a in key passed to get() and set() locks up connection and triggers a fatal timeout error

[hugovangalen]

FYI: after I had accidentally added binary data to the key value, I discovered that the value passed to get() and set() are allowed to contain data that can trigger a fatal error (memcache timeout after X seconds). This only affects the running PHP script.

See this short example below.

$data = [];
$key = "test\x0a";
$test = $m->get( $key );
$m->set( $key, $data, 300 );

Interestingly, it does not happen if the get() is commented out. It has to be get() first before set() hangs.

[ficus]

Hmm, behaviour is a little weird.

A)

$key = "test\x0a";
$m->set($key, 'abc', 300);
var_dump($m->getResultMessage());
---        
// immediate response
//...:string 'ERROR was returned by server' (length=28)

B)

$key = "test\x0a";
$test = $m->get($key);
$m->set($key, 'abc', 300);
var_dump($m->getResultMessage());
---        
//after 30 seconds hits PHP's max execution time, trace ties it to the line with `set` call
//Fatal error: Maximum execution time of 30 seconds exceeded in ...

C)

$key = "test\x0a";
$test = $m->get($key);
var_dump($m->getResultMessage());
---
//between 80 to 100 seconds later comes back with
//...string 'NOT FOUND' (length=9)

Keys can be up to 250 bytes. Can't contain:
null (0x00)
space (0x20)
tab (0x09)
newline (0x0a)
carriage-return (0x0d)

Side note, the value you are trying to set, an array, would not be valid. Argument 2 of set is a string.

I was using 3.0.2 php-memcached. STAT version 1.4.35 / STAT libevent 2.1.8-stable
Not sure how retrieval of invalid keys should be treated, the set seems to behave as would be expected. Not sure I can really explain variance btw B and C. Don't really see it as an issue. Just thought I'd chime in with what I observed.

[sodabrew]

It would make sense to me that the client library should validate keys that are documented as being invalid in memcached, so I will look into adding the necessary code.

[sodabrew]

Ok, I see the problem: the code that checks the validity of the keys is not null-safe, whereas the code that places the keys into the protocol stream is null-safe.

[sodabrew]

I was finally able to reproduce the problem - the unit test for key validity was actually enabling libmemcached to do its own key checks, using Memcached::OPT_VERIFY_KEY => truein the config array - and so the tests were not catching the incorrect code in the functions_memc_valid_key_ascii`.

In the short term, you can add Memcached::OPT_VERIFY_KEY => true` to your Memcached initialization. Meanwhile I've got improved code queued up for the next release.