Fix bug #65997 by switching to Serializable for GMP

Rather than using get_properties and __wakeup for serialization
the code now uses Serializable::serialize() and
Serializable::unserialize(). The get_properties handler is switched
to a get_debug_info handler. Thus get_gc will now return only
the normal properties and not do any modifications, thus fixing
the leak. This also avoids a $num property from being publicly
visible after the object was dumped or serialized, so that's an
extra plus.

diff --git a/ext/gmp/gmp.c b/ext/gmp/gmp.c
index 6b3dadf40576f4b748c8747107dd6d663af75efb..af9c73afa2b0a0fffcce4ecbf406d36eb0b7e64b 100644 (file)
--- a/ext/gmp/gmp.c
+++ b/ext/gmp/gmp.c
@@ -24,7 +24,10 @@
 #include "php_ini.h"
 #include "php_gmp.h"
 #include "ext/standard/info.h"
+#include "ext/standard/php_var.h"
+#include "ext/standard/php_smart_str_public.h"
 #include "zend_exceptions.h"
+#include "zend_interfaces.h"
 
 #if HAVE_GMP
 
@@ -564,12 +567,17 @@ static int gmp_cast_object(zval *readobj, zval *writeobj, int type TSRMLS_DC) /*
 }
 /* }}} */
 
-static HashTable *gmp_get_properties(zval *obj TSRMLS_DC) /* {{{ */
+static HashTable *gmp_get_debug_info(zval *obj, int *is_temp TSRMLS_DC) /* {{{ */
 {
-       HashTable *ht = zend_std_get_properties(obj TSRMLS_CC);
+       HashTable *ht, *props = zend_std_get_properties(obj TSRMLS_CC);
        mpz_ptr gmpnum = GET_GMP_FROM_ZVAL(obj);
        zval *zv;
 
+       *is_temp = 1;
+       ALLOC_HASHTABLE(ht);
+       ZEND_INIT_SYMTABLE_EX(ht, zend_hash_num_elements(props) + 1, 0);
+       zend_hash_copy(ht, props, (copy_ctor_func_t) zval_add_ref, NULL, sizeof(zval *));
+
        MAKE_STD_ZVAL(zv);
        gmp_strval(zv, gmpnum, 10);
        zend_hash_update(ht, "num", sizeof("num"), &zv, sizeof(zval *), NULL);
@@ -678,36 +686,96 @@ static int gmp_compare(zval *result, zval *op1, zval *op2 TSRMLS_DC) /* {{{ */
 }
 /* }}} */
 
-PHP_METHOD(GMP, __wakeup) /* {{{ */
+PHP_METHOD(GMP, serialize) /* {{{ */
 {
-       HashTable *props;
-       zval **num_zv;
+       mpz_ptr gmpnum = GET_GMP_FROM_ZVAL(getThis());
+       smart_str buf = {0};
+       php_serialize_data_t var_hash;
+       zval zv, *zv_ptr = &zv;
 
        if (zend_parse_parameters_none() == FAILURE) {
                return;
        }
 
-       props = zend_std_get_properties(getThis() TSRMLS_CC);
-       if (zend_hash_find(props, "num", sizeof("num"), (void **) &num_zv) == SUCCESS
-           && Z_TYPE_PP(num_zv) == IS_STRING && Z_STRLEN_PP(num_zv) > 0
+       PHP_VAR_SERIALIZE_INIT(var_hash);
+
+       INIT_PZVAL(zv_ptr);
+
+       gmp_strval(zv_ptr, gmpnum, 10);
+       php_var_serialize(&buf, &zv_ptr, &var_hash TSRMLS_CC);
+       zval_dtor(zv_ptr);
+
+       Z_ARRVAL_P(zv_ptr) = zend_std_get_properties(getThis() TSRMLS_CC);
+       Z_TYPE_P(zv_ptr) = IS_ARRAY;
+       php_var_serialize(&buf, &zv_ptr, &var_hash TSRMLS_CC);
+
+       PHP_VAR_SERIALIZE_DESTROY(var_hash);
+
+       if (buf.c) {
+               RETURN_STRINGL(buf.c, buf.len, 0);
+       }
+}
+/* }}} */
+
+PHP_METHOD(GMP, unserialize) /* {{{ */
+{
+       mpz_ptr gmpnum = GET_GMP_FROM_ZVAL(getThis());
+       char *str;
+       int str_len;
+       php_unserialize_data_t var_hash;
+       const unsigned char *p, *max;
+       zval zv, *zv_ptr = &zv;
+
+       if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "s", &str, &str_len) == FAILURE) {
+               return;
+       }
+
+       PHP_VAR_UNSERIALIZE_INIT(var_hash);
+
+       p = (unsigned char *) str;
+       max = (unsigned char *) str + str_len;
+
+       INIT_ZVAL(zv);
+       if (!php_var_unserialize(&zv_ptr, &p, max, &var_hash TSRMLS_CC)
+               || Z_TYPE_P(zv_ptr) != IS_STRING
+               || convert_to_gmp(gmpnum, zv_ptr, 10 TSRMLS_CC) == FAILURE
        ) {
-               mpz_ptr gmpnumber = GET_GMP_FROM_ZVAL(getThis());
-               if (convert_to_gmp(gmpnumber, *num_zv, 10 TSRMLS_CC) == SUCCESS) {
-                       return;
-               }
+               zend_throw_exception(NULL, "Could not unserialize number", 0 TSRMLS_CC);
+               goto exit;
        }
