Remove calls to v8::FunctionCallbackInfo::Holder()
Use This() instead. Modify retrieval of internal fields to try
the prototype when missing, assuming it might be GlobalObjectProxy,
We previously had this logic, but it wasn't being applied in all
cases.
-- Store object type in private data for quicker validation.
Bug: 333672197
Change-Id: I00e4f185e9f612ec7fe95422497552a5c6f10702
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/120132
Reviewed-by: Thomas Sepez <tsepez@google.com>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Tom Sepez <tsepez@chromium.org>
diff --git a/fxjs/cfxjs_engine.cpp b/fxjs/cfxjs_engine.cpp
index 9e5d9d9..4379f81 100644
--- a/fxjs/cfxjs_engine.cpp
+++ b/fxjs/cfxjs_engine.cpp
@@ -59,24 +59,37 @@
public:
~CFXJS_PerObjectData() = default;
- static void SetNewDataInObject(uint32_t nObjDefnID,
+ static void SetNewDataInObject(FXJSOBJTYPE eObjType,
+ uint32_t nObjDefnID,
v8::Local<v8::Object> pObj) {
if (pObj->InternalFieldCount() == 2) {
pObj->SetAlignedPointerInInternalField(
0, GetAlignedPointerForPerObjectDataTag());
pObj->SetAlignedPointerInInternalField(
- 1, new CFXJS_PerObjectData(nObjDefnID));
+ 1, new CFXJS_PerObjectData(eObjType, nObjDefnID));
}
}
static CFXJS_PerObjectData* GetFromObject(v8::Local<v8::Object> pObj) {
- if (pObj.IsEmpty() || pObj->InternalFieldCount() != 2 ||
- pObj->GetAlignedPointerFromInternalField(0) !=
- GetAlignedPointerForPerObjectDataTag()) {
+ if (pObj.IsEmpty()) {
return nullptr;
}
- return static_cast<CFXJS_PerObjectData*>(
- pObj->GetAlignedPointerFromInternalField(1));
+ if (HasInternalFields(pObj)) {
+ return ExtractFromObject(pObj);
+ }
+ // `pObj` might be the global object proxy, in which case its prototype
+ // is the global object with the internal fields.
+ v8::Local<v8::Value> proto = pObj->GetPrototype();
+ if (proto.IsEmpty() || !proto->IsObject()) {
+ return nullptr;
+ }
+ pObj = proto.As<v8::Object>();
+ if (!HasInternalFields(pObj)) {
+ return nullptr;
+ }
+ // Double-check that this was really the global object.
+ CFXJS_PerObjectData* result = ExtractFromObject(pObj);
+ return result->m_ObjType == FXJSOBJTYPE_GLOBAL ? result : nullptr;
}
uint32_t GetObjDefnID() const { return m_ObjDefnID; }
@@ -84,8 +97,21 @@
void SetPrivate(std::unique_ptr<CJS_Object> p) { m_pPrivate = std::move(p); }
private:
- explicit CFXJS_PerObjectData(uint32_t nObjDefnID) : m_ObjDefnID(nObjDefnID) {}
+ CFXJS_PerObjectData(FXJSOBJTYPE eObjType, uint32_t nObjDefnID)
+ : m_ObjType(eObjType), m_ObjDefnID(nObjDefnID) {}
+ static bool HasInternalFields(v8::Local<v8::Object> pObj) {
+ return pObj->InternalFieldCount() == 2 &&
+ pObj->GetAlignedPointerFromInternalField(0) ==
+ GetAlignedPointerForPerObjectDataTag();
+ }
+
+ static CFXJS_PerObjectData* ExtractFromObject(v8::Local<v8::Object> pObj) {
+ return static_cast<CFXJS_PerObjectData*>(
+ pObj->GetAlignedPointerFromInternalField(1));
+ }
+
+ const FXJSOBJTYPE m_ObjType;
const uint32_t m_ObjDefnID;
std::unique_ptr<CJS_Object> m_pPrivate;
};
@@ -184,7 +210,7 @@
fxv8::ThrowExceptionHelper(isolate, "not a dynamic object");
return;
}
- v8::Local<v8::Object> holder = info.Holder();
+ v8::Local<v8::Object> holder = info.This();
DCHECK_EQ(holder->InternalFieldCount(), 2);
holder->SetAlignedPointerInInternalField(0, nullptr);
holder->SetAlignedPointerInInternalField(1, nullptr);
@@ -504,7 +530,8 @@
v8::Local<v8::Context> v8Context = v8::Context::New(
GetIsolate(), nullptr, GetGlobalObjectTemplate(GetIsolate()));
- // May not have the internal fields when called from tests.
+ // May not have the internal fields when called from tests, so clear these
+ // in case we don't process a FXJSOBJTYPE_GLOBAL below.
v8::Local<v8::Object> pThisProxy = v8Context->Global();
if (pThisProxy->InternalFieldCount() == 2) {
pThisProxy->SetAlignedPointerInInternalField(0, nullptr);
@@ -523,7 +550,7 @@
for (uint32_t i = 1; i <= maxID; ++i) {
CFXJS_ObjDefinition* pObjDef = pIsolateData->ObjDefinitionForID(i);
if (pObjDef->GetObjType() == FXJSOBJTYPE_GLOBAL) {
- CFXJS_PerObjectData::SetNewDataInObject(i, pThis);
+ CFXJS_PerObjectData::SetNewDataInObject(FXJSOBJTYPE_GLOBAL, i, pThis);
pObjDef->RunConstructor(this, pThis, pThisProxy);
} else if (pObjDef->GetObjType() == FXJSOBJTYPE_STATIC) {
v8::Local<v8::String> pObjName = NewString(pObjDef->GetObjName());
@@ -534,6 +561,7 @@
}
}
}
+
m_V8Context.Reset(GetIsolate(), v8Context);
}
@@ -617,7 +645,7 @@
if (!pObjDef->GetInstanceTemplate()->NewInstance(context).ToLocal(&obj))
return v8::Local<v8::Object>();
- CFXJS_PerObjectData::SetNewDataInObject(nObjDefnID, obj);
+ CFXJS_PerObjectData::SetNewDataInObject(type, nObjDefnID, obj);
pObjDef->RunConstructor(this, obj, obj);
if (type == FXJSOBJTYPE_DYNAMIC) {
auto* pIsolateData = FXJS_PerIsolateData::Get(GetIsolate());
@@ -650,32 +678,7 @@
CJS_Object* CFXJS_Engine::GetObjectPrivate(v8::Isolate* pIsolate,
v8::Local<v8::Object> pObj) {
auto* pData = CFXJS_PerObjectData::GetFromObject(pObj);
- if (pData)
- return pData->GetPrivate();
-
- if (pObj.IsEmpty())
- return nullptr;
-
- // It could be a global proxy object, in which case the prototype holds
- // the actual bound object.
- v8::Local<v8::Value> val = pObj->GetPrototype();
- if (!val->IsObject())
- return nullptr;
-
- auto* pProtoData = CFXJS_PerObjectData::GetFromObject(val.As<v8::Object>());
- if (!pProtoData)
- return nullptr;
-
- auto* pIsolateData = FXJS_PerIsolateData::Get(pIsolate);
- if (!pIsolateData)
- return nullptr;
-
- CFXJS_ObjDefinition* pObjDef =
- pIsolateData->ObjDefinitionForID(pProtoData->GetObjDefnID());
- if (!pObjDef || pObjDef->GetObjType() != FXJSOBJTYPE_GLOBAL)
- return nullptr;
-
- return pProtoData->GetPrivate();
+ return pData ? pData->GetPrivate() : nullptr;
}
v8::Local<v8::Array> CFXJS_Engine::GetConstArray(const WideString& name) {
diff --git a/fxjs/cjs_globalarrays.cpp b/fxjs/cjs_globalarrays.cpp
index dfd9bc2..0887b3a 100644
--- a/fxjs/cjs_globalarrays.cpp
+++ b/fxjs/cjs_globalarrays.cpp
@@ -12,25 +12,25 @@
#include "v8/include/v8-container.h"
#include "v8/include/v8-isolate.h"
-#define GLOBAL_ARRAY(rt, name, ...) \
- { \
- static const wchar_t* const kValues[] = {__VA_ARGS__}; \
- v8::Local<v8::Array> array = (rt)->NewArray(); \
- v8::Local<v8::Context> ctx = (rt)->GetIsolate()->GetCurrentContext(); \
- uint32_t i = 0; \
- for (const auto* value : kValues) { \
- array->Set(ctx, i, (rt)->NewString(value)).FromJust(); \
- ++i; \
- } \
- (rt)->SetConstArray((name), array); \
- (rt)->DefineGlobalConst( \
- (name), [](const v8::FunctionCallbackInfo<v8::Value>& info) { \
- CJS_Object* pObj = CFXJS_Engine::GetObjectPrivate(info.GetIsolate(), \
- info.Holder()); \
- CJS_Runtime* pCurrentRuntime = pObj->GetRuntime(); \
- if (pCurrentRuntime) \
- info.GetReturnValue().Set(pCurrentRuntime->GetConstArray(name)); \
- }); \
+#define GLOBAL_ARRAY(rt, name, ...) \
+ { \
+ static const wchar_t* const kValues[] = {__VA_ARGS__}; \
+ v8::Local<v8::Array> array = (rt)->NewArray(); \
+ v8::Local<v8::Context> ctx = (rt)->GetIsolate()->GetCurrentContext(); \
+ uint32_t i = 0; \
+ for (const auto* value : kValues) { \
+ array->Set(ctx, i, (rt)->NewString(value)).FromJust(); \
+ ++i; \
+ } \
+ (rt)->SetConstArray((name), array); \
+ (rt)->DefineGlobalConst( \
+ (name), [](const v8::FunctionCallbackInfo<v8::Value>& info) { \
+ CJS_Object* pObj = \
+ CFXJS_Engine::GetObjectPrivate(info.GetIsolate(), info.This()); \
+ CJS_Runtime* pCurrentRuntime = pObj->GetRuntime(); \
+ if (pCurrentRuntime) \
+ info.GetReturnValue().Set(pCurrentRuntime->GetConstArray(name)); \
+ }); \
}
// static
diff --git a/fxjs/cjs_publicmethods.cpp b/fxjs/cjs_publicmethods.cpp
index debb3d8..c09930c 100644
--- a/fxjs/cjs_publicmethods.cpp
+++ b/fxjs/cjs_publicmethods.cpp
@@ -137,7 +137,7 @@
void JSGlobalFunc(const char* func_name_string,
const v8::FunctionCallbackInfo<v8::Value>& info) {
CJS_Object* pObj =
- CFXJS_Engine::GetObjectPrivate(info.GetIsolate(), info.Holder());
+ CFXJS_Engine::GetObjectPrivate(info.GetIsolate(), info.This());
if (!pObj)
return;
diff --git a/fxjs/js_define.h b/fxjs/js_define.h
index 579cf5e..8aa84c1 100644
--- a/fxjs/js_define.h
+++ b/fxjs/js_define.h
@@ -115,7 +115,7 @@
void JSMethod(const char* method_name_string,
const char* class_name_string,
const v8::FunctionCallbackInfo<v8::Value>& info) {
- auto pObj = JSGetObject<C>(info.GetIsolate(), info.Holder());
+ auto pObj = JSGetObject<C>(info.GetIsolate(), info.This());
if (!pObj)
return;
