From d064e791a6e055b6d2dbee45e708bd02c744cd44 Mon Sep 17 00:00:00 2001
From: Dylan Thacker-Smith <Dylan.Smith@shopify.com>
Date: Mon, 18 Oct 2021 10:19:21 -0400
Subject: [PATCH] Panic on get or set of out of range internal field index.

---
 helpers_test.go    |  8 ++++++++
 object.go          | 23 ++++++++++++++++++-----
 object_template.go | 10 ++++++++--
 object_test.go     | 35 ++++++++++++++++++++++-------------
 v8go.cc            | 24 +++++++++++++++++++-----
 v8go.h             |  4 +++-
 6 files changed, 78 insertions(+), 26 deletions(-)

diff --git a/helpers_test.go b/helpers_test.go
index 21c7f772..090d0350 100644
--- a/helpers_test.go
+++ b/helpers_test.go
@@ -8,3 +8,11 @@ func fatalIf(t *testing.T, err error) {
 		t.Fatal(err)
 	}
 }
+
+func recoverPanic(f func()) (recovered interface{}) {
+	defer func() {
+		recovered = recover()
+	}()
+	f()
+	return nil
+}
diff --git a/object.go b/object.go
index 61a4baaf..f9f691ea 100644
--- a/object.go
+++ b/object.go
@@ -84,7 +84,8 @@ func (o *Object) SetIdx(idx uint32, val interface{}) error {
 	return nil
 }
 
