From 9527a2a8a7c8f88b670acaf74dd17e9c9fb66e70 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 7 Aug 2020 23:23:54 +0200 Subject: [PATCH] deps: V8: cherry-pick e06ace6b5cdb Original commit message: [api] Fix empty Maybe crash in GetRealNamedPropertyAttributes `Object::GetRealNamedPropertyAttributes()` can crash if an empty `Maybe` is returned by `JSReceiver::GetPropertyAttributes()` because it was not checking for that. Fix that. Refs: https://github.com/nodejs/node/issues/34606 Change-Id: Ic83f904ba7134786bcd8f786eb2ce98adb4fea1e Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2335057 Commit-Queue: Leszek Swirski Reviewed-by: Leszek Swirski Cr-Commit-Position: refs/heads/master@{#69258} Refs: https://github.com/v8/v8/commit/e06ace6b5cdb64b6abfe8e1229f2159b7a38b4e7 PR-URL: https://github.com/nodejs/node/pull/34673 Fixes: https://github.com/nodejs/node/issues/34606 Reviewed-By: Jiawen Geng Reviewed-By: Rich Trott Reviewed-By: James M Snell --- common.gypi | 2 +- deps/v8/src/api/api.cc | 12 ++++++---- deps/v8/test/cctest/test-api.cc | 42 +++++++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 6 deletions(-) diff --git a/common.gypi b/common.gypi index 3ec4581ea811fb..882b3a694c5fc0 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.12', + 'v8_embedder_string': '-node.13', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/api/api.cc b/deps/v8/src/api/api.cc index 43f83529d82654..169c9bd7f90cea 100644 --- a/deps/v8/src/api/api.cc +++ b/deps/v8/src/api/api.cc @@ -4653,9 +4653,9 @@ Maybe v8::Object::GetRealNamedPropertyAttributesInPrototypeChain( Local context, Local key) { auto isolate = reinterpret_cast(context->GetIsolate()); - ENTER_V8_NO_SCRIPT(isolate, context, Object, - GetRealNamedPropertyAttributesInPrototypeChain, - Nothing(), i::HandleScope); + ENTER_V8(isolate, context, Object, + GetRealNamedPropertyAttributesInPrototypeChain, + Nothing(), i::HandleScope); i::Handle self = Utils::OpenHandle(this); if (!self->IsJSObject()) return Nothing(); i::Handle key_obj = Utils::OpenHandle(*key); @@ -4668,6 +4668,7 @@ v8::Object::GetRealNamedPropertyAttributesInPrototypeChain( i::LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR); Maybe result = i::JSReceiver::GetPropertyAttributes(&it); + has_pending_exception = result.IsNothing(); RETURN_ON_FAILED_EXECUTION_PRIMITIVE(PropertyAttribute); if (!it.IsFound()) return Nothing(); if (result.FromJust() == i::ABSENT) return Just(None); @@ -4692,14 +4693,15 @@ MaybeLocal v8::Object::GetRealNamedProperty(Local context, Maybe v8::Object::GetRealNamedPropertyAttributes( Local context, Local key) { auto isolate = reinterpret_cast(context->GetIsolate()); - ENTER_V8_NO_SCRIPT(isolate, context, Object, GetRealNamedPropertyAttributes, - Nothing(), i::HandleScope); + ENTER_V8(isolate, context, Object, GetRealNamedPropertyAttributes, + Nothing(), i::HandleScope); i::Handle self = Utils::OpenHandle(this); i::Handle key_obj = Utils::OpenHandle(*key); i::LookupIterator::Key lookup_key(isolate, key_obj); i::LookupIterator it(isolate, self, lookup_key, self, i::LookupIterator::PROTOTYPE_CHAIN_SKIP_INTERCEPTOR); auto result = i::JSReceiver::GetPropertyAttributes(&it); + has_pending_exception = result.IsNothing(); RETURN_ON_FAILED_EXECUTION_PRIMITIVE(PropertyAttribute); if (!it.IsFound()) return Nothing(); if (result.FromJust() == i::ABSENT) { diff --git a/deps/v8/test/cctest/test-api.cc b/deps/v8/test/cctest/test-api.cc index 151076296b41a8..18f7738033f8db 100644 --- a/deps/v8/test/cctest/test-api.cc +++ b/deps/v8/test/cctest/test-api.cc @@ -11959,6 +11959,48 @@ THREADED_TEST(VariousGetPropertiesAndThrowingCallbacks) { CHECK(result.IsEmpty()); } +THREADED_TEST(GetRealNamedPropertyAttributes_With_Proxy) { + LocalContext context; + HandleScope scope(context->GetIsolate()); + + { + Local proxy = + CompileRun( + "new Proxy({ p: 1 }, { getOwnPropertyDescriptor: _ => { " + " throw new Error('xyz'); } });") + .As(); + TryCatch try_catch(context->GetIsolate()); + v8::Maybe result = + proxy->GetRealNamedPropertyAttributes(context.local(), v8_str("p")); + CHECK(result.IsNothing()); + CHECK(try_catch.HasCaught()); + CHECK(try_catch.Exception() + .As() + ->Get(context.local(), v8_str("message")) + .ToLocalChecked() + ->StrictEquals(v8_str("xyz"))); + } + + { + Local proxy = + CompileRun( + "Object.create(" + " new Proxy({ p: 1 }, { getOwnPropertyDescriptor: _ => { " + " throw new Error('abc'); } }))") + .As(); + TryCatch try_catch(context->GetIsolate()); + v8::Maybe result = + proxy->GetRealNamedPropertyAttributesInPrototypeChain(context.local(), + v8_str("p")); + CHECK(result.IsNothing()); + CHECK(try_catch.HasCaught()); + CHECK(try_catch.Exception() + .As() + ->Get(context.local(), v8_str("message")) + .ToLocalChecked() + ->StrictEquals(v8_str("abc"))); + } +} static void ThrowingCallbackWithTryCatch( const v8::FunctionCallbackInfo& args) {