fixed importing of existing static properties, new test was added

shlomohass · Sep 26, 2012 · 24afca0 · 24afca0
1 parent c37ad34
commit 24afca0
Show file tree

Hide file tree

Showing 6 changed files with 69 additions and 58 deletions.
diff --git a/package.xml b/package.xml
@@ -193,6 +193,7 @@ Execute code in restricted environment (sandboxing).
     <file name="runkit_import_with_syntax_error.phpt" role="test" />
     <file name="runkit_sandbox_with_register_globals.phpt" role="test" />
     <file name="runkit_static_property_add.phpt" role="test" />
+    <file name="runkit_static_property_add_existing.phpt" role="test" />
     <file name="runkit_static_property_add_to_subclasses.phpt" role="test" />
     <file name="runkit_superglobals.phpt" role="test" />
    </dir> <!-- //tests -->

diff --git a/runkit_import.c b/runkit_import.c
@@ -381,13 +381,13 @@ static int php_runkit_import_class_props(zend_class_entry *dce, zend_class_entry
 					                                   NULL TSRMLS_CC) != SUCCESS) {
 						php_error_docref(NULL TSRMLS_CC, E_WARNING,
 						                 "Unable to remove old property %s%s%s, not importing",
-						                 dce->name, (property_info_ptr->flags & ZEND_ACC_STATIC) ? ":: " : "->",
+						                 dce->name, (property_info_ptr->flags & ZEND_ACC_STATIC) ? "::$" : "->",
 						                 property_info_ptr->name);
 						goto import_st54_prop_skip;
 					}
 				} else {
-					php_error_docref(NULL TSRMLS_CC, E_NOTICE, "%s->%s already exists, not importing",
-					                 dce->name, key);
+					php_error_docref(NULL TSRMLS_CC, E_NOTICE, "%s%s%s already exists, not importing",
+					                 dce->name, (property_info_ptr->flags & ZEND_ACC_STATIC) ? "::$" : "->", key);
 					goto import_st54_prop_skip;
 				}
 			}

