Reviewers: jochen,
Message:
ptal
Description:
Properly fix enumerate / Object.keys wrt access checked objects
BUG=chromium:509936
LOG=y
Please review this at https://codereview.chromium.org/1241953010/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+53, -26 lines):
M src/objects.cc
M src/runtime/runtime-object.cc
M test/cctest/test-api.cc
Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index
554f9dce970f44a62229e6dbd2eb56bf24f1aa47..538fbea0a0bbee899f19c4687cfaa4dc8e3a4575
100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -6147,10 +6147,13 @@ MaybeHandle<FixedArray>
JSReceiver::GetKeys(Handle<JSReceiver> object,
Handle<JSFunction> arguments_function(
JSFunction::cast(isolate->sloppy_arguments_map()->GetConstructor()));
+ PrototypeIterator::WhereToEnd end = type == OWN_ONLY
+ ?
PrototypeIterator::END_AT_NON_HIDDEN
+ : PrototypeIterator::END_AT_NULL;
// Only collect keys if access is permitted.
for (PrototypeIterator iter(isolate, object,
PrototypeIterator::START_AT_RECEIVER);
- !iter.IsAtEnd(); iter.Advance()) {
+ !iter.IsAtEnd(end); iter.Advance()) {
if (PrototypeIterator::GetCurrent(iter)->IsJSProxy()) {
Handle<JSProxy>
proxy(JSProxy::cast(*PrototypeIterator::GetCurrent(iter)),
isolate);
@@ -6177,7 +6180,11 @@ MaybeHandle<FixedArray>
JSReceiver::GetKeys(Handle<JSReceiver> object,
// Check access rights if required.
if (current->IsAccessCheckNeeded() && !isolate->MayAccess(current)) {
- return content;
+ if (iter.IsAtEnd(PrototypeIterator::END_AT_NON_HIDDEN)) {
+ isolate->ReportFailedAccessCheck(current);
+ RETURN_EXCEPTION_IF_SCHEDULED_EXCEPTION(isolate, FixedArray);
+ }
+ break;
}
// Compute the element keys.
@@ -6237,10 +6244,6 @@ MaybeHandle<FixedArray>
JSReceiver::GetKeys(Handle<JSReceiver> object,
}
DCHECK(ContainsOnlyValidKeys(content));
}
-
- // If we only want own properties we bail out after the first
- // iteration.
- if (type == OWN_ONLY) break;
}
return content;
}
Index: src/runtime/runtime-object.cc
diff --git a/src/runtime/runtime-object.cc b/src/runtime/runtime-object.cc
index
d0d215d43bb18b53fd6213974c81370517118da2..24ff5a7fc2d5f606cbf8739fedbec1ba1f6cd807
100644
--- a/src/runtime/runtime-object.cc
+++ b/src/runtime/runtime-object.cc
@@ -1010,20 +1010,6 @@ RUNTIME_FUNCTION(Runtime_OwnKeys) {
CONVERT_ARG_CHECKED(JSObject, raw_object, 0);
Handle<JSObject> object(raw_object);
- if (object->IsJSGlobalProxy()) {
- // Do access checks before going to the global object.
- if (object->IsAccessCheckNeeded() && !isolate->MayAccess(object)) {
- isolate->ReportFailedAccessCheck(object);
- RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
- return *isolate->factory()->NewJSArray(0);
- }
-
- PrototypeIterator iter(isolate, object);
- // If proxy is detached we simply return an empty array.
- if (iter.IsAtEnd()) return *isolate->factory()->NewJSArray(0);
- object = Handle<JSObject>::cast(PrototypeIterator::GetCurrent(iter));
- }
-
Handle<FixedArray> contents;
ASSIGN_RETURN_FAILURE_ON_EXCEPTION(
isolate, contents, JSReceiver::GetKeys(object,
JSReceiver::OWN_ONLY));
Index: test/cctest/test-api.cc
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
index
ce34d7d29bd768465e5e574db57e029d362a437a..f9f2310f50dcc8525f7293e5d48b96984b539bc4
100644
--- a/test/cctest/test-api.cc
+++ b/test/cctest/test-api.cc
@@ -8140,7 +8140,7 @@ THREADED_TEST(CrossDomainIsPropertyEnumerable) {
}
-THREADED_TEST(CrossDomainForIn) {
+THREADED_TEST(CrossDomainFor) {
LocalContext env1;
v8::HandleScope handle_scope(env1->GetIsolate());
v8::Handle<Context> env2 = Context::New(env1->GetIsolate());
@@ -8164,9 +8164,8 @@ THREADED_TEST(CrossDomainForIn) {
Context::Scope scope_env2(env2);
Local<Value> result = CompileRun(
"(function() {"
- " var obj = { '__proto__': env1 };"
" try {"
- " for (var p in obj) {"
+ " for (var p in env1) {"
" if (p == 'prop') return false;"
" }"
" return true;"
@@ -8179,6 +8178,45 @@ THREADED_TEST(CrossDomainForIn) {
}
+THREADED_TEST(CrossDomainForInOnPrototype) {
+ LocalContext env1;
+ v8::HandleScope handle_scope(env1->GetIsolate());
+ v8::Handle<Context> env2 = Context::New(env1->GetIsolate());
+
+ Local<Value> foo = v8_str("foo");
+ Local<Value> bar = v8_str("bar");
+
+ // Set to the same domain.
+ env1->SetSecurityToken(foo);
+ env2->SetSecurityToken(foo);
+
+ env1->Global()->Set(v8_str("prop"), v8_num(3));
+ env2->Global()->Set(v8_str("env1"), env1->Global());
+
+ // Change env2 to a different domain and set env1's global object
+ // as the __proto__ of an object in env2 and enumerate properties
+ // in for-in. It shouldn't enumerate properties on env1's global
+ // object.
+ env2->SetSecurityToken(bar);
+ {
+ Context::Scope scope_env2(env2);
+ Local<Value> result = CompileRun(
+ "(function() {"
+ " var obj = { '__proto__': env1 };"
+ " try {"
+ " for (var p in obj) {"
+ " if (p == 'prop') return false;"
+ " }"
+ " return false;"
+ " } catch (e) {"
+ " return true;"
+ " }"
+ "})()");
+ CHECK(result->IsTrue());
+ }
+}
+
+
TEST(ContextDetachGlobal) {
LocalContext env1;
v8::HandleScope handle_scope(env1->GetIsolate());
@@ -8627,9 +8665,9 @@ TEST(AccessControl) {
" return false;"
" }"
" }"
- " return true;"
- " } catch (e) {"
" return false;"
+ " } catch (e) {"
+ " return true;"
" }"
"})()");
CHECK(value->IsTrue());
@@ -8673,7 +8711,7 @@ TEST(AccessControlES5) {
global1->Set(v8_str("other"), global0);
// Regression test for issue 1154.
- CHECK(CompileRun("Object.keys(other)").IsEmpty());
+ CHECK(CompileRun("Object.keys(other).length == 0")->BooleanValue());
CHECK(CompileRun("other.blocked_prop").IsEmpty());
// Regression test for issue 1027.
--
--
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/d/optout.
