Revision: 15643
Author: [email protected]
Date: Fri Jul 12 04:42:07 2013
Log: Check for scheduled exceptions after a failed-access-check
callback.
[email protected]
BUG=v8:2524
Review URL: https://codereview.chromium.org/18298012
http://code.google.com/p/v8/source/detail?r=15643
Modified:
/branches/bleeding_edge/src/handles.cc
/branches/bleeding_edge/src/objects.cc
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/test/cctest/test-api.cc
=======================================
--- /branches/bleeding_edge/src/handles.cc Mon Jul 8 08:28:25 2013
+++ /branches/bleeding_edge/src/handles.cc Fri Jul 12 04:42:07 2013
@@ -650,6 +650,10 @@
isolate->heap()->undefined_value(),
v8::ACCESS_KEYS)) {
isolate->ReportFailedAccessCheck(*current, v8::ACCESS_KEYS);
+ if (isolate->has_scheduled_exception()) {
+ isolate->PromoteScheduledException();
+ *threw = true;
+ }
break;
}
=======================================
--- /branches/bleeding_edge/src/objects.cc Thu Jul 11 10:20:57 2013
+++ /branches/bleeding_edge/src/objects.cc Fri Jul 12 04:42:07 2013
@@ -551,7 +551,9 @@
// No accessible property found.
*attributes = ABSENT;
Heap* heap = name->GetHeap();
- heap->isolate()->ReportFailedAccessCheck(this, v8::ACCESS_GET);
+ Isolate* isolate = heap->isolate();
+ isolate->ReportFailedAccessCheck(this, v8::ACCESS_GET);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return heap->undefined_value();
}
@@ -925,6 +927,7 @@
Isolate* isolate = heap->isolate();
if (!isolate->MayIndexedAccess(js_object, index, v8::ACCESS_GET)) {
isolate->ReportFailedAccessCheck(js_object, v8::ACCESS_GET);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return heap->undefined_value();
}
}
@@ -3364,6 +3367,7 @@
HandleScope scope(isolate);
Handle<Object> value_handle(value, isolate);
isolate->ReportFailedAccessCheck(this, v8::ACCESS_SET);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return *value_handle;
}
@@ -5059,6 +5063,7 @@
if (IsAccessCheckNeeded() &&
!isolate->MayIndexedAccess(this, index, v8::ACCESS_DELETE)) {
isolate->ReportFailedAccessCheck(this, v8::ACCESS_DELETE);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return isolate->heap()->false_value();
}
@@ -5136,6 +5141,7 @@
if (IsAccessCheckNeeded() &&
!isolate->MayNamedAccess(this, name, v8::ACCESS_DELETE)) {
isolate->ReportFailedAccessCheck(this, v8::ACCESS_DELETE);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return isolate->heap()->false_value();
}
@@ -5366,6 +5372,7 @@
isolate->heap()->undefined_value(),
v8::ACCESS_KEYS)) {
isolate->ReportFailedAccessCheck(this, v8::ACCESS_KEYS);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return isolate->heap()->false_value();
}
@@ -5444,6 +5451,7 @@
heap->undefined_value(),
v8::ACCESS_KEYS)) {
isolate->ReportFailedAccessCheck(this, v8::ACCESS_KEYS);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return heap->false_value();
}
@@ -6257,6 +6265,7 @@
if (IsAccessCheckNeeded() &&
!isolate->MayNamedAccess(this, name, v8::ACCESS_SET)) {
isolate->ReportFailedAccessCheck(this, v8::ACCESS_SET);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return isolate->heap()->undefined_value();
}
@@ -6332,7 +6341,7 @@
}
-Object* JSObject::LookupAccessor(Name* name, AccessorComponent component) {
+MaybeObject* JSObject::LookupAccessor(Name* name, AccessorComponent
component) {
Heap* heap = GetHeap();
// Make sure that the top context does not change when doing callbacks or
@@ -6343,6 +6352,7 @@
if (IsAccessCheckNeeded() &&
!heap->isolate()->MayNamedAccess(this, name, v8::ACCESS_HAS)) {
heap->isolate()->ReportFailedAccessCheck(this, v8::ACCESS_HAS);
+ RETURN_IF_SCHEDULED_EXCEPTION(heap->isolate());
return heap->undefined_value();
}
@@ -12065,6 +12075,7 @@
if (IsAccessCheckNeeded()) {
if (!isolate->MayIndexedAccess(this, index, v8::ACCESS_SET)) {
isolate->ReportFailedAccessCheck(this, v8::ACCESS_SET);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return value_raw;
}
}
=======================================
--- /branches/bleeding_edge/src/objects.h Thu Jul 11 10:20:57 2013
+++ /branches/bleeding_edge/src/objects.h Fri Jul 12 04:42:07 2013
@@ -1944,7 +1944,7 @@
Handle<Object> setter,
PropertyAttributes attributes);
- Object* LookupAccessor(Name* name, AccessorComponent component);
+ MaybeObject* LookupAccessor(Name* name, AccessorComponent component);
MUST_USE_RESULT MaybeObject* DefineAccessor(AccessorInfo* info);
=======================================
--- /branches/bleeding_edge/src/runtime.cc Thu Jul 11 09:45:58 2013
+++ /branches/bleeding_edge/src/runtime.cc Fri Jul 12 04:42:07 2013
@@ -1456,6 +1456,7 @@
isolate->heap()->proto_string(),
v8::ACCESS_GET)) {
isolate->ReportFailedAccessCheck(JSObject::cast(obj),
v8::ACCESS_GET);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return isolate->heap()->undefined_value();
}
obj = obj->GetPrototype(isolate);
@@ -1558,21 +1559,6 @@
ACCESS_ALLOWED,
ACCESS_ABSENT
};
-
-
-static AccessCheckResult CheckElementAccess(
- JSObject* obj,
- uint32_t index,
- v8::AccessType access_type) {
- // TODO(1095): we should traverse hidden prototype hierachy as well.
- if (CheckGenericAccess(
- obj, obj, index, access_type, &Isolate::MayIndexedAccess)) {
- return ACCESS_ALLOWED;
- }
-
- obj->GetIsolate()->ReportFailedAccessCheck(obj, access_type);
- return ACCESS_FORBIDDEN;
-}
static AccessCheckResult CheckPropertyAccess(
@@ -1581,7 +1567,14 @@
v8::AccessType access_type) {
uint32_t index;
if (name->AsArrayIndex(&index)) {
- return CheckElementAccess(obj, index, access_type);
+ // TODO(1095): we should traverse hidden prototype hierachy as well.
+ if (CheckGenericAccess(
+ obj, obj, index, access_type, &Isolate::MayIndexedAccess)) {
+ return ACCESS_ALLOWED;
+ }
+
+ obj->GetIsolate()->ReportFailedAccessCheck(obj, access_type);
+ return ACCESS_FORBIDDEN;
}
LookupResult lookup(obj->GetIsolate());
@@ -1641,14 +1634,21 @@
Heap* heap = isolate->heap();
// Due to some WebKit tests, we want to make sure that we do not log
// more than one access failure here.
- switch (CheckPropertyAccess(*obj, *name, v8::ACCESS_HAS)) {
+ AccessCheckResult access_check_result =
+ CheckPropertyAccess(*obj, *name, v8::ACCESS_HAS);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
+ switch (access_check_result) {
case ACCESS_FORBIDDEN: return heap->false_value();
case ACCESS_ALLOWED: break;
case ACCESS_ABSENT: return heap->undefined_value();
}
PropertyAttributes attrs = obj->GetLocalPropertyAttribute(*name);
- if (attrs == ABSENT) return heap->undefined_value();
+ if (attrs == ABSENT) {
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
+ return heap->undefined_value();
+ }
+ ASSERT(!isolate->has_scheduled_exception());
AccessorPair* raw_accessors = obj->GetLocalPropertyAccessorPair(*name);
Handle<AccessorPair> accessors(raw_accessors, isolate);
@@ -1669,10 +1669,16 @@
Object* getter = accessors->GetComponent(ACCESSOR_GETTER);
Object* setter = accessors->GetComponent(ACCESSOR_SETTER);
if (!getter->IsMap() && CheckPropertyAccess(*obj, *name,
v8::ACCESS_GET)) {
+ ASSERT(!isolate->has_scheduled_exception());
elms->set(GETTER_INDEX, getter);
+ } else {
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
}
if (!setter->IsMap() && CheckPropertyAccess(*obj, *name,
v8::ACCESS_SET)) {
+ ASSERT(!isolate->has_scheduled_exception());
elms->set(SETTER_INDEX, setter);
+ } else {
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
}
}
@@ -4813,6 +4819,7 @@
bool fast = obj->HasFastProperties();
JSObject::DefineAccessor(obj, name, getter, setter, attr);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
if (fast) JSObject::TransformToFastProperties(obj, 0);
return isolate->heap()->undefined_value();
}
@@ -5339,9 +5346,9 @@
}
-static Object* HasLocalPropertyImplementation(Isolate* isolate,
- Handle<JSObject> object,
- Handle<Name> key) {
+static MaybeObject* HasLocalPropertyImplementation(Isolate* isolate,
+ Handle<JSObject> object,
+ Handle<Name> key) {
if (object->HasLocalProperty(*key)) return isolate->heap()->true_value();
// Handle hidden prototypes. If there's a hidden prototype above this
thing
// then we have to check it for properties, because they are supposed to
@@ -5353,6 +5360,7 @@
Handle<JSObject>::cast(proto),
key);
}
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return isolate->heap()->false_value();
}
@@ -5372,8 +5380,12 @@
// Fast case: either the key is a real named property or it is not
// an array index and there are no interceptors or hidden
// prototypes.
- if (object->HasRealNamedProperty(isolate, key))
+ if (object->HasRealNamedProperty(isolate, key)) {
+ ASSERT(!isolate->has_scheduled_exception());
return isolate->heap()->true_value();
+ } else {
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
+ }
Map* map = object->map();
if (!key_is_array_index &&
!map->has_named_interceptor() &&
@@ -5403,6 +5415,7 @@
CONVERT_ARG_CHECKED(Name, key, 1);
bool result = receiver->HasProperty(key);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
if (isolate->has_pending_exception()) return Failure::Exception();
return isolate->heap()->ToBoolean(result);
}
@@ -5415,6 +5428,7 @@
CONVERT_SMI_ARG_CHECKED(index, 1);
bool result = receiver->HasElement(index);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
if (isolate->has_pending_exception()) return Failure::Exception();
return isolate->heap()->ToBoolean(result);
}
@@ -5428,7 +5442,12 @@
CONVERT_ARG_CHECKED(Name, key, 1);
PropertyAttributes att = object->GetLocalPropertyAttribute(key);
- return isolate->heap()->ToBoolean(att != ABSENT && (att & DONT_ENUM) ==
0);
+ if (att == ABSENT || (att & DONT_ENUM) != 0) {
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
+ return isolate->heap()->false_value();
+ }
+ ASSERT(!isolate->has_scheduled_exception());
+ return isolate->heap()->true_value();
}
@@ -5506,6 +5525,7 @@
isolate->heap()->undefined_value(),
v8::ACCESS_KEYS)) {
isolate->ReportFailedAccessCheck(*obj, v8::ACCESS_KEYS);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return *isolate->factory()->NewJSArray(0);
}
obj = Handle<JSObject>(JSObject::cast(obj->GetPrototype()));
@@ -5525,6 +5545,7 @@
isolate->heap()->undefined_value(),
v8::ACCESS_KEYS)) {
isolate->ReportFailedAccessCheck(*jsproto, v8::ACCESS_KEYS);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return *isolate->factory()->NewJSArray(0);
}
int n;
@@ -5651,6 +5672,7 @@
!isolate->MayNamedAccess(*object,
isolate->heap()->undefined_value(),
v8::ACCESS_KEYS)) {
isolate->ReportFailedAccessCheck(*object, v8::ACCESS_KEYS);
+ RETURN_IF_SCHEDULED_EXCEPTION(isolate);
return *isolate->factory()->NewJSArray(0);
}
=======================================
--- /branches/bleeding_edge/test/cctest/test-api.cc Fri Jul 12 03:11:18 2013
+++ /branches/bleeding_edge/test/cctest/test-api.cc Fri Jul 12 04:42:07 2013
@@ -19787,12 +19787,59 @@
}
-TEST(Bug2778) {
- // Check that Object.observe includes access check.
- i::FLAG_harmony_observation = true;
+bool access_check_fail_thrown = false;
+bool catch_callback_called = false;
+
+
+// Failed access check callback that performs a GC on each invocation.
+void FailedAccessCheckThrows(Local<v8::Object> target,
+ v8::AccessType type,
+ Local<v8::Value> data) {
+ access_check_fail_thrown = true;
+ i::PrintF("Access check failed. Error thrown.\n");
+ v8::ThrowException(v8::Exception::Error(v8_str("cross context")));
+}
+
+
+void CatcherCallback(const v8::FunctionCallbackInfo<v8::Value>& args) {
+ for (int i = 0; i < args.Length(); i++) {
+ i::PrintF("%s\n", *String::Utf8Value(args[i]));
+ }
+ catch_callback_called = true;
+}
+
+
+void HasOwnPropertyCallback(const v8::FunctionCallbackInfo<v8::Value>&
args) {
+ args[0]->ToObject()->HasOwnProperty(args[1]->ToString());
+}
+
+
+void CheckCorrectThrow(const char* script) {
+ // Test that the script, when wrapped into a try-catch, triggers the
catch
+ // clause due to failed access check throwing an exception.
+ // The subsequent try-catch should run without any exception.
+ access_check_fail_thrown = false;
+ catch_callback_called = false;
+ i::ScopedVector<char> source(1024);
+ i::OS::SNPrintF(source, "try { %s; } catch (e) { catcher(e); }", script);
+ CompileRun(source.start());
+ CHECK(access_check_fail_thrown);
+ CHECK(catch_callback_called);
+
+ access_check_fail_thrown = false;
+ catch_callback_called = false;
+ CompileRun("try { [1, 2, 3].sort(); } catch (e) { catcher(e) };");
+ CHECK(!access_check_fail_thrown);
+ CHECK(!catch_callback_called);
+}
+
+
+TEST(AccessCheckThrows) {
+ i::FLAG_allow_natives_syntax = true;
v8::V8::Initialize();
- v8::Isolate* isolate = v8::Isolate::GetCurrent();
- v8::HandleScope scope(isolate);
+ v8::V8::SetFailedAccessCheckCallbackFunction(&FailedAccessCheckThrows);
+ v8::HandleScope scope(v8::Isolate::GetCurrent());
+
// Create an ObjectTemplate for global objects and install access
// check callbacks that will block access.
v8::Handle<v8::ObjectTemplate> global_template =
v8::ObjectTemplate::New();
@@ -19800,25 +19847,48 @@
IndexAccessAlwaysBlocked);
// Create a context and set an x property on it's global object.
- LocalContext outer_context(NULL, global_template);
- v8::Handle<v8::Object> outer_global = outer_context->Global();
- outer_global->Set(v8_str("x"), v8_num(42));
+ LocalContext context0(NULL, global_template);
+ context0->Global()->Set(v8_str("x"), v8_num(42));
+ v8::Handle<v8::Object> global0 = context0->Global();
- // Enter a new context.
- v8::Handle<v8::Context> inner_context = v8::Context::New(isolate);
- { v8::Context::Scope inner(inner_context);
- v8::Handle<v8::Object> inner_global = inner_context->Global();
- inner_global->Set(v8_str("other"), outer_global);
- v8::Handle<v8::FunctionTemplate> unreachable =
- v8::FunctionTemplate::New(UnreachableCallback);
- inner_global->Set(v8_str("unreachable"), unreachable->GetFunction());
- ExpectUndefined("other.x"); // Verify that access checks are in place.
- CompileRun("Object.observe(other, unreachable);"); // Install
observer.
+ // Create a context with a different security token so that the
+ // failed access check callback will be called on each access.
+ LocalContext context1(NULL, global_template);
+ context1->Global()->Set(v8_str("other"), global0);
+
+ v8::Handle<v8::FunctionTemplate> catcher_fun =
+ v8::FunctionTemplate::New(CatcherCallback);
+ context1->Global()->Set(v8_str("catcher"), catcher_fun->GetFunction());
+
+ v8::Handle<v8::FunctionTemplate> has_own_property_fun =
+ v8::FunctionTemplate::New(HasOwnPropertyCallback);
+ context1->Global()->Set(v8_str("has_own_property"),
+ has_own_property_fun->GetFunction());
+
+ { v8::TryCatch try_catch;
+ access_check_fail_thrown = false;
+ CompileRun("other.x;");
+ CHECK(access_check_fail_thrown);
+ CHECK(try_catch.HasCaught());
}
- ExpectInt32("x", 42);
- // This must not be observable by the observer set up in the inner
context.
- CompileRun("var a = 123;");
+ CheckCorrectThrow("other.x");
+ CheckCorrectThrow("other[1]");
+ CheckCorrectThrow("JSON.stringify(other)");
+ CheckCorrectThrow("has_own_property(other, 'x')");
+ CheckCorrectThrow("%GetProperty(other, 'x')");
+ CheckCorrectThrow("%SetProperty(other, 'x', 'foo', 1, 0)");
+ CheckCorrectThrow("%IgnoreAttributesAndSetProperty(other, 'x', 'foo')");
+ CheckCorrectThrow("%DeleteProperty(other, 'x', 0)");
+ CheckCorrectThrow("%DeleteProperty(other, '1', 0)");
+ CheckCorrectThrow("%HasLocalProperty(other, 'x')");
+ CheckCorrectThrow("%HasProperty(other, 'x')");
+ CheckCorrectThrow("%HasElement(other, 1)");
+ CheckCorrectThrow("%IsPropertyEnumerable(other, 'x')");
+ CheckCorrectThrow("%GetPropertyNames(other)");
+ CheckCorrectThrow("%GetLocalPropertyNames(other, true)");
+ CheckCorrectThrow("%DefineOrRedefineAccessorProperty("
+ "other, 'x', null, null, 1)");
}
#endif // WIN32
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
