RedisArray::del() clobbers single string argument #262

Closed
seansawyer opened this Issue Oct 8, 2012 · 6 comments

Comments

Projects
None yet
3 participants

Seems like when a single key is passed as an argument (not an array of one key), RedisArray::del() is freeing the memory where that argument is stored. In the following example you'll notice that $key somehow takes on the value of $vals, resulting in an array to string conversion notice:

<?php
function test($vals) {
    $r = new RedisArray(array('localhost:12001', 'localhost:12002', 'localhost:12003'));
    $key = 'somekey';
    $host = $r->_target($key);
    $p = $r->multi($host, Redis::PIPELINE);
    var_dump($key); // string(7) "somekey"
    $p->del($key);
    var_dump($key); // NULL
    foreach ($vals as $val) {
        var_dump($key); // array(1) { [0] => string(3) "foo" }
        $p->sadd($key, $val); // array to string conversion notice
    }
    $p->exec();
}

test(array('foo'));
?>

And in this example it simply segfaults:

<?php
$r = new RedisArray(array('localhost:12001', 'localhost:12002', 'localhost:12003'));
$key = 'somekey';
$vals = array('foo');
$host = $r->_target($key);
$p = $r->multi($host, Redis::PIPELINE);
var_dump($key); // string(7) "somekey"
$p->del($key);
var_dump($key); // NULL
foreach ($vals as $val) {
    var_dump($key); // NULL
    $p->sadd($key, $val); // segfault :(
}
$p->exec();
?>

FYI I'm using 7dfac44 (current HEAD of master) with PHP 5.4 on Arch Linux.

Contributor

nicolasff commented Oct 8, 2012

Thanks.
I can indeed reproduce the issue and will be pushing a fix soon.

Sweet, thanks @nicolasff

@nicolasff nicolasff added a commit that referenced this issue Oct 8, 2012

@nicolasff nicolasff Copy zval in multi/exec/pipe forwarded array calls
* Addresses GitHub issue #262
* Tested successfully with code provided by bug reporter
* array-tests.php passes
20f555e
Contributor

nicolasff commented Oct 8, 2012

@seansawyer I've pushed a fix and made sure your code works. Could you please try it out?

Yep, both examples above work for me with 20f555e

@nicolasff Not sure whether you're keeping this open for your own purposes, but I've been using 20f555e for a few days now and have seen zero problems with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment