From 6399b49b3f12d6b5a93c7d20f4ead0c0ee143d4a Mon Sep 17 00:00:00 2001 From: Stefan Siegl Date: Sun, 30 Nov 2014 21:00:42 +0100 Subject: [PATCH] Improve ArrayAccess enumeration When enumerating an ArrayAccess-style object the array keys should be returned, not the method names & properties of the PHP object. --- tests/array_access_006.phpt | 63 +++++++++++++++++++++ v8js_array_access.cc | 107 ++++++++++++++++++++++++++++++++---- v8js_array_access.h | 2 + v8js_object_export.cc | 12 +++- 4 files changed, 172 insertions(+), 12 deletions(-) create mode 100644 tests/array_access_006.phpt diff --git a/tests/array_access_006.phpt b/tests/array_access_006.phpt new file mode 100644 index 0000000..95cdf9c --- /dev/null +++ b/tests/array_access_006.phpt @@ -0,0 +1,63 @@ +--TEST-- +Test V8::executeString() : Enumerate ArrayAccess keys +--SKIPIF-- + +--INI-- +v8js.use_array_access = 1 +--FILE-- +data[$offset]); + } + + public function offsetGet($offset) { + return $this->data[$offset]; + } + + public function offsetSet($offset, $value) { + echo "set[$offset] = $value\n"; + $this->data[$offset] = $value; + } + + public function offsetUnset($offset) { + throw new Exception('Not implemented'); + } + + public function count() { + return count($this->data); + } +} + +$v8 = new V8Js(); +$v8->myarr = new MyArray(); + +$js = <<executeString($js); + +?> +===EOF=== +--EXPECT-- +string(1) "0" +string(1) "1" +string(1) "2" +string(1) "4" +string(1) "0" +string(1) "1" +string(1) "2" +string(1) "4" +===EOF=== diff --git a/v8js_array_access.cc b/v8js_array_access.cc index 8fad0d8..22d1630 100644 --- a/v8js_array_access.cc +++ b/v8js_array_access.cc @@ -131,15 +131,9 @@ void php_v8js_array_access_setter(uint32_t index, v8::Local value, } /* }}} */ -void php_v8js_array_access_length(v8::Local property, const v8::PropertyCallbackInfo& info) /* {{{ */ + +static int php_v8js_array_access_get_count_result(zval *object TSRMLS_DC) /* {{{ */ { - v8::Isolate *isolate = info.GetIsolate(); - v8::Local self = info.Holder(); - - V8JS_TSRMLS_FETCH(); - - v8::Local php_object = self->GetHiddenValue(V8JS_SYM(PHPJS_OBJECT_KEY)); - zval *object = reinterpret_cast(v8::External::Cast(*php_object)->Value()); zend_class_entry *ce = Z_OBJCE_P(object); zend_fcall_info fci; @@ -163,13 +157,106 @@ void php_v8js_array_access_length(v8::Local property, const v8::Prop zend_call_function(&fci, NULL TSRMLS_CC); - v8::Local ret_value = zval_to_v8js(php_value, isolate TSRMLS_CC); + if(Z_TYPE_P(php_value) != IS_LONG) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-numeric return value from count() method"); + return 0; + } + + int result = Z_LVAL_P(php_value); zval_ptr_dtor(&php_value); - info.GetReturnValue().Set(ret_value); + return result; } /* }}} */ +static bool php_v8js_array_access_isset_p(zval *object, int index TSRMLS_DC) /* {{{ */ +{ + zend_class_entry *ce = Z_OBJCE_P(object); + + /* Okay, let's call offsetExists. */ + zend_fcall_info fci; + zval *php_value; + + zval fmember; + INIT_ZVAL(fmember); + ZVAL_STRING(&fmember, "offsetExists", 0); + + zval zindex; + INIT_ZVAL(zindex); + ZVAL_LONG(&zindex, index); + + fci.size = sizeof(fci); + fci.function_table = &ce->function_table; + fci.function_name = &fmember; + fci.symbol_table = NULL; + fci.retval_ptr_ptr = &php_value; + + zval *zindex_ptr = &zindex; + zval **zindex_ptr_ptr = &zindex_ptr; + fci.param_count = 1; + fci.params = &zindex_ptr_ptr; + + fci.object_ptr = object; + fci.no_separation = 0; + + zend_call_function(&fci, NULL TSRMLS_CC); + + if(Z_TYPE_P(php_value) != IS_BOOL) { + php_error_docref(NULL TSRMLS_CC, E_WARNING, "Non-boolean return value from offsetExists() method"); + return false; + } + + bool result = Z_LVAL_P(php_value); + zval_ptr_dtor(&php_value); + + return result; +} +/* }}} */ + + +void php_v8js_array_access_length(v8::Local property, const v8::PropertyCallbackInfo& info) /* {{{ */ +{ + v8::Isolate *isolate = info.GetIsolate(); + v8::Local self = info.Holder(); + + V8JS_TSRMLS_FETCH(); + + v8::Local php_object = self->GetHiddenValue(V8JS_SYM(PHPJS_OBJECT_KEY)); + zval *object = reinterpret_cast(v8::External::Cast(*php_object)->Value()); + + int length = php_v8js_array_access_get_count_result(object TSRMLS_CC); + info.GetReturnValue().Set(V8JS_INT(length)); +} +/* }}} */ + +void php_v8js_array_access_enumerator(const v8::PropertyCallbackInfo& info) /* {{{ */ +{ + v8::Isolate *isolate = info.GetIsolate(); + v8::Local self = info.Holder(); + + V8JS_TSRMLS_FETCH(); + + v8::Local php_object = self->GetHiddenValue(V8JS_SYM(PHPJS_OBJECT_KEY)); + zval *object = reinterpret_cast(v8::External::Cast(*php_object)->Value()); + + int length = php_v8js_array_access_get_count_result(object TSRMLS_CC); + v8::Local result = v8::Array::New(isolate, length); + + int i = 0; + + for(int j = 0; j < length; j ++) { + if(php_v8js_array_access_isset_p(object, j TSRMLS_CC)) { + result->Set(i ++, V8JS_INT(j)); + } + } + + result->Set(V8JS_STR("length"), V8JS_INT(i)); + info.GetReturnValue().Set(result); +} +/* }}} */ + + + void php_v8js_array_access_named_getter(v8::Local property, const v8::PropertyCallbackInfo &info) /* {{{ */ { v8::String::Utf8Value cstr(property); diff --git a/v8js_array_access.h b/v8js_array_access.h index af4a0f9..812a197 100644 --- a/v8js_array_access.h +++ b/v8js_array_access.h @@ -20,6 +20,8 @@ void php_v8js_array_access_setter(uint32_t index, v8::Local value, const v8::PropertyCallbackInfo& info); void php_v8js_array_access_length(v8::Local property, const v8::PropertyCallbackInfo& info); +void php_v8js_array_access_enumerator(const v8::PropertyCallbackInfo& info); + /* Named Property Handlers */ void php_v8js_array_access_named_getter(v8::Local property, diff --git a/v8js_object_export.cc b/v8js_object_export.cc index eb57688..2d72391 100644 --- a/v8js_object_export.cc +++ b/v8js_object_export.cc @@ -866,6 +866,7 @@ static v8::Handle php_v8js_wrap_object(v8::Isolate *isolate, zend_cl v8::Local inst_tpl = new_tpl->InstanceTemplate(); v8::NamedPropertyGetterCallback getter = php_v8js_named_property_getter; + v8::NamedPropertyEnumeratorCallback enumerator = php_v8js_named_property_enumerator; /* Check for ArrayAccess object */ if (V8JSG(use_array_access) && ce) { @@ -883,12 +884,19 @@ static v8::Handle php_v8js_wrap_object(v8::Isolate *isolate, zend_cl if(has_array_access && has_countable) { inst_tpl->SetIndexedPropertyHandler(php_v8js_array_access_getter, - php_v8js_array_access_setter); + php_v8js_array_access_setter, + 0, /* query */ + 0, /* deleter */ + php_v8js_array_access_enumerator); /* Switch to special ArrayAccess getter, which falls back to * php_v8js_named_property_getter, but possibly bridges the * call to Array.prototype functions. */ getter = php_v8js_array_access_named_getter; + + /* Overwrite enumerator, since for(... in ...) loop should + * not see the methods but iterate over the elements. */ + enumerator = 0; } } @@ -899,7 +907,7 @@ static v8::Handle php_v8js_wrap_object(v8::Isolate *isolate, zend_cl php_v8js_named_property_setter, /* setter */ php_v8js_named_property_query, /* query */ php_v8js_named_property_deleter, /* deleter */ - php_v8js_named_property_enumerator, /* enumerator */ + enumerator, /* enumerator */ V8JS_NULL /* data */ ); // add __invoke() handler