Skip to content

Commit b57ea9d

Browse files
author
Andrei Zmievski
committed
Merge a number of patches.
libmemcached may return NULL as a result if the length of the value is 0. Make zval_from_payload defensive accept NULL as payload Fixed memcached-api.php to match get and get by key cast token parameter. Separated CAS tests, and new tests.
1 parent 9f8724b commit b57ea9d

File tree

6 files changed

+114
-29
lines changed

6 files changed

+114
-29
lines changed

‎memcached-api.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ class Memcached {
7575

7676
public function __construct( $persistent_id = '' ) {}
7777

78-
public function get( $key, &$cas_token = null, $cache_cb = null ) {}
78+
public function get( $key, $cache_cb = null, &$cas_token = null ) {}
7979

80-
public function getByKey( $server_key, $key, $cache_cb = null ) {}
80+
public function getByKey( $server_key, $key, $cache_cb = null, &$cas_token = null ) {}
8181

8282
public function getMulti( array $keys, &$cas_tokens = null, $flags = 0 ) {}
8383

‎php_memcached.c‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,7 @@ static void php_memc_get_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key)
422422
uint32_t dummy_flags;
423423
int rc;
424424
memcached_return dummy_status;
425+
bool return_value_set = false;
425426

426427
status = memcached_mget_by_key(i_obj->memc, server_key, server_key_len, &key, &key_len, 1);
427428
payload = memcached_fetch(i_obj->memc, NULL, NULL, &payload_len, &flags, &status);
@@ -432,10 +433,12 @@ static void php_memc_get_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key)
432433
/*
433434
* If payload wasn't found and we have a read-through callback, invoke it to get
434435
* the value. The callback will take care of storing the value back into memcache.
436+
* The callback will set the return value.
435437
*/
436438
if (payload == NULL && status == MEMCACHED_NOTFOUND && fci.size != 0) {
437439
status = php_memc_do_cache_callback(getThis(), &fci, &fcc, key, key_len,
438440
return_value TSRMLS_CC);
441+
return_value_set = true;
439442
}
440443

441444
(void)memcached_fetch(i_obj->memc, NULL, NULL, &dummy_length, &dummy_flags, &dummy_status);
@@ -447,8 +450,8 @@ static void php_memc_get_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool by_key)
447450
RETURN_FALSE;
448451
}
449452

