Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Compatibility with PHP 8.2 #102

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Compatibility with PHP 8.2 #102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0ae2393
prepare for PHP 8.2
Sep 9, 2022
d05a129
declare username and password properties explicitly, add tests for me…
pmurzakov Oct 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
41 changes: 34 additions & 7 deletions src/memcache.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,9 @@ static zend_function_entry php_memcache_class_functions[] = {
ZEND_FE_END
};

#define Z_MEMCACHE_CONNECTION_P(zv) OBJ_PROP_NUM(Z_OBJ_P(zv), 0)
#define Z_MEMCACHE__FAILURECALLBACK_P(zv) OBJ_PROP_NUM(Z_OBJ_P(zv), 1)

/* }}} */

/* {{{ memcache_module_entry
Expand Down Expand Up @@ -737,6 +740,30 @@ PHP_MINIT_FUNCTION(memcache)
INIT_CLASS_ENTRY(ce, "Memcache", php_memcache_class_functions);
memcache_ce = zend_register_internal_class_ex(&ce, memcache_pool_ce);

zval property_connection_default_value;
ZVAL_NULL(&property_connection_default_value);
zend_string *property_connection_name = zend_string_init("connection", sizeof("connection") - 1, 1);
zend_declare_typed_property(memcache_ce, property_connection_name, &property_connection_default_value, ZEND_ACC_PUBLIC, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_ANY));
zend_string_release(property_connection_name);

zval property_failure_callback_default_value;
ZVAL_NULL(&property_failure_callback_default_value);
zend_string *property_failure_callback_name = zend_string_init("_failureCallback", sizeof("_failureCallback") - 1, 1);
zend_declare_typed_property(memcache_ce, property_failure_callback_name, &property_failure_callback_default_value, ZEND_ACC_PRIVATE, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_CALLABLE|MAY_BE_NULL));
zend_string_release(property_failure_callback_name);

zval property_username_default_value;
ZVAL_NULL(&property_username_default_value);
zend_string *property_username_name = zend_string_init("username", sizeof("username") - 1, 1);
zend_declare_typed_property(memcache_ce, property_username_name, &property_username_default_value, ZEND_ACC_PUBLIC, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_STRING|MAY_BE_NULL));
zend_string_release(property_username_name);

zval property_password_default_value;
ZVAL_NULL(&property_password_default_value);
zend_string *property_password_name = zend_string_init("password", sizeof("password") - 1, 1);
zend_declare_typed_property(memcache_ce, property_password_name, &property_password_default_value, ZEND_ACC_PUBLIC, NULL, (zend_type) ZEND_TYPE_INIT_MASK(MAY_BE_STRING|MAY_BE_NULL));
zend_string_release(property_password_name);

le_memcache_pool = zend_register_list_destructors_ex(_mmc_pool_list_dtor, NULL, "memcache connection", module_number);
le_memcache_server = zend_register_list_destructors_ex(NULL, _mmc_server_list_dtor, "persistent memcache connection", module_number);

Expand Down Expand Up @@ -813,7 +840,7 @@ static int mmc_get_pool(zval *id, mmc_pool_t **pool) /* {{{ */
{
zval *zv;

if (Z_TYPE_P(id) != IS_OBJECT || (zv = zend_hash_str_find(Z_OBJPROP_P(id), "connection", sizeof("connection")-1)) == NULL) {
if (Z_TYPE_P(id) != IS_OBJECT || (zv = Z_MEMCACHE_CONNECTION_P(id)) == NULL || Z_TYPE_P(zv) == IS_NULL) {
php_error_docref(NULL, E_WARNING, "No servers added to memcache connection");
return 0;
}
Expand Down Expand Up @@ -1233,11 +1260,11 @@ static mmc_t *php_mmc_pool_addserver(
return NULL;
}
/* initialize pool if need be */
if ((connection = zend_hash_str_find(Z_OBJPROP_P(mmc_object), "connection", sizeof("connection")-1)) == NULL) {
if (Z_TYPE_P(connection = Z_MEMCACHE_CONNECTION_P(mmc_object)) != IS_RESOURCE) {
pool = mmc_pool_new();
pool->failure_callback = (mmc_failure_callback) &php_mmc_failure_callback;
list_res = zend_register_resource(pool, le_memcache_pool);
add_property_resource(mmc_object, "connection", list_res);
ZVAL_RES(Z_MEMCACHE_CONNECTION_P(mmc_object), list_res);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

original code would free the old value of $memcache->connection, this replacement just overwrites without checking the old value. (e.g. $memcache->connection = new stdClass(); $memcache->addServer(...); would no longer free the stdClass)

It isn't performance sensitive for something that'd be called once total per Memcache instance


#if PHP_VERSION_ID < 70300
GC_REFCOUNT(list_res)++;
Expand Down Expand Up @@ -1333,7 +1360,7 @@ static void php_mmc_connect(INTERNAL_FUNCTION_PARAMETERS, zend_bool persistent)
list_res = zend_register_resource(pool, le_memcache_pool);
mmc_object = return_value;
object_init_ex(mmc_object, memcache_ce);
add_property_resource(mmc_object, "connection", list_res);
ZVAL_RES(Z_MEMCACHE_CONNECTION_P(mmc_object), list_res);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original code would free the original property value if something set it before the call to connect (e.g. user manually calling <?php $memcache->connection = new stdClass(); $memcache->connect();)

#if PHP_VERSION_ID < 70300
GC_REFCOUNT(list_res)++;
#else
Expand Down Expand Up @@ -1486,7 +1513,7 @@ static void php_mmc_failure_callback(mmc_pool_t *pool, mmc_t *mmc, zval *param)
zval *callback;

/* check for userspace callback */
if (!Z_ISUNDEF_P(param) && (callback = zend_hash_str_find(Z_OBJPROP_P((zval *)param), "_failureCallback", sizeof("_failureCallback")-1)) != NULL && Z_TYPE_P(callback) != IS_NULL) {
if (!Z_ISUNDEF_P(param) && (callback = Z_MEMCACHE__FAILURECALLBACK_P(param)) != NULL && Z_TYPE_P(callback) != IS_NULL) {
if (MEMCACHE_IS_CALLABLE(callback, 0, NULL)) {
zval retval;
zval *host, *tcp_port, *udp_port, *error, *errnum;
Expand Down Expand Up @@ -1546,15 +1573,15 @@ static void php_mmc_set_failure_callback(mmc_pool_t *pool, zval *mmc_object, zva
callback_tmp = *callback;
zval_copy_ctor(&callback_tmp);

add_property_zval(mmc_object, "_failureCallback", &callback_tmp);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add_property_zval frees the old value of _failureCallback, but the old code might leak the old value (ZVAL_COPY overwrites without checking the original value). It should be safe to keep the original code (not a performance sensitive part)?

E.g. add tests of calling https://www.php.net/manual/en/memcache.setserverparams.php more than once with a non-null failure callback and run in a debug php build

ZVAL_COPY(Z_MEMCACHE__FAILURECALLBACK_P(mmc_object), &callback_tmp);

zval_ptr_dtor(&callback_tmp);

pool->failure_callback_param = *mmc_object;
Z_ADDREF_P(mmc_object);
}
else {
add_property_null(mmc_object, "_failureCallback");
ZVAL_NULL(Z_MEMCACHE__FAILURECALLBACK_P(mmc_object));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

ZVAL_UNDEF(&pool->failure_callback_param);
}
}
Expand Down
6 changes: 6 additions & 0 deletions tests/017.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,11 @@ foo
object(test)%s%d) {
["connection"]=>
resource(%d) of type (memcache connection)
["_failureCallback":"Memcache":private]=>
NULL
["username"]=>
NULL
["password"]=>
NULL
}
Done
3 changes: 2 additions & 1 deletion tests/029.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ if (is_array($result))
sort($result);
var_dump($result);

$result = ini_set('memcache.allow_failover', "abc");
// @ is to prevent warning on 8.2
$result = @ini_set('memcache.allow_failover', "abc");
var_dump($result);

?>
Expand Down
2 changes: 2 additions & 0 deletions tests/045.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ Nested get's in __wakeup()
include 'connect.inc';

class testclass {
public $result;

function __wakeup() {
global $memcache;
$this->result = $memcache->get('_test_key3');
Expand Down
6 changes: 5 additions & 1 deletion tests/059.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,11 @@ Class [ <internal:memcache> class Memcache extends MemcachePool ] {
- Static methods [0] {
}

- Properties [0] {
- Properties [4] {
Property [ public mixed $connection = NULL ]
Property [ private ?callable $_failureCallback = NULL ]
Property [ public ?string $username = NULL ]
Property [ public ?string $password = NULL ]
}

- Methods [24] {
Expand Down
23 changes: 23 additions & 0 deletions tests/101.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
--TEST--
memcache->setSaslAuthData() and memcache_set_sasl_auth_data()
--SKIPIF--
<?php include 'connect.inc'; ?>
--FILE--
<?php
include 'connect.inc';

$memcache->setSaslAuthData('testuser1', 'testpassword1');

var_dump($memcache->username);
var_dump($memcache->password);

memcache_set_sasl_auth_data($memcache, 'testuser2', 'testpassword2');

var_dump($memcache->username);
var_dump($memcache->password);

--EXPECTF--
string(9) "testuser1"
string(13) "testpassword1"
string(9) "testuser2"
string(13) "testpassword2"