From f7b7b6aa9eb43e5969dac9c8fcd92f0f2cb3e6d7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gustavo=20Andr=C3=A9=20dos=20Santos=20Lopes?= Date: Sun, 27 Mar 2011 22:44:34 +0000 Subject: [PATCH] - Improved upon r309729. - Extented strategy to remaining the classes on spl_directory.c, even those that don't crash. - UPGRADING. - Better bug54384.phpt, with all the classes covered. --- UPGRADING | 12 ++++ ext/spl/php_spl.c | 26 +++++++-- ext/spl/spl_directory.c | 39 +++++-------- ext/spl/tests/bug54384.phpt | 107 +++++++++++++++++++++++++++++------- 4 files changed, 132 insertions(+), 52 deletions(-) diff --git a/UPGRADING b/UPGRADING index c856b13f2f..c5d0272e21 100755 --- a/UPGRADING +++ b/UPGRADING @@ -184,6 +184,18 @@ UPGRADE NOTES - PHP X.Y stream_truncate that will respond to truncation, e.g. through ftruncate. Strictly speaking, this is an addition to the user-space stream wrapper template, not a change to an actual class. +- Constructors of userspace subclasses of the following SPL classes are now + required to call the parent constructor and not clear any exceptions they + throw (replacing the thrown exception by another one is permitted): + SplFileInfo, DirectoryIterator, FilesystemIterator, GlobIterator, + SplFileObject, SplTempFileObject, RecursiveDirectoryIterator, + IteratorIterator, FilterIterator, RecursiveFilterIterator, ParentIterator, + LimitIterator, CachingIterator, RecursiveCachingIterator, NoRewindIterator, + AppendIterator, InfiniteIterator, RegexIterator, RecursiveRegexIterator, and + RecursiveTreeIterator. + It is no longer possible to defer the parent constructor call until after the + object is constructed. + ============= 7. Deprecated diff --git a/ext/spl/php_spl.c b/ext/spl/php_spl.c index cd9a548d8f..de59a02c93 100755 --- a/ext/spl/php_spl.c +++ b/ext/spl/php_spl.c @@ -66,7 +66,7 @@ static PHP_GINIT_FUNCTION(spl) cwf->type = ZEND_INTERNAL_FUNCTION; cwf->common.function_name = "internal_construction_wrapper"; - cwf->common.scope = NULL; /* to be filled */ + cwf->common.scope = NULL; /* to be filled w/ object runtime class */ cwf->common.fn_flags = ZEND_ACC_PRIVATE; cwf->common.prototype = NULL; cwf->common.num_args = 0; /* not necessarily true but not enforced */ @@ -815,8 +815,18 @@ static void construction_wrapper(INTERNAL_FUNCTION_PARAMETERS) /* {{{ */ zval *retval_ptr = NULL; object_data = zend_object_store_get_object(this TSRMLS_CC); - zf = zend_get_std_object_handlers()->get_constructor(this TSRMLS_CC); this_ce = Z_OBJCE_P(this); + + /* The call of this internal function did not change the scope because + * zend_do_fcall_common_helper doesn't do that for internal instance + * function calls. So the visibility checks on zend_std_get_constructor + * will still work. Reflection refuses to instantiate classes whose + * constructor is not public so we're OK there too*/ + zf = zend_std_get_constructor(this TSRMLS_CC); + + if (zf == NULL) { + return; + } fci.size = sizeof(fci); fci.function_table = &this_ce->function_table; @@ -828,15 +838,17 @@ static void construction_wrapper(INTERNAL_FUNCTION_PARAMETERS) /* {{{ */ fci.params = emalloc(fci.param_count * sizeof *fci.params); if (zend_get_parameters_array_ex(ZEND_NUM_ARGS(), fci.params) == FAILURE) { zend_throw_exception(NULL, "Unexpected error fetching arguments", 0 TSRMLS_CC); - return; + goto cleanup; } } fci.object_ptr = this; fci.no_separation = 0; - + fci_cache.initialized = 1; - fci_cache.called_scope = EG(current_execute_data)->called_scope; - fci_cache.calling_scope = EG(current_execute_data)->current_scope; + fci_cache.called_scope = this_ce; /* set called scope to class of this */ + /* function->common.scope will replace it, except for + * ZEND_OVERLOADED_FUNCTION, which we won't get */ + fci_cache.calling_scope = EG(scope); fci_cache.function_handler = zf; fci_cache.object_ptr = this; @@ -852,6 +864,8 @@ static void construction_wrapper(INTERNAL_FUNCTION_PARAMETERS) /* {{{ */ "and its exceptions cannot be cleared", this_ce->name); cleanup: + /* no need to cleanup zf, zend_std_get_constructor never allocates a new + * function (so no ZEND_OVERLOADED_FUNCTION or call-via-handlers) */ if (fci.params != NULL) { efree(fci.params); } diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index b2220fee1b..4cb3aece62 100755 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -48,7 +48,6 @@ /* declare the class handlers */ static zend_object_handlers spl_filesystem_object_handlers; -static zend_object_handlers spl_filesystem_object_constru_check_handlers; /* declare the class entry */ PHPAPI zend_class_entry *spl_ce_SplFileInfo; @@ -133,7 +132,7 @@ static void spl_filesystem_object_free_storage(void *object TSRMLS_DC) /* {{{ */ - clone - new */ -static zend_object_value spl_filesystem_object_new_ex(zend_class_entry *class_type, zend_object_handlers *handlers, spl_filesystem_object **obj TSRMLS_DC) +static zend_object_value spl_filesystem_object_new_ex(zend_class_entry *class_type, spl_filesystem_object **obj TSRMLS_DC) { zend_object_value retval; spl_filesystem_object *intern; @@ -151,11 +150,8 @@ static zend_object_value spl_filesystem_object_new_ex(zend_class_entry *class_ty object_properties_init(&intern->std, class_type); retval.handle = zend_objects_store_put(intern, (zend_objects_store_dtor_t) zend_objects_destroy_object, (zend_objects_free_object_storage_t) spl_filesystem_object_free_storage, NULL TSRMLS_CC); - if (!handlers) { - retval.handlers = &spl_filesystem_object_handlers; - } else { - retval.handlers = handlers; - } + retval.handlers = &spl_filesystem_object_handlers; + return retval; } /* }}} */ @@ -164,13 +160,7 @@ static zend_object_value spl_filesystem_object_new_ex(zend_class_entry *class_ty /* See spl_filesystem_object_new_ex */ static zend_object_value spl_filesystem_object_new(zend_class_entry *class_type TSRMLS_DC) { - return spl_filesystem_object_new_ex(class_type, NULL /* spl_filesystem_object_handlers */, NULL TSRMLS_CC); -} -/* }}} */ - -static zend_object_value spl_filesystem_object_constru_check_new(zend_class_entry *class_type TSRMLS_DC) -{ - return spl_filesystem_object_new_ex(class_type, &spl_filesystem_object_constru_check_handlers, NULL TSRMLS_CC); + return spl_filesystem_object_new_ex(class_type, NULL TSRMLS_CC); } /* }}} */ @@ -320,7 +310,7 @@ static zend_object_value spl_filesystem_object_clone(zval *zobject TSRMLS_DC) old_object = zend_objects_get_address(zobject TSRMLS_CC); source = (spl_filesystem_object*)old_object; - new_obj_val = spl_filesystem_object_new_ex(old_object->ce, NULL, &intern TSRMLS_CC); + new_obj_val = spl_filesystem_object_new_ex(old_object->ce, &intern TSRMLS_CC); new_object = &intern->std; intern->flags = source->flags; @@ -419,7 +409,7 @@ static spl_filesystem_object * spl_filesystem_object_create_info(spl_filesystem_ zend_update_class_constants(ce TSRMLS_CC); - return_value->value.obj = spl_filesystem_object_new_ex(ce, NULL, &intern TSRMLS_CC); + return_value->value.obj = spl_filesystem_object_new_ex(ce, &intern TSRMLS_CC); Z_TYPE_P(return_value) = IS_OBJECT; if (ce->constructor->common.scope != spl_ce_SplFileInfo) { @@ -462,7 +452,7 @@ static spl_filesystem_object * spl_filesystem_object_create_type(int ht, spl_fil zend_update_class_constants(ce TSRMLS_CC); - return_value->value.obj = spl_filesystem_object_new_ex(ce, NULL, &intern TSRMLS_CC); + return_value->value.obj = spl_filesystem_object_new_ex(ce, &intern TSRMLS_CC); Z_TYPE_P(return_value) = IS_OBJECT; spl_filesystem_object_get_file_name(source TSRMLS_CC); @@ -483,7 +473,7 @@ static spl_filesystem_object * spl_filesystem_object_create_type(int ht, spl_fil zend_update_class_constants(ce TSRMLS_CC); - return_value->value.obj = spl_filesystem_object_new_ex(ce, NULL, &intern TSRMLS_CC); + return_value->value.obj = spl_filesystem_object_new_ex(ce, &intern TSRMLS_CC); Z_TYPE_P(return_value) = IS_OBJECT; spl_filesystem_object_get_file_name(source TSRMLS_CC); @@ -2975,6 +2965,8 @@ PHP_MINIT_FUNCTION(spl_directory) spl_filesystem_object_handlers.clone_obj = spl_filesystem_object_clone; spl_filesystem_object_handlers.cast_object = spl_filesystem_object_cast; spl_filesystem_object_handlers.get_debug_info = spl_filesystem_object_get_debug_info; + spl_filesystem_object_handlers.get_constructor = spl_filesystem_object_get_constructor; + spl_ce_SplFileInfo->serialize = zend_class_serialize_deny; spl_ce_SplFileInfo->unserialize = zend_class_unserialize_deny; @@ -3003,17 +2995,12 @@ PHP_MINIT_FUNCTION(spl_directory) REGISTER_SPL_SUB_CLASS_EX(RecursiveDirectoryIterator, FilesystemIterator, spl_filesystem_object_new, spl_RecursiveDirectoryIterator_functions); REGISTER_SPL_IMPLEMENTS(RecursiveDirectoryIterator, RecursiveIterator); - /* These need the parent constructor call check if extended in userspace. - * The previous ones probably don't work very well if */ - memcpy(&spl_filesystem_object_constru_check_handlers, &spl_filesystem_object_handlers, sizeof(zend_object_handlers)); - spl_filesystem_object_constru_check_handlers.get_constructor = spl_filesystem_object_get_constructor; - #ifdef HAVE_GLOB - REGISTER_SPL_SUB_CLASS_EX(GlobIterator, FilesystemIterator, spl_filesystem_object_constru_check_new, spl_GlobIterator_functions); + REGISTER_SPL_SUB_CLASS_EX(GlobIterator, FilesystemIterator, spl_filesystem_object_new, spl_GlobIterator_functions); REGISTER_SPL_IMPLEMENTS(GlobIterator, Countable); #endif - REGISTER_SPL_SUB_CLASS_EX(SplFileObject, SplFileInfo, spl_filesystem_object_constru_check_new, spl_SplFileObject_functions); + REGISTER_SPL_SUB_CLASS_EX(SplFileObject, SplFileInfo, spl_filesystem_object_new, spl_SplFileObject_functions); REGISTER_SPL_IMPLEMENTS(SplFileObject, RecursiveIterator); REGISTER_SPL_IMPLEMENTS(SplFileObject, SeekableIterator); @@ -3022,7 +3009,7 @@ PHP_MINIT_FUNCTION(spl_directory) REGISTER_SPL_CLASS_CONST_LONG(SplFileObject, "SKIP_EMPTY", SPL_FILE_OBJECT_SKIP_EMPTY); REGISTER_SPL_CLASS_CONST_LONG(SplFileObject, "READ_CSV", SPL_FILE_OBJECT_READ_CSV); - REGISTER_SPL_SUB_CLASS_EX(SplTempFileObject, SplFileObject, spl_filesystem_object_constru_check_new, spl_SplTempFileObject_functions); + REGISTER_SPL_SUB_CLASS_EX(SplTempFileObject, SplFileObject, spl_filesystem_object_new, spl_SplTempFileObject_functions); return SUCCESS; } /* }}} */ diff --git a/ext/spl/tests/bug54384.phpt b/ext/spl/tests/bug54384.phpt index 92b9bfea64..5f82dccc12 100644 --- a/ext/spl/tests/bug54384.phpt +++ b/ext/spl/tests/bug54384.phpt @@ -132,42 +132,46 @@ $o->rewind(); } ); - -function test2($f) { - try { - $f(); - echo "ran normally (expected)\n"; - } catch (LogicException $e) { - echo "exception (unexpected)\n\n"; - } -} - echo "SplFileInfo... "; class SplFileInfoTest extends SplFileInfo { function __construct(){} } -test2 ( function() { +test ( function() { $o = new SplFileInfoTest; -/* handles with fatal error */ -/* echo $o->getMTime(), " : "; */ +$o->getMTime(); } ); echo "DirectoryIterator... "; class DirectoryIteratortest extends DirectoryIterator { function __construct(){} } -test2 ( function() { +test ( function() { $o = new DirectoryIteratorTest; foreach ($o as $a) { echo $a,"\n"; } } ); +echo "DirectoryIterator (exception cleared)... "; +class DirectoryIteratorTest2 extends DirectoryIterator { + function __construct(){ + try { + parent::__construct(); + } catch (Exception $e) { } + } +} +test ( function() { +$o = new DirectoryIteratorTest2; +foreach ($o as $a) { +echo $a,"\n"; +} +} ); + echo "FileSystemIterator... "; class FileSystemIteratorTest extends DirectoryIterator { function __construct(){} } -test2 ( function() { +test ( function() { $o = new FileSystemIteratorTest; foreach ($o as $a) { echo $a,"\n"; @@ -178,12 +182,69 @@ echo "RecursiveDirectoryIterator... "; class RecursiveDirectoryIteratorTest extends RecursiveDirectoryIterator { function __construct(){} } -test2 ( function() { +test ( function() { $o = new RecursiveDirectoryIteratorTest; foreach ($o as $a) { echo $a,"\n"; } } ); + +echo "RecursiveTreeIterator... "; +class RecursiveTreeIteratorTest extends RecursiveDirectoryIterator { + function __construct(){} +} +test ( function() { +$o = new RecursiveTreeIteratorTest; +foreach ($o as $a) { +echo $a,"\n"; +} +} ); + +echo "AppendIterator... "; +class AppendIteratorTest extends AppendIterator { + function __construct(){} +} +test ( function() { +$o = new AppendIteratorTest; +foreach ($o as $a) { +echo $a,"\n"; +} +} ); + +echo "InfiniteIterator... "; +class InfiniteIteratorTest extends InfiniteIterator { + function __construct(){} +} +test ( function() { +$o = new InfiniteIteratorTest; +foreach ($o as $a) { +echo $a,"\n"; +} +} ); + +echo "CallbackFilterIterator... "; +class CallbackFilterIteratorTest extends CallbackFilterIterator { + function __construct(){} +} +test ( function() { +$o = new CallbackFilterIteratorTest; +foreach ($o as $a) { +echo $a,"\n"; +} +} ); + +echo "RecursiveCallbackFilterIterator... "; +class RecursiveCallbackFilterIteratorTest extends RecursiveCallbackFilterIterator { + function __construct(){} +} +test ( function() { +$o = new RecursiveCallbackFilterIteratorTest; +foreach ($o as $a) { +echo $a,"\n"; +} +} ); + + --EXPECT-- IteratorIterator... exception (expected) FilterIterator... exception (expected) @@ -198,7 +259,13 @@ RecursiveRegexIterator... exception (expected) GlobIterator... exception (expected) SplFileObject... exception (expected) SplTempFileObject... exception (expected) -SplFileInfo... ran normally (expected) -DirectoryIterator... ran normally (expected) -FileSystemIterator... ran normally (expected) -RecursiveDirectoryIterator... ran normally (expected) +SplFileInfo... exception (expected) +DirectoryIterator... exception (expected) +DirectoryIterator (exception cleared)... exception (expected) +FileSystemIterator... exception (expected) +RecursiveDirectoryIterator... exception (expected) +RecursiveTreeIterator... exception (expected) +AppendIterator... exception (expected) +InfiniteIterator... exception (expected) +CallbackFilterIterator... exception (expected) +RecursiveCallbackFilterIterator... exception (expected) -- 2.40.0