diff --git a/runkit_props.c b/runkit_props.c
@@ -115,22 +115,22 @@ int php_runkit_def_prop_add_int(zend_class_entry *ce, const char *propname, int
 #if PHP_MAJOR_VERSION >= 5
 	if (zend_hash_quick_find(&ce->properties_info, (char *) propname, propname_len + 1, h, (void*) &prop_info_ptr) == SUCCESS && !override) {
 		zval_ptr_dtor(&pcopyval);
-		php_error_docref(NULL TSRMLS_CC, E_NOTICE, "%s%s%s already exists, not importing",
-		                 ce->name, (prop_info_ptr->flags & ZEND_ACC_STATIC) ? "::" : "->", propname);
+		php_error_docref(NULL TSRMLS_CC, E_NOTICE, "%s%s%s already exists, not adding",
+		                 ce->name, (prop_info_ptr->flags & ZEND_ACC_STATIC) ? "::$" : "->", propname);
 		return FAILURE;
 	}
 #else
 	if (zend_hash_exists(&ce->default_properties, (char *) propname, propname_len + 1)) {
 		if (override) {
 			if (zend_hash_del(&ce->default_properties, (char *) propname, propname_len + 1) == FAILURE) {
 				zval_ptr_dtor(&pcopyval);
-				php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Failed to remove property %s->%s, not importing",
+				php_error_docref(NULL TSRMLS_CC, E_NOTICE, "Failed to remove property %s->%s, not adding",
 				                 ce->name, propname);
 				return FAILURE;
 			}
 		} else {
 			zval_ptr_dtor(&pcopyval);
-			php_error_docref(NULL TSRMLS_CC, E_NOTICE, "%s->%s already exists, not importing",
+			php_error_docref(NULL TSRMLS_CC, E_NOTICE, "%s->%s already exists, not adding",
 			                 ce->name, propname);
 			return FAILURE;
 		}
@@ -239,10 +239,7 @@ static int php_runkit_def_prop_add(char *classname, int classname_len, char *pro
 	char *key = propname;
 	int key_len = propname_len;
 #ifdef ZEND_ENGINE_2
-#if (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION <= 3)
-	char *temp;
-	int temp_len;
-#endif
+	zend_property_info *existing_prop;
 #endif
 
 	if (php_runkit_fetch_class_int(classname, classname_len, &ce TSRMLS_CC)==FAILURE) {
@@ -256,63 +253,31 @@ static int php_runkit_def_prop_add(char *classname, int classname_len, char *pro
 		copyval = value;
 	}
 
+#if PHP_MAJOR_VERSION >= 5
 	/* Check for existing property by this name */
 	/* Existing public? */
-#if (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION >= 4) || (PHP_MAJOR_VERSION >= 6)
-	if (zend_hash_exists(&ce->properties_info, key, key_len + 1)) {
-		zval_ptr_dtor(&copyval);
-		FREE_ZVAL(copyval);
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s::%s already exists", classname, propname);
-		return FAILURE;
-	}
-#else
-	if (zend_hash_exists(&ce->default_properties, key, key_len + 1)) {
-		zval_ptr_dtor(&copyval);
-		FREE_ZVAL(copyval);
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s::%s already exists", classname, propname);
+	if (zend_hash_find(&ce->properties_info, key, key_len + 1, (void *) &existing_prop) == SUCCESS) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s%s%s already exists", classname,
+		                 (existing_prop->flags & ZEND_ACC_STATIC) ? "::$" : "->", propname);
 		return FAILURE;
 	}
-#endif // (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION >= 4) || (PHP_MAJOR_VERSION >= 6)
-
-#if PHP_MAJOR_VERSION >= 5
 	if (
 	    Z_TYPE_P(copyval) == IS_CONSTANT_ARRAY
-#if RUNKIT_ABOVE53
+#	if RUNKIT_ABOVE53
 	    || (Z_TYPE_P(copyval) & IS_CONSTANT_TYPE_MASK) == IS_CONSTANT
-#endif
+#	endif
 	) {
 		zval_update_constant_ex(&copyval, (void*) 1, ce TSRMLS_CC);
 	}
-#endif
-
-#ifdef ZEND_ENGINE_2
-#if (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION <= 3)
-	/* Existing Protected? */
-	zend_mangle_property_name(&temp, &temp_len, "*", 1, propname, propname_len, 0);
-	if (zend_hash_exists(&ce->default_properties, temp, temp_len + 1)) {
-		zval_ptr_dtor(&copyval);
-		FREE_ZVAL(copyval);
-		efree(temp);
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s::%s already exists", classname, propname);
-		return FAILURE;
-	}
-	efree(temp);
-
-	/* Existing Private? */
-	zend_mangle_property_name(&temp, &temp_len, classname, classname_len, propname, propname_len, 0);
-	if (zend_hash_exists(&ce->default_properties, temp, temp_len + 1)) {
-		zval_ptr_dtor(&copyval);
-		FREE_ZVAL(copyval);
-		efree(temp);
-		php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s::%s already exists", classname, propname);
+#else
+	if (zend_hash_exists(&ce->default_properties, key, key_len + 1)) {
+		php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s->%s already exists", classname, propname);
 		return FAILURE;
 	}
-	efree(temp);
-#endif // (PHP_MAJOR_VERSION == 5 && PHP_MINOR_VERSION <= 3)
-#endif // ZEND_ENGINE_2
+#endif // PHP_MAJOR_VERSION >= 5
+
+
 	if (php_runkit_def_prop_add_int(ce, propname, propname_len, copyval, visibility, NULL, 0, ce, 0 TSRMLS_CC) != SUCCESS) {
-		zval_ptr_dtor(&copyval);
-		FREE_ZVAL(copyval);
 		return FAILURE;
 	}
 

diff --git a/tests/runkit_import_class_property_not_overriding_subclasses.phpt b/tests/runkit_import_class_property_not_overriding_subclasses.phpt
@@ -28,9 +28,9 @@ echo TestSub::$s, "\n";
 v
 s
 
-Notice: runkit_import(): TestSub::s already exists, not importing in %s on line %d
+Notice: runkit_import(): TestSub::$s already exists, not adding in %s on line %d
 
-Notice: runkit_import(): TestSub->v already exists, not importing in %s on line %d
+Notice: runkit_import(): TestSub->v already exists, not adding in %s on line %d
 v
 s
 IMPORTED: v

diff --git a/tests/runkit_import_class_property_not_overriding_subclasses4.phpt b/tests/runkit_import_class_property_not_overriding_subclasses4.phpt
@@ -23,6 +23,6 @@ echo $o->v, "\n";
 --EXPECTF--
 v
 
-Notice: runkit_import(): testsub->v already exists, not importing in %s on line %d
+Notice: runkit_import(): testsub->v already exists, not adding in %s on line %d
 v
 IMPORTED: v
diff --git a/tests/runkit_static_property_add_existing.phpt b/tests/runkit_static_property_add_existing.phpt
@@ -0,0 +1,45 @@
+--TEST--
+runkit_default_property_add() function for existing static properties
+--SKIPIF--
+<?php if(!extension_loaded("runkit") || !RUNKIT_FEATURE_MANIPULATION) print "skip";
+      if(array_shift(explode('.', PHP_VERSION)) < 5) print "skip";
+?>
+--INI--
+error_reporting=E_ALL
+display_errors=on
+--FILE--
+<?php
+class RunkitParent {
+}
+
+class RunkitClass extends RunkitParent {
+    public static $oldProperty = 'old';
+}
+
+class RunkitSubClass extends RunkitClass {
+}
+
+runkit_default_property_add('RunkitClass', 'oldProperty', array('a'=>1), RUNKIT_ACC_STATIC);
+var_dump(RunkitClass::$oldProperty);
+runkit_default_property_add('RunkitSubClass', 'oldProperty', array('a'=>1), RUNKIT_ACC_STATIC);
+var_dump(RunkitSubClass::$oldProperty);
+runkit_default_property_add('RunkitParent', 'oldProperty', array('a'=>1), RUNKIT_ACC_STATIC);
+var_dump(RunkitParent::$oldProperty);
+var_dump(RunkitClass::$oldProperty);
+var_dump(RunkitSubClass::$oldProperty);
+?>
+--EXPECTF--
+
+Warning: runkit_default_property_add(): RunkitClass::$oldProperty already exists in %s on line %d
+string(3) "old"
+
+Warning: runkit_default_property_add(): RunkitSubClass::$oldProperty already exists in %s on line %d
+string(3) "old"
+
+Notice: runkit_default_property_add(): RunkitClass::$oldProperty already exists, not adding in %s on line %d
+array(1) {
+  ["a"]=>
+  int(1)
+}
+string(3) "old"
+string(3) "old"