merged branch char101/master (PR #844)

This PR was merged into the master branch.

Commits
-------

c23ef25 Add assertEquals to NativeExtensionTest.php
9126dc6 Twig extension: fix case when accessing property of an array casted into object
34cf8e1 Fix double free
4980903 Enhancements for twig extension
db3cb80 Fix NativeExtensionTest
3485ee7 Native extension: handle dynamic properties defined in the get_properties handler in a per instance fashion.

Discussion
----------

Native extension: call get_properties in per instance manner instead of caching it.

Since dynamic properties of an object can be defined by its get_properties handler, we need to call it for each instance.

---------------------------------------------------------------------------

by char101 at 2012-09-21T10:30:37Z

PHPUnit test result

```
PHPUnit 3.7.1 by Sebastian Bergmann.

.S...........................................................   61 / 1253 (  4%)
.............................................................  122 / 1253 (  9%)
.............................................................  183 / 1253 ( 14%)
.............................................................  244 / 1253 ( 19%)
.............................................................  305 / 1253 ( 24%)
.............................................................  366 / 1253 ( 29%)
.............................................................  427 / 1253 ( 34%)
.............................................................  488 / 1253 ( 38%)
.............................................................  549 / 1253 ( 43%)
.............................................................  610 / 1253 ( 48%)
.............................................................  671 / 1253 ( 53%)
.............................................................  732 / 1253 ( 58%)
.............................................................  793 / 1253 ( 63%)
.............................................................  854 / 1253 ( 68%)
.............................................................  915 / 1253 ( 73%)
.............................................................  976 / 1253 ( 77%)
............................................................. 1037 / 1253 ( 82%)
............................................................. 1098 / 1253 ( 87%)
............................................................. 1159 / 1253 ( 92%)
............................................................. 1220 / 1253 ( 97%)
.................................

Time: 4 seconds, Memory: 13.25Mb

OK, but incomplete or skipped tests!
Tests: 1253, Assertions: 2969, Skipped: 1.
```

---------------------------------------------------------------------------

by stof at 2012-09-21T11:58:51Z

@char101 My previous comment about the way the test should be implemented is still valid. Please rewrite it to use the same way to all other integration tests in Twig

---------------------------------------------------------------------------

by char101 at 2012-09-23T04:51:36Z

@stof I don't see the reason of using a fixture. The test case works, it accomplishes its goal. It's simple. It doesn't test for a feature, it tests for a specific case where PHP crashes.

---------------------------------------------------------------------------

by stof at 2012-09-23T13:53:41Z

@char101 I see one: you are building a Twig instance and rendering a template here, which is exactely what the integration tests are doing.
Btw, your test would fail when running phpunit in strict mode as it does not assert anything

---------------------------------------------------------------------------

by char101 at 2012-09-24T02:05:58Z

@stof I don't have the desire to change what isn't broken, but you are free to change it as you see fit. As for the assert, I have added it to the test.
This commit is contained in:
Fabien Potencier
2012-09-28 23:45:13 +02:00
2 changed files with 53 additions and 28 deletions
+24 -28
View File
@@ -317,6 +317,19 @@ int TWIG_HAS_PROPERTY(zval *object, zval *propname TSRMLS_DC)
return 0;
}
int TWIG_HAS_DYNAMIC_PROPERTY(zval *object, char *prop, int prop_len TSRMLS_DC)
{
if (Z_OBJ_HT_P(object)->get_properties) {
return zend_hash_quick_exists(
Z_OBJ_HT_P(object)->get_properties(object TSRMLS_CC), // the properties hash
prop, // property name
prop_len + 1, // property length
zend_get_hash_value(prop, prop_len + 1) // hash value
);
}
return 0;
}
zval *TWIG_PROPERTY_CHAR(zval *object, char *propname TSRMLS_DC)
{
zval *tmp_name_zval, *tmp;
@@ -563,7 +576,7 @@ char *TWIG_GET_CLASS_NAME(zval *object TSRMLS_DC)
if (Z_TYPE_P(object) != IS_OBJECT) {
return "";
}
zend_get_object_classname(object, &class_name, &class_name_len TSRMLS_CC);
zend_get_object_classname(object, (const char **) &class_name, &class_name_len TSRMLS_CC);
return class_name;
}
@@ -605,33 +618,13 @@ static int twig_add_property_to_class(void *pDest APPLY_TSRMLS_DC, int num_args,
ce = *va_arg(args, zend_class_entry**);
retval = va_arg(args, zval*);
zend_unmangle_property_name(pptr->name, pptr->name_length, &class_name, &prop_name);
zend_unmangle_property_name(pptr->name, pptr->name_length, (const char **) &class_name, (const char **) &prop_name);
add_assoc_string(retval, prop_name, prop_name, 1);
return 0;
}
/* {{{ _adddynproperty */
static int twig_add_dyn_property_to_class(void *pDest APPLY_TSRMLS_DC, int num_args, va_list args, zend_hash_key *hash_key)
{
APPLY_TSRMLS_FETCH();
zend_class_entry *ce = *va_arg(args, zend_class_entry**);
zval *retval = va_arg(args, zval*), member;
char *class_name, *prop_name;
if (hash_key->nKeyLength < 1 || hash_key->arKey[0] == '\0') {
return 0; /* non public cannot be dynamic */
}
ZVAL_STRINGL(&member, hash_key->arKey, hash_key->nKeyLength-1, 0);
if (zend_get_property_info(ce, &member, 1 TSRMLS_CC) == &EG(std_property_info)) {
zend_unmangle_property_name((&EG(std_property_info))->name, (&EG(std_property_info))->name_length, &class_name, &prop_name);
add_assoc_string(retval, prop_name, prop_name, 1);
}
return 0;
}
static void twig_add_class_to_cache(zval *cache, zval *object, char *class_name TSRMLS_DC)
{
zval *class_info, *class_methods, *class_properties;
@@ -649,10 +642,6 @@ static void twig_add_class_to_cache(zval *cache, zval *object, char *class_name
zend_hash_apply_with_arguments(&class_ce->function_table APPLY_TSRMLS_CC, twig_add_method_to_class, 1, class_methods);
zend_hash_apply_with_arguments(&class_ce->properties_info APPLY_TSRMLS_CC, twig_add_property_to_class, 2, &class_ce, class_properties);
if (object && Z_OBJ_HT_P(object)->get_properties) {
HashTable *properties = Z_OBJ_HT_P(object)->get_properties(object TSRMLS_CC);
zend_hash_apply_with_arguments(properties APPLY_TSRMLS_CC, twig_add_dyn_property_to_class, 2, &class_ce, class_properties);
}
add_assoc_zval(class_info, "methods", class_methods);
add_assoc_zval(class_info, "properties", class_properties);
add_assoc_zval(cache, class_name, class_info);
@@ -853,8 +842,7 @@ PHP_FUNCTION(twig_template_get_attributes)
efree(class_name);
if (tmp_item || TWIG_HAS_PROPERTY(object, &zitem TSRMLS_CC) || TWIG_ARRAY_KEY_EXISTS(object, item, item_len) // FIXME: Array key? is that array access here?
) {
if (tmp_item || TWIG_HAS_PROPERTY(object, &zitem TSRMLS_CC) || TWIG_HAS_DYNAMIC_PROPERTY(object, item, item_len TSRMLS_CC)) {
if (isDefinedTest) {
RETURN_TRUE;
}
@@ -925,6 +913,10 @@ PHP_FUNCTION(twig_template_get_attributes)
}
*/
} else {
efree(tmp_method_name_get);
efree(tmp_method_name_is);
efree(lcItem);
if (isDefinedTest) {
RETURN_FALSE;
}
@@ -934,6 +926,7 @@ PHP_FUNCTION(twig_template_get_attributes)
TWIG_THROW_EXCEPTION("Twig_Error_Runtime" TSRMLS_CC, "Method \"%s\" for object \"%s\" does not exist", item, TWIG_GET_CLASS_NAME(object TSRMLS_CC));
return;
}
if (isDefinedTest) {
efree(tmp_method_name_get);
efree(tmp_method_name_is);
@@ -949,6 +942,9 @@ PHP_FUNCTION(twig_template_get_attributes)
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);
efree(tmp_method_name_is);
efree(lcItem);
return;
}
/*