-// SetInternalField sets the value of an internal field.
+// SetInternalField sets the value of an internal field for an ObjectTemplate instance.
+// Panics if the index isn't in the range set by (*ObjectTemplate).SetInternalFieldCount.
 func (o *Object) SetInternalField(idx uint32, val interface{}) error {
 	value, err := coerceValue(o.ctx.iso, val)
 
@@ -95,12 +96,18 @@ func (o *Object) SetInternalField(idx uint32, val interface{}) error {
 	inserted := C.ObjectSetInternalField(o.ptr, C.uint32_t(idx), value.ptr)
 
 	if inserted == 0 {
-		return errors.New("v8go: index exceeded internal field count")
+		panic(fmt.Errorf("index out of range [%v] with length %v", idx, o.InternalFieldCount()))
 	}
 
 	return nil
 }
 
+// InternalFieldCount returns the number of internal fields this Object has.
+func (o *Object) InternalFieldCount() uint32 {
+	count := C.ObjectInternalFieldCount(o.ptr)
+	return uint32(count)
+}
+
 // Get tries to get a Value for a given Object property key.
 func (o *Object) Get(key string) (*Value, error) {
 	ckey := C.CString(key)
@@ -110,10 +117,16 @@ func (o *Object) Get(key string) (*Value, error) {
 	return valueResult(o.ctx, rtn)
 }
 
-// GetInternalField tries to get a Value for a given Object internal property key.
-func (o *Object) GetInternalField(idx uint32) (*Value, error) {
+// GetInternalField gets the Value set by SetInternalField for the given index
+// or the JS undefined value if the index hadn't been set.
+// Panics if given an out of range index.
+func (o *Object) GetInternalField(idx uint32) *Value {
 	rtn := C.ObjectGetInternalField(o.ptr, C.uint32_t(idx))
-	return valueResult(o.ctx, rtn)
+	if rtn == nil {
+		panic(fmt.Errorf("index out of range [%v] with length %v", idx, o.InternalFieldCount()))
+	}
+	return &Value{rtn, o.ctx}
+
 }
 
 // GetIdx tries to get a Value at a give Object index.
diff --git a/object_template.go b/object_template.go
index 77800733..c1a7b320 100644
--- a/object_template.go
+++ b/object_template.go
@@ -60,12 +60,18 @@ func (o *ObjectTemplate) NewInstance(ctx *Context) (*Object, error) {
 	return objectResult(ctx, rtn)
 }
 
-// SetInternalFieldCount sets the amount of internal fields that we want our
-// object to have.
+// SetInternalFieldCount sets the number of internal fields that instances of this
+// template will have.
 func (o *ObjectTemplate) SetInternalFieldCount(fieldCount uint32) {
 	C.ObjectTemplateSetInternalFieldCount(o.ptr, C.uint32_t(fieldCount))
 }
 
+// InternalFieldCount returns the number of internal fields that instances of this
+// template will have.
+func (o *ObjectTemplate) InternalFieldCount() uint32 {
+	return uint32(C.ObjectTemplateInternalFieldCount(o.ptr))
+}
+
 func (o *ObjectTemplate) apply(opts *contextOptions) {
 	opts.gTmpl = o
 }
diff --git a/object_test.go b/object_test.go
index 6cea20d7..b577e38f 100644
--- a/object_test.go
+++ b/object_test.go
@@ -80,29 +80,38 @@ func TestObjectInternalFields(t *testing.T) {
 	defer ctx.Close()
 
 	tmpl := v8.NewObjectTemplate(iso)
-	obj, _ := tmpl.NewInstance(ctx)
-	err := obj.SetInternalField(0, "This should fail")
-
-	if err == nil {
-		t.Errorf("Setting an internal field without calling SetInternalFieldCount should fail")
+	obj, err := tmpl.NewInstance(ctx)
+	fatalIf(t, err)
+	if count := obj.InternalFieldCount(); count != 0 {
+		t.Errorf("expected 0 got %v", count)
+	}
+	if recoverPanic(func() { obj.GetInternalField(0) }) == nil {
+		t.Error("expected panic")
 	}
 
 	tmpl = v8.NewObjectTemplate(iso)
 	tmpl.SetInternalFieldCount(1)
+	if count := tmpl.InternalFieldCount(); count != 1 {
+		t.Errorf("expected 1 got %v", count)
+	}
 
-	obj, _ = tmpl.NewInstance(ctx)
-
-	obj.SetInternalField(0, "baz")
-	v, err := obj.GetInternalField(0)
+	obj, err = tmpl.NewInstance(ctx)
+	fatalIf(t, err)
+	if count := obj.InternalFieldCount(); count != 1 {
+		t.Errorf("expected 1 got %v", count)
+	}
 
-	if v.String() != "baz" || err != nil {
+	if v := obj.GetInternalField(0); !v.SameValue(v8.Undefined(iso)) {
 		t.Errorf("unexpected value: %q", v)
 	}
 
-	err = obj.SetInternalField(1, "baz")
+	obj.SetInternalField(0, "baz")
+	if v := obj.GetInternalField(0); v.String() != "baz" {
+		t.Errorf("unexpected value: %q", v)
+	}
 
-	if err == nil {
-		t.Error("Should get \"index exceeded internal field count\" error")
+	if recoverPanic(func() { obj.SetInternalField(1, "baz") }) == nil {
+		t.Error("expected panic from index out of bounds")
 	}
 }
 
diff --git a/v8go.cc b/v8go.cc
index 505de704..8fda5de9 100644
--- a/v8go.cc
+++ b/v8go.cc
@@ -284,6 +284,13 @@ void ObjectTemplateSetInternalFieldCount(TemplatePtr ptr, uint32_t field_count)
   obj_tmpl->SetInternalFieldCount(field_count);
 }
 
+int ObjectTemplateInternalFieldCount(TemplatePtr ptr) {
+  LOCAL_TEMPLATE(ptr);
+
+  Local<ObjectTemplate> obj_tmpl = tmpl.As<ObjectTemplate>();
+  return obj_tmpl->InternalFieldCount();
+}
+
 /********** FunctionTemplate **********/
 
 static void FunctionTemplateCallback(const FunctionCallbackInfo<Value>& info) {
@@ -1059,7 +1066,7 @@ int ObjectSetInternalField(ValuePtr ptr, uint32_t idx, ValuePtr val_ptr) {
   LOCAL_OBJECT(ptr);
   m_value* prop_val = static_cast<m_value*>(val_ptr);
 
-  if (obj->InternalFieldCount() <= idx) {
+  if (idx >= obj->InternalFieldCount()) {
     return 0;
   }
 
@@ -1068,6 +1075,11 @@ int ObjectSetInternalField(ValuePtr ptr, uint32_t idx, ValuePtr val_ptr) {
   return 1;
 }
 
+int ObjectInternalFieldCount(ValuePtr ptr) {
+  LOCAL_OBJECT(ptr);
+  return obj->InternalFieldCount();
+}
+
 RtnValue ObjectGet(ValuePtr ptr, const char* key) {
   LOCAL_OBJECT(ptr);
   RtnValue rtn = {nullptr, nullptr};
@@ -1093,9 +1105,12 @@ RtnValue ObjectGet(ValuePtr ptr, const char* key) {
   return rtn;
 }
 
-RtnValue ObjectGetInternalField(ValuePtr ptr, uint32_t idx) {
+ValuePtr ObjectGetInternalField(ValuePtr ptr, uint32_t idx) {
   LOCAL_OBJECT(ptr);
-  RtnValue rtn = {nullptr, nullptr};
+
+  if (idx >= obj->InternalFieldCount()) {
+    return nullptr;
+  }
 
   Local<Value> result = obj->GetInternalField(idx);
 
@@ -1105,8 +1120,7 @@ RtnValue ObjectGetInternalField(ValuePtr ptr, uint32_t idx) {
   new_val->ptr = Persistent<Value, CopyablePersistentTraits<Value>>(
       iso, result);
 
-  rtn.value = tracked_value(ctx, new_val);
-  return rtn;
+  return tracked_value(ctx, new_val);
 }
 
 RtnValue ObjectGetIdx(ValuePtr ptr, uint32_t idx) {
diff --git a/v8go.h b/v8go.h
index e86451b9..371ec10e 100644
--- a/v8go.h
+++ b/v8go.h
@@ -98,6 +98,7 @@ extern void TemplateSetTemplate(TemplatePtr ptr,
 extern TemplatePtr NewObjectTemplate(IsolatePtr iso_ptr);
 extern RtnValue ObjectTemplateNewInstance(TemplatePtr ptr, ContextPtr ctx_ptr);
 extern void ObjectTemplateSetInternalFieldCount(TemplatePtr ptr, uint32_t field_count);
+extern int ObjectTemplateInternalFieldCount(TemplatePtr ptr);
 
 extern TemplatePtr NewFunctionTemplate(IsolatePtr iso_ptr, int callback_ref);
 extern RtnValue FunctionTemplateGetFunction(TemplatePtr ptr,
@@ -185,9 +186,10 @@ int ValueIsModuleNamespaceObject(ValuePtr ptr);
 extern void ObjectSet(ValuePtr ptr, const char* key, ValuePtr val_ptr);
 extern void ObjectSetIdx(ValuePtr ptr, uint32_t idx, ValuePtr val_ptr);
 extern int ObjectSetInternalField(ValuePtr ptr, uint32_t idx, ValuePtr val_ptr);
+extern int ObjectInternalFieldCount(ValuePtr ptr);
 extern RtnValue ObjectGet(ValuePtr ptr, const char* key);
 extern RtnValue ObjectGetIdx(ValuePtr ptr, uint32_t idx);
-extern RtnValue ObjectGetInternalField(ValuePtr ptr, uint32_t idx);
+extern ValuePtr ObjectGetInternalField(ValuePtr ptr, uint32_t idx);
 int ObjectHas(ValuePtr ptr, const char* key);
 int ObjectHasIdx(ValuePtr ptr, uint32_t idx);
 int ObjectDelete(ValuePtr ptr, const char* key);