450-
/* payload will be NULL if the callback was invoked */
451-
if (payload != NULL) {
453+
/* if memcached gave a value and there was no callback, payload may be NULL */
454+
if (!return_value_set) {
452455
rc = php_memc_zval_from_payload(return_value, payload, payload_len, flags TSRMLS_CC);
453456
free(payload);
454457
if (rc < 0) {
@@ -1954,7 +1957,6 @@ static char *php_memc_zval_to_payload(zval *value, size_t *payload_len, uint32_t
19541957
} else {
19551958
#endif
19561959
php_serialize_data_t var_hash;
1957-
19581960
PHP_VAR_SERIALIZE_INIT(var_hash);
19591961
php_var_serialize(&buf, &value, &var_hash TSRMLS_CC);
19601962
PHP_VAR_SERIALIZE_DESTROY(var_hash);
@@ -2004,8 +2006,16 @@ static char *php_memc_zval_to_payload(zval *value, size_t *payload_len, uint32_t
20042006

20052007
static int php_memc_zval_from_payload(zval *value, char *payload, size_t payload_len, uint32_t flags TSRMLS_DC)
20062008
{
2007-
if (payload == NULL) {
2009+
/*
2010+
A NULL payload is completely valid if length is 0, it is simply empty.
2011+
*/
2012+
char dummy_payload[1] = { 0 };
2013+
if (payload == NULL && payload_len > 0) {
2014+
php_error_docref(NULL TSRMLS_CC, E_WARNING,
2015+
"Could not handle non-existing value of length %zu", payload_len);
20082016
return -1;
2017+
} else if (payload == NULL) {
2018+
payload = dummy_payload;
20092019
}
20102020

20112021
if (flags & MEMC_VAL_COMPRESSED) {
@@ -2084,8 +2094,7 @@ static int php_memc_zval_from_payload(zval *value, char *payload, size_t payload
20842094
double dval = zend_strtod(payload, NULL);
20852095
ZVAL_DOUBLE(value, dval);
20862096
} else if (flags & MEMC_VAL_IS_BOOL) {
2087-
long bval = strtol(payload, NULL, 10);
2088-
ZVAL_BOOL(value, bval);
2097+
ZVAL_BOOL(value, payload_len > 0 && payload[0] == '1');
20892098
} else {
20902099
ZVAL_STRINGL(value, payload, payload_len, 1);
20912100
}

‎tests/cas.phpt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
Memcached fetch cas & set cas
3+
--SKIPIF--
4+
<?php if (!extension_loaded("memcached")) print "skip"; ?>
5+
--FILE--
6+
<?php
7+
$m = new Memcached();
8+
$m->addServer('127.0.0.1', 11211, 1);
9+
10+
$m->delete('cas_test');
11+
$cas_token = null;
12+
13+
$m->set('cas_test', 10);
14+
$v = $m->get('cas_test', null, $cas_token);
15+
16+
if (is_null($cas_token)) {
17+
echo "Null cas token for key: cas_test value: 10\n";
18+
return;
19+
}
20+
21+
$v = $m->cas($cas_token, 'cas_test', 11);
22+
if (!$v) {
23+
echo "Error setting key: cas_test value: 11 with CAS: $cas_token\n";
24+
return;
25+
}
26+
27+
$v = $m->get('cas_test');
28+
29+
if ($v !== 11) {
30+
echo "Wanted cas_test to be 11, value is: ";
31+
var_dump($v);
32+
}
33+
?>
34+
--EXPECT--

‎tests/cas_multi.phpt‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
--TEST--
2+
Memcached multi fetch cas & set cas
3+
--SKIPIF--
4+
<?php if (!extension_loaded("memcached")) print "skip"; ?>
5+
--FILE--
6+
<?php
7+
$m = new Memcached();
8+
$m->addServer('127.0.0.1', 11211, 1);
9+
10+
$data = array(
11+
'cas_test_1' => 1,
12+
'cas_test_2' => 2,
13+
);
14+
15+
foreach ($data as $key => $v) {
16+
$m->delete($key);
17+
}
18+
19+
$cas_tokens = array();
20+
$m->setMulti($data, 10);
21+
$actual = $m->getMulti(array_keys($data), $cas_tokens);
22+
23+
foreach ($data as $key => $v) {
24+
if (is_null($cas_tokens[$key])) {
25+
echo "missing cas token(s)\n";
26+
echo "data: ";
27+
var_dump($data);
28+
echo "actual data: ";
29+
var_dump($actual);
30+
echo "cas tokens: ";
31+
var_dump($cas_tokens);
32+
return;
33+
}
34+
35+
$v = $m->cas($cas_tokens[$key], $key, 11);
36+
if (!$v) {
37+
echo "Error setting key: $key value: 11 with CAS: ", $cas_tokens[$key], "\n";
38+
return;
39+
}
40+
$v = $m->get($key);
41+
if ($v !== 11) {
42+
echo "Wanted $key to be 11, value is: ";
43+
var_dump($v);
44+
return;
45+
}
46+
}
47+
48+
49+
?>
50+
--EXPECT--

‎tests/types.phpt‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -36,29 +36,25 @@ $data = array(
3636
array('object_dummy', new testclass()),
3737
);
3838

39+
foreach ($data as $key => $value) {
40+
$m->delete($key);
41+
}
42+
3943
foreach ($data as $types) {
4044
$m->set($types[0], $types[1]);
4145
$actual = $m->get($types[0]);
42-
$cas = null;
43-
$actual_cas = $m->get($types[0], $cas);
44-
if ($types[1] !== $actual || $types[1] !== $actual_cas) {
45-
if (isset($cas) && is_object($types[1])
46+
if ($types[1] !== $actual) {
47+
if (is_object($types[1])
4648
&& $types[1] == $actual
47-
&& $types[1] == $actual_cas
48-
&& get_class($types[1]) == get_class($actual)
49-
&& get_class($types[1]) == get_class($actual_cas)) {
49+
&& get_class($types[1]) == get_class($actual)) {
5050
continue;
5151
}
5252
echo "=== $types[0] ===\n";
5353
echo "Expected: ";
5454
var_dump($types[1]);
5555
echo "Actual: ";
5656
var_dump($actual);
57-
echo "Actual CAS: ";
58-
var_dump($actual_cas);
59-
echo "Cas: ", $cas, "\n";
6057
}
61-
6258
}
6359

6460
?>

‎tests/types_multi.phpt‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,28 +36,24 @@ $data = array(
3636
'object_dummy' => new testclass(),
3737
);
3838

39+
foreach ($data as $key => $value) {
40+
$m->delete($key);
41+
}
3942
$m->setMulti($data);
4043
$actual = $m->getMulti(array_keys($data));
41-
$cas = array();
42-
$actual_cas = $m->getMulti(array_keys($data), $cas);
4344

4445
foreach ($data as $key => $value) {
45-
if ($value !== $actual[$key] || $value !== $actual_cas[$key] || !isset($cas[$key])) {
46-
if (isset($cas[$key]) && is_object($value)
46+
if ($value !== $actual[$key]) {
47+
if (is_object($value)
4748
&& $value == $actual[$key]
48-
&& $value == $actual_cas[$key]
49-
&& get_class($value) == get_class($actual[$key])
50-
&& get_class($value) == get_class($actual_cas[$key])) {
49+
&& get_class($value) == get_class($actual[$key])) {
5150
continue;
5251
}
5352
echo "=== $key ===\n";
5453
echo "Expected: ";
5554
var_dump($value);
5655
echo "Actual: ";
5756
var_dump($actual[$key]);
58-
echo "Actual CAS: ";
59-
var_dump($actual_cas[$key]);
60-
echo "Cas: ", $cas[$key], "\n";
6157
}
6258
}
6359

0 commit comments

Comments
 (0)