From 01fc2ab69bf5b8b71e629b21beb065fcacc0856a Mon Sep 17 00:00:00 2001 From: "yangguo@chromium.org" Date: Mon, 14 Apr 2014 14:03:20 +0000 Subject: [PATCH] Allow allocation and GC in access check callbacks. R=ishell@chromium.org Review URL: https://codereview.chromium.org/234913003 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@20730 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/handles.cc | 7 +-- src/isolate.cc | 142 +++++++++++++++++++++++------------------------- src/isolate.h | 24 ++------ src/objects.cc | 80 ++++++++++++++------------- src/runtime.cc | 59 +++++++++----------- test/cctest/test-api.cc | 113 ++++++++++++++++++++++++++++---------- 6 files changed, 229 insertions(+), 196 deletions(-) diff --git a/src/handles.cc b/src/handles.cc index f70848f..a316cc0 100644 --- a/src/handles.cc +++ b/src/handles.cc @@ -458,10 +458,9 @@ MaybeHandle GetKeysInFixedArrayFor(Handle object, // Check access rights if required. if (current->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(current, - isolate->factory()->undefined_value(), - v8::ACCESS_KEYS)) { - isolate->ReportFailedAccessCheckWrapper(current, v8::ACCESS_KEYS); + !isolate->MayNamedAccess( + current, isolate->factory()->undefined_value(), v8::ACCESS_KEYS)) { + isolate->ReportFailedAccessCheck(current, v8::ACCESS_KEYS); RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, FixedArray); break; } diff --git a/src/isolate.cc b/src/isolate.cc index 3b38422..0c108fb 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -693,28 +693,41 @@ void Isolate::SetFailedAccessCheckCallback( } -void Isolate::ReportFailedAccessCheck(JSObject* receiver, v8::AccessType type) { +static inline AccessCheckInfo* GetAccessCheckInfo(Isolate* isolate, + Handle receiver) { + JSFunction* constructor = JSFunction::cast(receiver->map()->constructor()); + if (!constructor->shared()->IsApiFunction()) return NULL; + + Object* data_obj = + constructor->shared()->get_api_func_data()->access_check_info(); + if (data_obj == isolate->heap()->undefined_value()) return NULL; + + return AccessCheckInfo::cast(data_obj); +} + + +void Isolate::ReportFailedAccessCheck(Handle receiver, + v8::AccessType type) { if (!thread_local_top()->failed_access_check_callback_) return; ASSERT(receiver->IsAccessCheckNeeded()); ASSERT(context()); // Get the data object from access check info. - JSFunction* constructor = JSFunction::cast(receiver->map()->constructor()); - if (!constructor->shared()->IsApiFunction()) return; - Object* data_obj = - constructor->shared()->get_api_func_data()->access_check_info(); - if (data_obj == heap_.undefined_value()) return; - HandleScope scope(this); - Handle receiver_handle(receiver); - Handle data(AccessCheckInfo::cast(data_obj)->data(), this); - { VMState state(this); - thread_local_top()->failed_access_check_callback_( - v8::Utils::ToLocal(receiver_handle), + Handle data; + { DisallowHeapAllocation no_gc; + AccessCheckInfo* access_check_info = GetAccessCheckInfo(this, receiver); + if (!access_check_info) return; + data = handle(access_check_info->data(), this); + } + + // Leaving JavaScript. + VMState state(this); + thread_local_top()->failed_access_check_callback_( + v8::Utils::ToLocal(receiver), type, v8::Utils::ToLocal(data)); - } } @@ -724,13 +737,14 @@ enum MayAccessDecision { static MayAccessDecision MayAccessPreCheck(Isolate* isolate, - JSObject* receiver, + Handle receiver, v8::AccessType type) { + DisallowHeapAllocation no_gc; // During bootstrapping, callback functions are not enabled yet. if (isolate->bootstrapper()->IsActive()) return YES; if (receiver->IsJSGlobalProxy()) { - Object* receiver_context = JSGlobalProxy::cast(receiver)->native_context(); + Object* receiver_context = JSGlobalProxy::cast(*receiver)->native_context(); if (!receiver_context->IsContext()) return NO; // Get the native context of current top context. @@ -748,16 +762,14 @@ static MayAccessDecision MayAccessPreCheck(Isolate* isolate, } -bool Isolate::MayNamedAccess(JSObject* receiver, Object* key, +bool Isolate::MayNamedAccess(Handle receiver, + Handle key, v8::AccessType type) { ASSERT(receiver->IsJSGlobalProxy() || receiver->IsAccessCheckNeeded()); - // The callers of this method are not expecting a GC. - DisallowHeapAllocation no_gc; - // Skip checks for hidden properties access. Note, we do not // require existence of a context in this case. - if (key == heap_.hidden_string()) return true; + if (key.is_identical_to(factory()->hidden_string())) return true; // Check for compatibility between the security tokens in the // current lexical context and the accessed object. @@ -766,39 +778,30 @@ bool Isolate::MayNamedAccess(JSObject* receiver, Object* key, MayAccessDecision decision = MayAccessPreCheck(this, receiver, type); if (decision != UNKNOWN) return decision == YES; - // Get named access check callback - JSFunction* constructor = JSFunction::cast(receiver->map()->constructor()); - if (!constructor->shared()->IsApiFunction()) return false; - - Object* data_obj = - constructor->shared()->get_api_func_data()->access_check_info(); - if (data_obj == heap_.undefined_value()) return false; - - Object* fun_obj = AccessCheckInfo::cast(data_obj)->named_callback(); - v8::NamedSecurityCallback callback = - v8::ToCData(fun_obj); - - if (!callback) return false; - HandleScope scope(this); - Handle receiver_handle(receiver, this); - Handle key_handle(key, this); - Handle data(AccessCheckInfo::cast(data_obj)->data(), this); - LOG(this, ApiNamedSecurityCheck(key)); - bool result = false; - { - // Leaving JavaScript. - VMState state(this); - result = callback(v8::Utils::ToLocal(receiver_handle), - v8::Utils::ToLocal(key_handle), - type, - v8::Utils::ToLocal(data)); + Handle data; + v8::NamedSecurityCallback callback; + { DisallowHeapAllocation no_gc; + AccessCheckInfo* access_check_info = GetAccessCheckInfo(this, receiver); + if (!access_check_info) return false; + Object* fun_obj = access_check_info->named_callback(); + callback = v8::ToCData(fun_obj); + if (!callback) return false; + data = handle(access_check_info->data(), this); } - return result; + + LOG(this, ApiNamedSecurityCheck(*key)); + + // Leaving JavaScript. + VMState state(this); + return callback(v8::Utils::ToLocal(receiver), + v8::Utils::ToLocal(key), + type, + v8::Utils::ToLocal(data)); } -bool Isolate::MayIndexedAccess(JSObject* receiver, +bool Isolate::MayIndexedAccess(Handle receiver, uint32_t index, v8::AccessType type) { ASSERT(receiver->IsJSGlobalProxy() || receiver->IsAccessCheckNeeded()); @@ -809,34 +812,25 @@ bool Isolate::MayIndexedAccess(JSObject* receiver, MayAccessDecision decision = MayAccessPreCheck(this, receiver, type); if (decision != UNKNOWN) return decision == YES; - // Get indexed access check callback - JSFunction* constructor = JSFunction::cast(receiver->map()->constructor()); - if (!constructor->shared()->IsApiFunction()) return false; - - Object* data_obj = - constructor->shared()->get_api_func_data()->access_check_info(); - if (data_obj == heap_.undefined_value()) return false; - - Object* fun_obj = AccessCheckInfo::cast(data_obj)->indexed_callback(); - v8::IndexedSecurityCallback callback = - v8::ToCData(fun_obj); - - if (!callback) return false; - HandleScope scope(this); - Handle receiver_handle(receiver, this); - Handle data(AccessCheckInfo::cast(data_obj)->data(), this); - LOG(this, ApiIndexedSecurityCheck(index)); - bool result = false; - { - // Leaving JavaScript. - VMState state(this); - result = callback(v8::Utils::ToLocal(receiver_handle), - index, - type, - v8::Utils::ToLocal(data)); + Handle data; + v8::IndexedSecurityCallback callback; + { DisallowHeapAllocation no_gc; + // Get named access check callback + AccessCheckInfo* access_check_info = GetAccessCheckInfo(this, receiver); + if (!access_check_info) return false; + Object* fun_obj = access_check_info->indexed_callback(); + callback = v8::ToCData(fun_obj); + if (!callback) return false; + data = handle(access_check_info->data(), this); } - return result; + + LOG(this, ApiIndexedSecurityCheck(index)); + + // Leaving JavaScript. + VMState state(this); + return callback( + v8::Utils::ToLocal(receiver), index, type, v8::Utils::ToLocal(data)); } diff --git a/src/isolate.h b/src/isolate.h index 7035b06..3801901 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -785,31 +785,15 @@ class Isolate { // the result is false, the pending exception is guaranteed to be // set. - // TODO(yangguo): temporary wrappers - bool MayNamedAccessWrapper(Handle receiver, - Handle key, - v8::AccessType type) { - return MayNamedAccess(*receiver, *key, type); - } - bool MayIndexedAccessWrapper(Handle receiver, - uint32_t index, - v8::AccessType type) { - return MayIndexedAccess(*receiver, index, type); - } - void ReportFailedAccessCheckWrapper(Handle receiver, - v8::AccessType type) { - ReportFailedAccessCheck(*receiver, type); - } - - bool MayNamedAccess(JSObject* receiver, - Object* key, + bool MayNamedAccess(Handle receiver, + Handle key, v8::AccessType type); - bool MayIndexedAccess(JSObject* receiver, + bool MayIndexedAccess(Handle receiver, uint32_t index, v8::AccessType type); void SetFailedAccessCheckCallback(v8::FailedAccessCheckCallback callback); - void ReportFailedAccessCheck(JSObject* receiver, v8::AccessType type); + void ReportFailedAccessCheck(Handle receiver, v8::AccessType type); // Exception throwing support. The caller should use the result // of Throw() as its return value. diff --git a/src/objects.cc b/src/objects.cc index eb5728a..d4116b3 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -520,7 +520,7 @@ MaybeHandle JSObject::GetPropertyWithFailedAccessCheck( // No accessible property found. *attributes = ABSENT; - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_GET); + isolate->ReportFailedAccessCheck(object, v8::ACCESS_GET); RETURN_HANDLE_IF_SCHEDULED_EXCEPTION(isolate, Object); return isolate->factory()->undefined_value(); } @@ -584,7 +584,8 @@ PropertyAttributes JSObject::GetPropertyAttributeWithFailedAccessCheck( } } - object->GetIsolate()->ReportFailedAccessCheckWrapper(object, v8::ACCESS_HAS); + object->GetIsolate()->ReportFailedAccessCheck(object, v8::ACCESS_HAS); + // TODO(yangguo): Issue 3269, check for scheduled exception missing? return ABSENT; } @@ -785,7 +786,7 @@ MaybeHandle Object::GetProperty(Handle object, // property from the current object, we still check that we have // access to it. Handle checked = Handle::cast(current); - if (!isolate->MayNamedAccessWrapper(checked, name, v8::ACCESS_GET)) { + if (!isolate->MayNamedAccess(checked, name, v8::ACCESS_GET)) { return JSObject::GetPropertyWithFailedAccessCheck( checked, receiver, result, name, attributes); } @@ -881,8 +882,8 @@ MaybeHandle Object::GetElementWithReceiver(Isolate* isolate, // Check access rights if needed. if (js_object->IsAccessCheckNeeded()) { - if (!isolate->MayIndexedAccessWrapper(js_object, index, v8::ACCESS_GET)) { - isolate->ReportFailedAccessCheckWrapper(js_object, v8::ACCESS_GET); + if (!isolate->MayIndexedAccess(js_object, index, v8::ACCESS_GET)) { + isolate->ReportFailedAccessCheck(js_object, v8::ACCESS_GET); RETURN_HANDLE_IF_SCHEDULED_EXCEPTION(isolate, Object); return isolate->factory()->undefined_value(); } @@ -3377,7 +3378,7 @@ MaybeHandle JSObject::SetPropertyWithFailedAccessCheck( } Isolate* isolate = object->GetIsolate(); - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_SET); + isolate->ReportFailedAccessCheck(object, v8::ACCESS_SET); RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object); return value; } @@ -3927,7 +3928,7 @@ MaybeHandle JSObject::SetPropertyForResult( // Check access rights if needed. if (object->IsAccessCheckNeeded()) { - if (!isolate->MayNamedAccessWrapper(object, name, v8::ACCESS_SET)) { + if (!isolate->MayNamedAccess(object, name, v8::ACCESS_SET)) { return SetPropertyWithFailedAccessCheck(object, lookup, name, value, true, strict_mode); } @@ -4067,7 +4068,7 @@ MaybeHandle JSObject::SetLocalPropertyIgnoreAttributes( // Check access rights if needed. if (object->IsAccessCheckNeeded()) { - if (!isolate->MayNamedAccessWrapper(object, name, v8::ACCESS_SET)) { + if (!isolate->MayNamedAccess(object, name, v8::ACCESS_SET)) { return SetPropertyWithFailedAccessCheck(object, &lookup, name, value, false, SLOPPY); } @@ -4259,7 +4260,7 @@ PropertyAttributes JSReceiver::GetPropertyAttributeForResult( if (object->IsAccessCheckNeeded()) { Heap* heap = object->GetHeap(); Handle obj = Handle::cast(object); - if (!heap->isolate()->MayNamedAccessWrapper(obj, name, v8::ACCESS_HAS)) { + if (!heap->isolate()->MayNamedAccess(obj, name, v8::ACCESS_HAS)) { return JSObject::GetPropertyAttributeWithFailedAccessCheck( obj, lookup, name, continue_search); } @@ -4312,8 +4313,9 @@ PropertyAttributes JSObject::GetElementAttributeWithReceiver( // Check access rights if needed. if (object->IsAccessCheckNeeded()) { - if (!isolate->MayIndexedAccessWrapper(object, index, v8::ACCESS_HAS)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_HAS); + if (!isolate->MayIndexedAccess(object, index, v8::ACCESS_HAS)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_HAS); + // TODO(yangguo): Issue 3269, check for scheduled exception missing? return ABSENT; } } @@ -5189,8 +5191,8 @@ MaybeHandle JSObject::DeleteElement(Handle object, // Check access rights if needed. if (object->IsAccessCheckNeeded() && - !isolate->MayIndexedAccessWrapper(object, index, v8::ACCESS_DELETE)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_DELETE); + !isolate->MayIndexedAccess(object, index, v8::ACCESS_DELETE)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_DELETE); RETURN_HANDLE_IF_SCHEDULED_EXCEPTION(isolate, Object); return factory->false_value(); } @@ -5258,8 +5260,8 @@ MaybeHandle JSObject::DeleteProperty(Handle object, // Check access rights if needed. if (object->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(object, name, v8::ACCESS_DELETE)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_DELETE); + !isolate->MayNamedAccess(object, name, v8::ACCESS_DELETE)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_DELETE); RETURN_HANDLE_IF_SCHEDULED_EXCEPTION(isolate, Object); return isolate->factory()->false_value(); } @@ -5488,10 +5490,9 @@ Handle JSObject::PreventExtensions(Handle object) { if (!object->map()->is_extensible()) return object; if (object->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(object, - isolate->factory()->undefined_value(), - v8::ACCESS_KEYS)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_KEYS); + !isolate->MayNamedAccess( + object, isolate->factory()->undefined_value(), v8::ACCESS_KEYS)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_KEYS); RETURN_HANDLE_IF_SCHEDULED_EXCEPTION(isolate, Object); return isolate->factory()->false_value(); } @@ -5572,10 +5573,9 @@ MaybeHandle JSObject::Freeze(Handle object) { Isolate* isolate = object->GetIsolate(); if (object->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(object, - isolate->factory()->undefined_value(), - v8::ACCESS_KEYS)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_KEYS); + !isolate->MayNamedAccess( + object, isolate->factory()->undefined_value(), v8::ACCESS_KEYS)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_KEYS); RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object); return isolate->factory()->false_value(); } @@ -6264,7 +6264,7 @@ void JSObject::DefinePropertyAccessor(Handle object, bool JSObject::CanSetCallback(Handle object, Handle name) { Isolate* isolate = object->GetIsolate(); ASSERT(!object->IsAccessCheckNeeded() || - isolate->MayNamedAccessWrapper(object, name, v8::ACCESS_SET)); + isolate->MayNamedAccess(object, name, v8::ACCESS_SET)); // Check if there is an API defined callback object which prohibits // callback overwriting in this object or its prototype chain. @@ -6387,8 +6387,9 @@ void JSObject::DefineAccessor(Handle object, Isolate* isolate = object->GetIsolate(); // Check access rights if needed. if (object->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(object, name, v8::ACCESS_SET)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_SET); + !isolate->MayNamedAccess(object, name, v8::ACCESS_SET)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_SET); + // TODO(yangguo): Issue 3269, check for scheduled exception missing? return; } @@ -6571,8 +6572,8 @@ Handle JSObject::SetAccessor(Handle object, // Check access rights if needed. if (object->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(object, name, v8::ACCESS_SET)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_SET); + !isolate->MayNamedAccess(object, name, v8::ACCESS_SET)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_SET); RETURN_HANDLE_IF_SCHEDULED_EXCEPTION(isolate, Object); return factory->undefined_value(); } @@ -6657,8 +6658,8 @@ Handle JSObject::GetAccessor(Handle object, // Check access rights if needed. if (object->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(object, name, v8::ACCESS_HAS)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_HAS); + !isolate->MayNamedAccess(object, name, v8::ACCESS_HAS)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_HAS); RETURN_HANDLE_IF_SCHEDULED_EXCEPTION(isolate, Object); return isolate->factory()->undefined_value(); } @@ -12512,8 +12513,8 @@ MaybeHandle JSObject::SetElement(Handle object, // Check access rights if needed. if (object->IsAccessCheckNeeded()) { - if (!isolate->MayIndexedAccessWrapper(object, index, v8::ACCESS_SET)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_SET); + if (!isolate->MayIndexedAccess(object, index, v8::ACCESS_SET)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_SET); RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, Object); return value; } @@ -13291,8 +13292,9 @@ bool JSObject::HasRealNamedProperty(Handle object, SealHandleScope shs(isolate); // Check access rights if needed. if (object->IsAccessCheckNeeded()) { - if (!isolate->MayNamedAccessWrapper(object, key, v8::ACCESS_HAS)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_HAS); + if (!isolate->MayNamedAccess(object, key, v8::ACCESS_HAS)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_HAS); + // TODO(yangguo): Issue 3269, check for scheduled exception missing? return false; } } @@ -13308,8 +13310,9 @@ bool JSObject::HasRealElementProperty(Handle object, uint32_t index) { HandleScope scope(isolate); // Check access rights if needed. if (object->IsAccessCheckNeeded()) { - if (!isolate->MayIndexedAccessWrapper(object, index, v8::ACCESS_HAS)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_HAS); + if (!isolate->MayIndexedAccess(object, index, v8::ACCESS_HAS)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_HAS); + // TODO(yangguo): Issue 3269, check for scheduled exception missing? return false; } } @@ -13333,8 +13336,9 @@ bool JSObject::HasRealNamedCallbackProperty(Handle object, SealHandleScope shs(isolate); // Check access rights if needed. if (object->IsAccessCheckNeeded()) { - if (!isolate->MayNamedAccessWrapper(object, key, v8::ACCESS_HAS)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_HAS); + if (!isolate->MayNamedAccess(object, key, v8::ACCESS_HAS)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_HAS); + // TODO(yangguo): Issue 3269, check for scheduled exception missing? return false; } } diff --git a/src/runtime.cc b/src/runtime.cc index 3c5d1c6..54cd3a0 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -1717,11 +1717,11 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetPrototype) { ASSERT(!obj->IsAccessCheckNeeded() || obj->IsJSObject()); do { if (obj->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(Handle::cast(obj), - isolate->factory()->proto_string(), - v8::ACCESS_GET)) { - isolate->ReportFailedAccessCheckWrapper(Handle::cast(obj), - v8::ACCESS_GET); + !isolate->MayNamedAccess(Handle::cast(obj), + isolate->factory()->proto_string(), + v8::ACCESS_GET)) { + isolate->ReportFailedAccessCheck(Handle::cast(obj), + v8::ACCESS_GET); RETURN_IF_SCHEDULED_EXCEPTION(isolate); return isolate->heap()->undefined_value(); } @@ -1749,10 +1749,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_SetPrototype) { CONVERT_ARG_HANDLE_CHECKED(JSObject, obj, 0); CONVERT_ARG_HANDLE_CHECKED(Object, prototype, 1); if (obj->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(obj, - isolate->factory()->proto_string(), - v8::ACCESS_SET)) { - isolate->ReportFailedAccessCheckWrapper(obj, v8::ACCESS_SET); + !isolate->MayNamedAccess( + obj, isolate->factory()->proto_string(), v8::ACCESS_SET)) { + isolate->ReportFailedAccessCheck(obj, v8::ACCESS_SET); RETURN_IF_SCHEDULED_EXCEPTION(isolate); return isolate->heap()->undefined_value(); } @@ -1851,11 +1850,11 @@ static AccessCheckResult CheckPropertyAccess(Handle obj, if (name->AsArrayIndex(&index)) { // TODO(1095): we should traverse hidden prototype hierachy as well. if (CheckGenericAccess( - obj, obj, index, access_type, &Isolate::MayIndexedAccessWrapper)) { + obj, obj, index, access_type, &Isolate::MayIndexedAccess)) { return ACCESS_ALLOWED; } - obj->GetIsolate()->ReportFailedAccessCheckWrapper(obj, access_type); + obj->GetIsolate()->ReportFailedAccessCheck(obj, access_type); return ACCESS_FORBIDDEN; } @@ -1866,7 +1865,7 @@ static AccessCheckResult CheckPropertyAccess(Handle obj, if (!lookup.IsProperty()) return ACCESS_ABSENT; Handle holder(lookup.holder(), isolate); if (CheckGenericAccess >( - obj, holder, name, access_type, &Isolate::MayNamedAccessWrapper)) { + obj, holder, name, access_type, &Isolate::MayNamedAccess)) { return ACCESS_ALLOWED; } @@ -1894,7 +1893,7 @@ static AccessCheckResult CheckPropertyAccess(Handle obj, break; } - isolate->ReportFailedAccessCheckWrapper(obj, access_type); + isolate->ReportFailedAccessCheck(obj, access_type); return ACCESS_FORBIDDEN; } @@ -5793,10 +5792,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetLocalPropertyNames) { if (obj->IsJSGlobalProxy()) { // Only collect names if access is permitted. if (obj->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(obj, - isolate->factory()->undefined_value(), - v8::ACCESS_KEYS)) { - isolate->ReportFailedAccessCheckWrapper(obj, v8::ACCESS_KEYS); + !isolate->MayNamedAccess( + obj, isolate->factory()->undefined_value(), v8::ACCESS_KEYS)) { + isolate->ReportFailedAccessCheck(obj, v8::ACCESS_KEYS); RETURN_IF_SCHEDULED_EXCEPTION(isolate); return *isolate->factory()->NewJSArray(0); } @@ -5813,10 +5811,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_GetLocalPropertyNames) { for (int i = 0; i < length; i++) { // Only collect names if access is permitted. if (jsproto->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(jsproto, - isolate->factory()->undefined_value(), - v8::ACCESS_KEYS)) { - isolate->ReportFailedAccessCheckWrapper(jsproto, v8::ACCESS_KEYS); + !isolate->MayNamedAccess( + jsproto, isolate->factory()->undefined_value(), v8::ACCESS_KEYS)) { + isolate->ReportFailedAccessCheck(jsproto, v8::ACCESS_KEYS); RETURN_IF_SCHEDULED_EXCEPTION(isolate); return *isolate->factory()->NewJSArray(0); } @@ -5966,10 +5963,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_LocalKeys) { if (object->IsJSGlobalProxy()) { // Do access checks before going to the global object. if (object->IsAccessCheckNeeded() && - !isolate->MayNamedAccessWrapper(object, - isolate->factory()->undefined_value(), - v8::ACCESS_KEYS)) { - isolate->ReportFailedAccessCheckWrapper(object, v8::ACCESS_KEYS); + !isolate->MayNamedAccess( + object, isolate->factory()->undefined_value(), v8::ACCESS_KEYS)) { + isolate->ReportFailedAccessCheck(object, v8::ACCESS_KEYS); RETURN_IF_SCHEDULED_EXCEPTION(isolate); return *isolate->factory()->NewJSArray(0); } @@ -14865,9 +14861,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_IsAccessAllowedForObserver) { Handle key = args.at(2); SaveContext save(isolate); isolate->set_context(observer->context()); - if (!isolate->MayNamedAccessWrapper(object, - isolate->factory()->undefined_value(), - v8::ACCESS_KEYS)) { + if (!isolate->MayNamedAccess( + object, isolate->factory()->undefined_value(), v8::ACCESS_KEYS)) { return isolate->heap()->false_value(); } bool access_allowed = false; @@ -14875,12 +14870,12 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_IsAccessAllowedForObserver) { if (key->ToArrayIndex(&index) || (key->IsString() && String::cast(*key)->AsArrayIndex(&index))) { access_allowed = - isolate->MayIndexedAccessWrapper(object, index, v8::ACCESS_GET) && - isolate->MayIndexedAccessWrapper(object, index, v8::ACCESS_HAS); + isolate->MayIndexedAccess(object, index, v8::ACCESS_GET) && + isolate->MayIndexedAccess(object, index, v8::ACCESS_HAS); } else { access_allowed = - isolate->MayNamedAccessWrapper(object, key, v8::ACCESS_GET) && - isolate->MayNamedAccessWrapper(object, key, v8::ACCESS_HAS); + isolate->MayNamedAccess(object, key, v8::ACCESS_GET) && + isolate->MayNamedAccess(object, key, v8::ACCESS_HAS); } return isolate->heap()->ToBoolean(access_allowed); } diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc index 1da3cd5..3f6351c 100644 --- a/test/cctest/test-api.cc +++ b/test/cctest/test-api.cc @@ -8252,34 +8252,6 @@ THREADED_TEST(TypeSwitch) { } -// For use within the TestSecurityHandler() test. -static bool g_security_callback_result = false; -static bool NamedSecurityTestCallback(Local global, - Local name, - v8::AccessType type, - Local data) { - // Always allow read access. - if (type == v8::ACCESS_GET) - return true; - - // Sometimes allow other access. - return g_security_callback_result; -} - - -static bool IndexedSecurityTestCallback(Local global, - uint32_t key, - v8::AccessType type, - Local data) { - // Always allow read access. - if (type == v8::ACCESS_GET) - return true; - - // Sometimes allow other access. - return g_security_callback_result; -} - - static int trouble_nesting = 0; static void TroubleCallback(const v8::FunctionCallbackInfo& args) { ApiTestFuzzer::Fuzz(); @@ -8406,6 +8378,36 @@ TEST(TryCatchFinallyUsingTryCatchHandler) { } +// For use within the TestSecurityHandler() test. +static bool g_security_callback_result = false; +static bool NamedSecurityTestCallback(Local global, + Local name, + v8::AccessType type, + Local data) { + printf("a\n"); + // Always allow read access. + if (type == v8::ACCESS_GET) + return true; + + // Sometimes allow other access. + return g_security_callback_result; +} + + +static bool IndexedSecurityTestCallback(Local global, + uint32_t key, + v8::AccessType type, + Local data) { + printf("b\n"); + // Always allow read access. + if (type == v8::ACCESS_GET) + return true; + + // Sometimes allow other access. + return g_security_callback_result; +} + + // SecurityHandler can't be run twice TEST(SecurityHandler) { v8::Isolate* isolate = CcTest::isolate(); @@ -8577,6 +8579,61 @@ THREADED_TEST(SecurityChecksForPrototypeChain) { } +static bool named_security_check_with_gc_called; + +static bool NamedSecurityCallbackWithGC(Local global, + Local name, + v8::AccessType type, + Local data) { + CcTest::heap()->CollectAllGarbage(i::Heap::kNoGCFlags); + named_security_check_with_gc_called = true; + return true; +} + + +static bool indexed_security_check_with_gc_called; + +static bool IndexedSecurityTestCallbackWithGC(Local global, + uint32_t key, + v8::AccessType type, + Local data) { + CcTest::heap()->CollectAllGarbage(i::Heap::kNoGCFlags); + indexed_security_check_with_gc_called = true; + return true; +} + + +TEST(SecurityTestGCAllowed) { + v8::Isolate* isolate = CcTest::isolate(); + v8::HandleScope handle_scope(isolate); + v8::Handle object_template = + v8::ObjectTemplate::New(isolate); + object_template->SetAccessCheckCallbacks(NamedSecurityCallbackWithGC, + IndexedSecurityTestCallbackWithGC); + + v8::Handle context = Context::New(isolate); + v8::Context::Scope context_scope(context); + + context->Global()->Set(v8_str("obj"), object_template->NewInstance()); + + named_security_check_with_gc_called = false; + CompileRun("obj.foo = new String(1001);"); + CHECK(named_security_check_with_gc_called); + + indexed_security_check_with_gc_called = false; + CompileRun("obj[0] = new String(1002);"); + CHECK(indexed_security_check_with_gc_called); + + named_security_check_with_gc_called = false; + CHECK(CompileRun("obj.foo")->ToString()->Equals(v8_str("1001"))); + CHECK(named_security_check_with_gc_called); + + indexed_security_check_with_gc_called = false; + CHECK(CompileRun("obj[0]")->ToString()->Equals(v8_str("1002"))); + CHECK(indexed_security_check_with_gc_called); +} + + THREADED_TEST(CrossDomainDelete) { LocalContext env1; v8::HandleScope handle_scope(env1->GetIsolate()); -- 2.7.4