+       zval_dtor(&zv);
 
-       zend_throw_exception(
-               NULL, "Invalid serialization data", 0 TSRMLS_CC 
-       );
+       INIT_ZVAL(zv);
+       if (!php_var_unserialize(&zv_ptr, &p, max, &var_hash TSRMLS_CC)
+               || Z_TYPE_P(zv_ptr) != IS_ARRAY
+       ) {
+               zend_throw_exception(NULL, "Could not unserialize properties", 0 TSRMLS_CC);
+               goto exit;
+       }
+
+       if (zend_hash_num_elements(Z_ARRVAL_P(zv_ptr)) != 0) {
+               zend_hash_copy(
+                       zend_std_get_properties(getThis() TSRMLS_CC), Z_ARRVAL_P(zv_ptr),
+                       (copy_ctor_func_t) zval_add_ref, NULL, sizeof(zval *)
+               );
+       }
+
+exit:
+       zval_dtor(&zv);
+       PHP_VAR_UNSERIALIZE_DESTROY(var_hash);
 }
 /* }}} */
 
-ZEND_BEGIN_ARG_INFO_EX(arginfo_wakeup, 0, 0, 0)
+ZEND_BEGIN_ARG_INFO_EX(arginfo_serialize, 0, 0, 0)
+ZEND_END_ARG_INFO()
+
+ZEND_BEGIN_ARG_INFO_EX(arginfo_unserialize, 0, 0, 1)
+ZEND_ARG_INFO(0, serialized)
 ZEND_END_ARG_INFO()
 
 const zend_function_entry gmp_methods[] = {
-       PHP_ME(GMP, __wakeup, arginfo_wakeup, ZEND_ACC_PUBLIC)
+       PHP_ME(GMP, serialize, arginfo_serialize, ZEND_ACC_PUBLIC)
+       PHP_ME(GMP, unserialize, arginfo_unserialize, ZEND_ACC_PUBLIC)
        PHP_FE_END
 };
 
@@ -724,13 +792,14 @@ static ZEND_GINIT_FUNCTION(gmp)
 ZEND_MINIT_FUNCTION(gmp)
 {
        zend_class_entry tmp_ce;
-       INIT_CLASS_ENTRY(tmp_ce, "GMP", gmp_methods); /* No methods on the class for now */
+       INIT_CLASS_ENTRY(tmp_ce, "GMP", gmp_methods);
        gmp_ce = zend_register_internal_class(&tmp_ce TSRMLS_CC);
+       zend_class_implements(gmp_ce TSRMLS_CC, 1, zend_ce_serializable);
        gmp_ce->create_object = gmp_create_object;
 
        memcpy(&gmp_object_handlers, zend_get_std_object_handlers(), sizeof(zend_object_handlers));
        gmp_object_handlers.cast_object = gmp_cast_object;
-       gmp_object_handlers.get_properties = gmp_get_properties;
+       gmp_object_handlers.get_debug_info = gmp_get_debug_info;
        gmp_object_handlers.clone_obj = gmp_clone_obj;
        gmp_object_handlers.do_operation = gmp_do_operation;
        gmp_object_handlers.compare = gmp_compare;

diff --git a/ext/gmp/tests/bug659967.phpt b/ext/gmp/tests/bug659967.phpt
new file mode 100644 (file)
index 0000000..6ba2202
--- /dev/null
+++ b/ext/gmp/tests/bug659967.phpt
@@ -0,0 +1,15 @@
+--TEST--
+Bug #65997: Leak when using gc_collect_cycles with new GMP implementation
+--SKIPIF--
+<?php if (!extension_loaded("gmp")) print "skip"; ?>
+--FILE--
+<?php
+
+gc_enable();
+$gmp = gmp_init('10');
+gc_collect_cycles();
+
+?>
+===DONE===
+--EXPECT--
+===DONE===

diff --git a/ext/gmp/tests/serialize.phpt b/ext/gmp/tests/serialize.phpt
index 26716b659c368526c54ceef520bcd1306d9253ba..208e0e98001e767b6f5a1c194263f6df80288865 100644 (file)
--- a/ext/gmp/tests/serialize.phpt
+++ b/ext/gmp/tests/serialize.phpt
@@ -9,14 +9,34 @@ var_dump($n = gmp_init(42));
 var_dump($s = serialize($n));
 var_dump(unserialize($s));
 
+$n = gmp_init(13);
+$n->foo = "bar";
+var_dump(unserialize(serialize($n)));
+
+try {
+    unserialize('C:3:"GMP":0:{}');
+} catch (Exception $e) { var_dump($e->getMessage()); }
+
+try {
+    unserialize('C:3:"GMP":8:{s:2:"42"}');
+} catch (Exception $e) { var_dump($e->getMessage()); }
+
 ?>
 --EXPECTF--    
 object(GMP)#%d (1) {
   ["num"]=>
   string(2) "42"
 }
-string(33) "O:3:"GMP":1:{s:3:"num";s:2:"42";}"
+string(30) "C:3:"GMP":15:{s:2:"42";a:0:{}}"
 object(GMP)#%d (1) {
   ["num"]=>
   string(2) "42"
 }
+object(GMP)#%d (2) {
+  ["foo"]=>
+  string(3) "bar"
+  ["num"]=>
+  string(2) "13"
+}
+string(28) "Could not unserialize number"
+string(32) "Could not unserialize properties"