cataphract                               Sun, 27 Mar 2011 22:44:34 +0000

Revision: http://svn.php.net/viewvc?view=revision&revision=309751

Log:
- 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.

Bug: http://bugs.php.net/54384 (Assigned) Several SPL classes crash when parent 
constructor is not called
      
Changed paths:
    U   php/php-src/trunk/UPGRADING
    U   php/php-src/trunk/ext/spl/php_spl.c
    U   php/php-src/trunk/ext/spl/spl_directory.c
    U   php/php-src/trunk/ext/spl/tests/bug54384.phpt

Modified: php/php-src/trunk/UPGRADING
===================================================================
--- php/php-src/trunk/UPGRADING	2011-03-27 20:24:40 UTC (rev 309750)
+++ php/php-src/trunk/UPGRADING	2011-03-27 22:44:34 UTC (rev 309751)
@@ -184,6 +184,18 @@
   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

Modified: php/php-src/trunk/ext/spl/php_spl.c
===================================================================
--- php/php-src/trunk/ext/spl/php_spl.c	2011-03-27 20:24:40 UTC (rev 309750)
+++ php/php-src/trunk/ext/spl/php_spl.c	2011-03-27 22:44:34 UTC (rev 309751)
@@ -66,7 +66,7 @@

 	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 @@
 	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 @@
 		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 @@
 			"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);
 	}

Modified: php/php-src/trunk/ext/spl/spl_directory.c
===================================================================
--- php/php-src/trunk/ext/spl/spl_directory.c	2011-03-27 20:24:40 UTC (rev 309750)
+++ php/php-src/trunk/ext/spl/spl_directory.c	2011-03-27 22:44:34 UTC (rev 309751)
@@ -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 @@
    - 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 @@
 	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,16 +160,10 @@
 /* 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);
+	return spl_filesystem_object_new_ex(class_type, 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);
-}
-/* }}} */
-
 PHPAPI char* spl_filesystem_object_get_path(spl_filesystem_object *intern, int *len TSRMLS_DC) /* {{{ */
 {
 #ifdef HAVE_GLOB
@@ -320,7 +310,7 @@
 	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 @@

 	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 @@

 		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 @@

 		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 @@
 	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 @@
 	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 @@
 	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;
 }
 /* }}} */

Modified: php/php-src/trunk/ext/spl/tests/bug54384.phpt
===================================================================
--- php/php-src/trunk/ext/spl/tests/bug54384.phpt	2011-03-27 20:24:40 UTC (rev 309750)
+++ php/php-src/trunk/ext/spl/tests/bug54384.phpt	2011-03-27 22:44:34 UTC (rev 309751)
@@ -132,42 +132,46 @@
 } );


-
-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 @@
 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 @@
 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)
-- 
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php

Reply via email to