merged branch Tobion/patch-1 (PR #1030)

This PR was merged into the master branch. Discussion ---------- fix numeric & boolean & float keys in array Reopened PR of #878 Commits ------- 2c56b70 Merge pull request #3 from tucksaun/patch-1 17d6cc7 [ext] fixed tabs 46012ce [ext] improvements 21f8062 [ext] code cleaning 9347c52 [ext] updated PHP code documenting C code b41ce60 updated C extension accordingly e5291e1 rename item to attribute in the exception message because that's the term that is used in twig, e.g. the function a93f0dc further improved exception message and distinguish array_call and any_call access 26bac14 added tests for array access with confusable keys db744d5 fix test that that would not fail if no exception is thrown 9ea16ec refactored getAttribute 6fb5afa improved error message for non-existent or invalid attributes fdfd506 added changelog entry about fixed boolean array access da616c5 fixed getAttribute for array access with a boolean or float key b6acf9d updated CHANGELOG c4d5b3f added some unit tests for previous merge e84448b fix numeric keys in array
2026-09-21 07:37:43 +00:00 · 2013-05-07 20:56:07 +02:00
parent 8503ff8101 2c56b706e2
commit cbbbab9065
4 changed files with 208 additions and 122 deletions
@@ -76,12 +76,29 @@ zend_module_entry twig_module_entry = {
 ZEND_GET_MODULE(twig)
 #endif

-int TWIG_ARRAY_KEY_EXISTS(zval *array, char* key, int key_len)
+int TWIG_ARRAY_KEY_EXISTS(zval *array, zval *key)
 {
+	zval temp;
+	int result;
+
 	if (Z_TYPE_P(array) != IS_ARRAY) {
 		return 0;
 	}
-	return zend_symtable_exists(Z_ARRVAL_P(array), key, key_len + 1);
+
+	switch (Z_TYPE_P(key)) {
+		case IS_NULL:
+			return zend_hash_exists(Z_ARRVAL_P(array), "", 1);
+
+		case IS_BOOL:
+		case IS_DOUBLE:
+			convert_to_long(key);
+		case IS_LONG:
+			return zend_hash_index_exists(Z_ARRVAL_P(array), Z_LVAL_P(key));
+
+		default:
+			convert_to_string(key);
+			return zend_symtable_exists(Z_ARRVAL_P(array), Z_STRVAL_P(key), Z_STRLEN_P(key) + 1);
+	}
 }

 int TWIG_INSTANCE_OF(zval *object, zend_class_entry *interface TSRMLS_DC)
@@ -106,23 +123,23 @@ int TWIG_INSTANCE_OF_USERLAND(zval *object, char *interface TSRMLS_DC)

 zval *TWIG_GET_ARRAYOBJECT_ELEMENT(zval *object, zval *offset TSRMLS_DC)
 {
-    zend_class_entry *ce = Z_OBJCE_P(object);
-    zval *retval;
+	zend_class_entry *ce = Z_OBJCE_P(object);
+	zval *retval;

 	if (Z_TYPE_P(object) == IS_OBJECT) {
 		SEPARATE_ARG_IF_REF(offset);
 		zend_call_method_with_1_params(&object, ce, NULL, "offsetget", &retval, offset);

-        zval_ptr_dtor(&offset);
+		zval_ptr_dtor(&offset);

-        if (!retval) {
-            if (!EG(exception)) {
-                zend_error(E_ERROR, "Undefined offset for object of type %s used as array", ce->name);
-            }
-            return NULL;
-        }
+		if (!retval) {
+			if (!EG(exception)) {
+				zend_error(E_ERROR, "Undefined offset for object of type %s used as array", ce->name);
+			}
+			return NULL;
+		}

-        return retval;
+		return retval;
 	}
 	return NULL;
 }
@@ -245,7 +262,7 @@ zval *TWIG_GET_ARRAY_ELEMENT_ZVAL(zval *class, zval *prop_name TSRMLS_DC)
 	zval **tmp_zval;
 	char *tmp_name;

-	if (class == NULL || Z_TYPE_P(class) != IS_ARRAY || Z_TYPE_P(prop_name) != IS_STRING) {
+	if (class == NULL || Z_TYPE_P(class) != IS_ARRAY) {
 		if (class != NULL && Z_TYPE_P(class) == IS_OBJECT && TWIG_INSTANCE_OF(class, zend_ce_arrayaccess TSRMLS_CC)) {
 			// array access object
 			return TWIG_GET_ARRAYOBJECT_ELEMENT(class, prop_name TSRMLS_CC);
@@ -253,11 +270,23 @@ zval *TWIG_GET_ARRAY_ELEMENT_ZVAL(zval *class, zval *prop_name TSRMLS_DC)
 		return NULL;
 	}

-	convert_to_string(prop_name);
-	tmp_name = Z_STRVAL_P(prop_name);
-	if (zend_symtable_find(HASH_OF(class), tmp_name, strlen(tmp_name)+1, (void**) &tmp_zval) == SUCCESS) {
-		return *tmp_zval;
+	switch(Z_TYPE_P(prop_name)) {
+		case IS_NULL:
+			zend_hash_find(HASH_OF(class), "", 1, (void**) &tmp_zval);
+			return *tmp_zval;
+
+		case IS_BOOL:
+		case IS_DOUBLE:
+			convert_to_long(prop_name);
+		case IS_LONG:
+			zend_hash_index_find(HASH_OF(class), Z_LVAL_P(prop_name), (void **) &tmp_zval);
+			return *tmp_zval;
+
+		case IS_STRING:
+			zend_symtable_find(HASH_OF(class), Z_STRVAL_P(prop_name), Z_STRLEN_P(prop_name) + 1, (void**) &tmp_zval);
+			return *tmp_zval;
 	}
+
 	return NULL;
 }

@@ -717,7 +746,7 @@ PHP_FUNCTION(twig_template_get_attributes)
 	zval *object;
 	char *item;
 	int  item_len;
-	zval  zitem;
+	zval *zitem, ztmpitem;
 	zval *arguments = NULL;
 	zval *ret = NULL;
 	char *type = NULL;
@@ -728,22 +757,17 @@ PHP_FUNCTION(twig_template_get_attributes)
 	zval *tmp_self_cache;


-	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ozs|asbb", &template, &object, &item, &item_len, &arguments, &type, &type_len, &isDefinedTest, &ignoreStrictCheck) == FAILURE) {
+	if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "ozz|asbb", &template, &object, &zitem, &arguments, &type, &type_len, &isDefinedTest, &ignoreStrictCheck) == FAILURE) {
 		return;
 	}

-	INIT_PZVAL(&zitem);
-	ZVAL_STRINGL(&zitem, item, item_len, 0);
-
-    switch (is_numeric_string(item, item_len, &Z_LVAL(zitem), &Z_DVAL(zitem), 0)) {
-    case IS_LONG:
-        Z_TYPE(zitem) = IS_LONG;
-        break;
-    case IS_DOUBLE:
-        Z_TYPE(zitem) = IS_DOUBLE;
-        convert_to_long(&zitem);
-        break;
-    }
+	// convert the item to a string
+	ztmpitem = *zitem;
+	zval_copy_ctor(&ztmpitem);
+	convert_to_string(&ztmpitem);
+	item_len = Z_STRLEN(ztmpitem);
+	item = estrndup(Z_STRVAL(ztmpitem), item_len);
+	zval_dtor(&ztmpitem);

 	if (!type) {
 		type = "any";
@@ -752,28 +776,31 @@ PHP_FUNCTION(twig_template_get_attributes)
 /*
 	// array
 	if (Twig_TemplateInterface::METHOD_CALL !== $type) {
-		if ((is_array($object) && array_key_exists($item, $object))
-			|| ($object instanceof ArrayAccess && isset($object[$item]))
+		$arrayItem = is_bool($item) || is_float($item) ? (int) $item : $item;
+
+		if ((is_array($object) && array_key_exists($arrayItem, $object))
+			|| ($object instanceof ArrayAccess && isset($object[$arrayItem]))
 		) {
 			if ($isDefinedTest) {
 				return true;
 			}

-			return $object[$item];
+			return $object[$arrayItem];
 		}
 */
+
+
 	if (strcmp("method", type) != 0) {
-//		printf("XXXmethod: %s\n", type);
-		if ((TWIG_ARRAY_KEY_EXISTS(object, item, item_len))
-			|| (TWIG_INSTANCE_OF(object, zend_ce_arrayaccess TSRMLS_CC) && TWIG_ISSET_ARRAYOBJECT_ELEMENT(object, &zitem TSRMLS_CC))
+		if ((TWIG_ARRAY_KEY_EXISTS(object, zitem))
+			|| (TWIG_INSTANCE_OF(object, zend_ce_arrayaccess TSRMLS_CC) && TWIG_ISSET_ARRAYOBJECT_ELEMENT(object, zitem TSRMLS_CC))
 		) {
-			zval *ret;

 			if (isDefinedTest) {
 				RETURN_TRUE;
 			}

-			ret = TWIG_GET_ARRAY_ELEMENT(object, item, item_len TSRMLS_CC);
+			ret = TWIG_GET_ARRAY_ELEMENT_ZVAL(object, zitem TSRMLS_CC);
+
 			if (!ret) {
 				ret = &EG(uninitialized_zval);
 			}
@@ -792,7 +819,7 @@ PHP_FUNCTION(twig_template_get_attributes)
 				return null;
 			}
 */
-		if (strcmp("array", type) == 0) {
+		if (strcmp("array", type) == 0 || Z_TYPE_P(object) != IS_OBJECT) {
 			if (isDefinedTest) {
 				RETURN_FALSE;
 			}
@@ -801,11 +828,13 @@ PHP_FUNCTION(twig_template_get_attributes)
 			}
 /*
 			if (is_object($object)) {
-				throw new Twig_Error_Runtime(sprintf('Key "%s" in object (with ArrayAccess) of type "%s" does not exist', $item, get_class($object)), -1, $this->getTemplateName());
+				throw new Twig_Error_Runtime(sprintf('Key "%s" in object (with ArrayAccess) of type "%s" does not exist', $arrayItem, get_class($object)), -1, $this->getTemplateName());
 			} elseif (is_array($object)) {
-				throw new Twig_Error_Runtime(sprintf('Key "%s" for array with keys "%s" does not exist', $item, implode(', ', array_keys($object))), -1, $this->getTemplateName());
+				throw new Twig_Error_Runtime(sprintf('Key "%s" for array with keys "%s" does not exist', $arrayItem, implode(', ', array_keys($object))), -1, $this->getTemplateName());
+			} elseif (Twig_TemplateInterface::ARRAY_CALL === $type) {
+				throw new Twig_Error_Runtime(sprintf('Impossible to access a key ("%s") on a %s variable ("%s")', $item, gettype($object), $object), -1, $this->getTemplateName());
 			} else {
-				throw new Twig_Error_Runtime(sprintf('Impossible to access a key ("%s") on a "%s" variable', $item, gettype($object)), -1, $this->getTemplateName());
+				throw new Twig_Error_Runtime(sprintf('Impossible to access an attribute ("%s") on a %s variable ("%s")', $item, gettype($object), $object), -1, $this->getTemplateName());
 			}
 		}
 	}
@@ -815,7 +844,15 @@ PHP_FUNCTION(twig_template_get_attributes)
 			} else if (Z_TYPE_P(object) == IS_ARRAY) {
 				TWIG_RUNTIME_ERROR(template TSRMLS_CC, "Key \"%s\" for array with keys \"%s\" does not exist", item, TWIG_IMPLODE_ARRAY_KEYS(", ", object TSRMLS_CC));
 			} else {
-				TWIG_RUNTIME_ERROR(template TSRMLS_CC, "Impossible to access a key (\"%s\") on a \"%s\" variable", item, zend_zval_type_name(object));
+				char *type_name = zend_zval_type_name(object);
+				Z_ADDREF_P(object);
+				convert_to_string(object);
+				TWIG_RUNTIME_ERROR(template TSRMLS_CC,
+					(strcmp("array", type) == 0)
+						? "Impossible to access a key (\"%s\") on a %s variable (\"%s\")"
+						: "Impossible to access an attribute (\"%s\") on a %s variable (\"%s\")",
+					item, type_name, Z_STRVAL_P(object));
+				zval_ptr_dtor(&object);
 			}
 			return;
 		}
@@ -836,58 +873,47 @@ PHP_FUNCTION(twig_template_get_attributes)
 		if ($ignoreStrictCheck || !$this->env->isStrictVariables()) {
 			return null;
 		}
-		throw new Twig_Error_Runtime(sprintf('Item "%s" for "%s" does not exist', $item, implode(', ', array_keys($object))));
+		throw new Twig_Error_Runtime(sprintf('Impossible to invoke a method ("%s") on a %s variable ("%s")', $item, gettype($object), $object), -1, $this->getTemplateName());
 	}
 */
 		if (ignoreStrictCheck || !TWIG_CALL_BOOLEAN(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "isStrictVariables" TSRMLS_CC)) {
-			RETURN_FALSE;
-		}
-		if (Z_TYPE_P(object) == IS_ARRAY) {
-			TWIG_RUNTIME_ERROR(template TSRMLS_CC, "Item \"%s\" for \"Array\" does not exist", item);
-		} else {
-			Z_ADDREF_P(object);
-			convert_to_string_ex(&object);
-			TWIG_RUNTIME_ERROR(template TSRMLS_CC, "Item \"%s\" for \"%s\" does not exist", item, Z_STRVAL_P(object));
-			zval_ptr_dtor(&object);
+			return;
 		}
+
+		char *type_name = zend_zval_type_name(object);
+		Z_ADDREF_P(object);
+		convert_to_string_ex(&object);
+
+		TWIG_RUNTIME_ERROR(template TSRMLS_CC, "Impossible to invoke a method (\"%s\") on a %s variable (\"%s\")", item, type_name, Z_STRVAL_P(object));
+
+		zval_ptr_dtor(&object);
+
 		return;
 	}
 /*
-	// get some information about the object
 	$class = get_class($object);
-	if (!isset(self::$cache[$class])) {
-		$r = new ReflectionClass($class);
-		self::$cache[$class] = array('methods' => array(), 'properties' => array());
-		foreach ($r->getMethods(ReflectionMethod::IS_PUBLIC) as $method) {
-			self::$cache[$class]['methods'][strtolower($method->getName())] = true;
-		}
-
-		foreach ($r->getProperties(ReflectionProperty::IS_PUBLIC) as $property) {
-			self::$cache[$class]['properties'][$property->getName()] = true;
-		}
-	}
 */
-	if (Z_TYPE_P(object) == IS_OBJECT) {
-		char *class_name = NULL;
+	char *class_name = NULL;
+	zval *tmp_class;

-		class_name = TWIG_GET_CLASS_NAME(object TSRMLS_CC);
-		tmp_self_cache = TWIG_GET_STATIC_PROPERTY(template, "cache" TSRMLS_CC);
+	class_name = TWIG_GET_CLASS_NAME(object TSRMLS_CC);
+	tmp_self_cache = TWIG_GET_STATIC_PROPERTY(template, "cache" TSRMLS_CC);
+	tmp_class = TWIG_GET_ARRAY_ELEMENT(tmp_self_cache, class_name, strlen(class_name) TSRMLS_CC);

-		if (!TWIG_GET_ARRAY_ELEMENT(tmp_self_cache, class_name, strlen(class_name) TSRMLS_CC)) {
-			twig_add_class_to_cache(tmp_self_cache, object, class_name TSRMLS_CC);
-		}
-		efree(class_name);
+	if (!tmp_class) {
+		twig_add_class_to_cache(tmp_self_cache, object, class_name TSRMLS_CC);
+		tmp_class = TWIG_GET_ARRAY_ELEMENT(tmp_self_cache, class_name, strlen(class_name) TSRMLS_CC);
 	}
+	efree(class_name);

 /*
 	// object property
 	if (Twig_TemplateInterface::METHOD_CALL !== $type) {
-		if (isset(self::$cache[$class]['properties'][$item])
-			|| isset($object->$item) || array_key_exists($item, $object)
-		) {
+		if (isset($object->$item) || array_key_exists((string) $item, $object)) {
 			if ($isDefinedTest) {
 				return true;
 			}
+
 			if ($this->env->hasExtension('sandbox')) {
 				$this->env->getExtension('sandbox')->checkPropertyAllowed($object, $item);
 			}
@@ -897,42 +923,41 @@ PHP_FUNCTION(twig_template_get_attributes)
 	}
 */
 	if (strcmp("method", type) != 0) {
-		zval *tmp_class, *tmp_properties, *tmp_item;
-		char *class_name = NULL;
+		zval *tmp_properties, *tmp_item;

-		class_name = TWIG_GET_CLASS_NAME(object TSRMLS_CC);
-		tmp_class = TWIG_GET_ARRAY_ELEMENT(tmp_self_cache, class_name, strlen(class_name) TSRMLS_CC);
 		tmp_properties = TWIG_GET_ARRAY_ELEMENT(tmp_class, "properties", strlen("properties") TSRMLS_CC);
 		tmp_item = TWIG_GET_ARRAY_ELEMENT(tmp_properties, item, item_len TSRMLS_CC);

-		efree(class_name);
-
-		if (tmp_item || TWIG_HAS_PROPERTY(object, &zitem TSRMLS_CC) || TWIG_HAS_DYNAMIC_PROPERTY(object, item, item_len TSRMLS_CC)) {
+		if (tmp_item || TWIG_HAS_PROPERTY(object, zitem TSRMLS_CC) || TWIG_HAS_DYNAMIC_PROPERTY(object, item, item_len TSRMLS_CC)) {
 			if (isDefinedTest) {
 				RETURN_TRUE;
 			}
 			if (TWIG_CALL_SB(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "hasExtension", "sandbox" TSRMLS_CC)) {
-				TWIG_CALL_ZZ(TWIG_CALL_S(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "getExtension", "sandbox" TSRMLS_CC), "checkPropertyAllowed", object, &zitem TSRMLS_CC);
+				TWIG_CALL_ZZ(TWIG_CALL_S(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "getExtension", "sandbox" TSRMLS_CC), "checkPropertyAllowed", object, zitem TSRMLS_CC);
 			}
 			if (EG(exception)) {
 				return;
 			}

-			ret = TWIG_PROPERTY(object, &zitem TSRMLS_CC);
+			ret = TWIG_PROPERTY(object, zitem TSRMLS_CC);
 			RETURN_ZVAL(ret, 1, 0);
 		}
 	}
 /*
 	// object method
+	if (!isset(self::$cache[$class]['methods'])) {
+		self::$cache[$class]['methods'] = array_change_key_case(array_flip(get_class_methods($object)));
+	}
+
 	$lcItem = strtolower($item);
 	if (isset(self::$cache[$class]['methods'][$lcItem])) {
-		$method = $item;
+		$method = (string) $item;
 	} elseif (isset(self::$cache[$class]['methods']['get'.$lcItem])) {
 		$method = 'get'.$item;
 	} elseif (isset(self::$cache[$class]['methods']['is'.$lcItem])) {
 		$method = 'is'.$item;
 	} elseif (isset(self::$cache[$class]['methods']['__call'])) {
-		$method = $item;
+		$method = (string) $item;
 */
 	{
 		char *lcItem = TWIG_STRTOLOWER(item, item_len);
@@ -940,10 +965,8 @@ PHP_FUNCTION(twig_template_get_attributes)
 		char *method = NULL;
 		char *tmp_method_name_get;
 		char *tmp_method_name_is;
-		zval *tmp_class, *tmp_methods;
-		char *class_name = NULL;
+		zval *tmp_methods;

-		class_name = TWIG_GET_CLASS_NAME(object TSRMLS_CC);
 		lcItem_length = strlen(lcItem);
 		tmp_method_name_get = emalloc(4 + lcItem_length);
 		tmp_method_name_is  = emalloc(3 + lcItem_length);
@@ -951,9 +974,7 @@ PHP_FUNCTION(twig_template_get_attributes)
 		sprintf(tmp_method_name_get, "get%s", lcItem);
 		sprintf(tmp_method_name_is, "is%s", lcItem);

-		tmp_class = TWIG_GET_ARRAY_ELEMENT(tmp_self_cache, class_name, strlen(class_name) TSRMLS_CC);
 		tmp_methods = TWIG_GET_ARRAY_ELEMENT(tmp_class, "methods", strlen("methods") TSRMLS_CC);
-		efree(class_name);

 		if (TWIG_GET_ARRAY_ELEMENT(tmp_methods, lcItem, lcItem_length TSRMLS_CC)) {
 			method = item;
@@ -968,11 +989,14 @@ PHP_FUNCTION(twig_template_get_attributes)
 		if ($isDefinedTest) {
 			return false;
 		}
+
 		if ($ignoreStrictCheck || !$this->env->isStrictVariables()) {
 			return null;
 		}
-		throw new Twig_Error_Runtime(sprintf('Method "%s" for object "%s" does not exist', $item, get_class($object)));
+
+		throw new Twig_Error_Runtime(sprintf('Method "%s" for object "%s" does not exist', $item, get_class($object)), -1, $this->getTemplateName());
 	}
+
 	if ($isDefinedTest) {
 		return true;
 	}
@@ -1004,7 +1028,7 @@ PHP_FUNCTION(twig_template_get_attributes)
 	}
 */
 		if (TWIG_CALL_SB(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "hasExtension", "sandbox" TSRMLS_CC)) {
-			TWIG_CALL_ZZ(TWIG_CALL_S(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "getExtension", "sandbox" TSRMLS_CC), "checkMethodAllowed", object, &zitem TSRMLS_CC);
+			TWIG_CALL_ZZ(TWIG_CALL_S(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "getExtension", "sandbox" TSRMLS_CC), "checkMethodAllowed", object, zitem TSRMLS_CC);
 		}
 		if (EG(exception)) {
 			efree(tmp_method_name_get);
@@ -1015,15 +1039,15 @@ PHP_FUNCTION(twig_template_get_attributes)
 /*
 	$ret = call_user_func_array(array($object, $method), $arguments);
 */
-		if (Z_TYPE_P(object) == IS_OBJECT) {
-			ret = TWIG_CALL_USER_FUNC_ARRAY(object, method, arguments TSRMLS_CC);
-			free_ret = 1;
-		}
+		ret = TWIG_CALL_USER_FUNC_ARRAY(object, method, arguments TSRMLS_CC);
+		free_ret = 1;
 		efree(tmp_method_name_get);
 		efree(tmp_method_name_is);
 		efree(lcItem);
 	}
 /*
+	// useful when calling a template method from a template
+	// this is not supported but unfortunately heavily used in the Symfony profiler
 	if ($object instanceof Twig_TemplateInterface) {
 		return $ret === '' ? '' : new Twig_Markup($ret, $this->env->getCharset());
 	}
@@ -1033,9 +1057,7 @@ PHP_FUNCTION(twig_template_get_attributes)
 	// ret can be null, if e.g. the called method throws an exception
 	if (ret) {
 		if (TWIG_INSTANCE_OF_USERLAND(object, "Twig_TemplateInterface" TSRMLS_CC)) {
-			if (Z_STRLEN_P(ret) == 0) {
-				free_ret = 1;
-			} else {
+			if (Z_STRLEN_P(ret) != 0) {
 				zval *charset = TWIG_CALL_USER_FUNC_ARRAY(TWIG_PROPERTY_CHAR(template, "env" TSRMLS_CC), "getCharset", NULL TSRMLS_CC);
 				TWIG_NEW(return_value, "Twig_Markup", ret, charset TSRMLS_CC);
 				zval_ptr_dtor(&charset);