syscall/js: improve type checks of js.Value's methods

Add more explicit checks if the given js.Value is of the correct type
instead of erroring on the JavaScript layer.

Change-Id: I30b18a76820fb68f6ac279bb88a57456f5bab467
Reviewed-on: https://go-review.googlesource.com/c/go/+/168886
Run-TryBot: Richard Musiol <neelance@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/syscall/js/js.go b/src/syscall/js/js.go
index 0893db022d356255ea86f5fc1b11e02e77ed6b92..bccf188fa51d6daa7f387302194ec64d681380d6 100644 (file)
--- a/src/syscall/js/js.go
+++ b/src/syscall/js/js.go
@@ -216,6 +216,10 @@ func (t Type) String() string {
        }
 }
 
+func (t Type) isObject() bool {
+       return t == TypeObject || t == TypeFunction
+}
+
 // Type returns the JavaScript type of the value v. It is similar to JavaScript's typeof operator,
 // except that it returns TypeNull instead of TypeObject for null.
 func (v Value) Type() Type {
@@ -244,28 +248,44 @@ func (v Value) Type() Type {
 }
 
 // Get returns the JavaScript property p of value v.
+// It panics if v is not a JavaScript object.
 func (v Value) Get(p string) Value {
+       if vType := v.Type(); !vType.isObject() {
+               panic(&ValueError{"Value.Get", vType})
+       }
        return makeValue(valueGet(v.ref, p))
 }
 
 func valueGet(v ref, p string) ref
 
 // Set sets the JavaScript property p of value v to ValueOf(x).
+// It panics if v is not a JavaScript object.
 func (v Value) Set(p string, x interface{}) {
+       if vType := v.Type(); !vType.isObject() {
+               panic(&ValueError{"Value.Set", vType})
+       }
        valueSet(v.ref, p, ValueOf(x).ref)
 }
 
 func valueSet(v ref, p string, x ref)
 
 // Index returns JavaScript index i of value v.
+// It panics if v is not a JavaScript object.
 func (v Value) Index(i int) Value {
+       if vType := v.Type(); !vType.isObject() {
+               panic(&ValueError{"Value.Index", vType})
+       }
        return makeValue(valueIndex(v.ref, i))
 }
 
 func valueIndex(v ref, i int) ref
 
 // SetIndex sets the JavaScript index i of value v to ValueOf(x).
+// It panics if v is not a JavaScript object.
 func (v Value) SetIndex(i int, x interface{}) {
+       if vType := v.Type(); !vType.isObject() {
+               panic(&ValueError{"Value.SetIndex", vType})
+       }
        valueSetIndex(v.ref, i, ValueOf(x).ref)
 }
 
@@ -280,7 +300,11 @@ func makeArgs(args []interface{}) []ref {
 }
 
 // Length returns the JavaScript property "length" of v.
+// It panics if v is not a JavaScript object.
 func (v Value) Length() int {
+       if vType := v.Type(); !vType.isObject() {
+               panic(&ValueError{"Value.SetIndex", vType})
+       }
        return valueLength(v.ref)
 }
 
@@ -292,7 +316,7 @@ func valueLength(v ref) int
 func (v Value) Call(m string, args ...interface{}) Value {
        res, ok := valueCall(v.ref, m, makeArgs(args))
        if !ok {
-               if vType := v.Type(); vType != TypeObject && vType != TypeFunction { // check here to avoid overhead in success case
+               if vType := v.Type(); !vType.isObject() { // check here to avoid overhead in success case
                        panic(&ValueError{"Value.Call", vType})
                }
                if propType := v.Get(m).Type(); propType != TypeFunction {
@@ -306,7 +330,7 @@ func (v Value) Call(m string, args ...interface{}) Value {
 func valueCall(v ref, m string, args []ref) (ref, bool)
 
 // Invoke does a JavaScript call of the value v with the given arguments.
-// It panics if v is not a function.
+// It panics if v is not a JavaScript function.
 // The arguments get mapped to JavaScript values according to the ValueOf function.
 func (v Value) Invoke(args ...interface{}) Value {
        res, ok := valueInvoke(v.ref, makeArgs(args))
@@ -322,11 +346,14 @@ func (v Value) Invoke(args ...interface{}) Value {
 func valueInvoke(v ref, args []ref) (ref, bool)
 
 // New uses JavaScript's "new" operator with value v as constructor and the given arguments.
-// It panics if v is not a function.
+// It panics if v is not a JavaScript function.
 // The arguments get mapped to JavaScript values according to the ValueOf function.
 func (v Value) New(args ...interface{}) Value {
        res, ok := valueNew(v.ref, makeArgs(args))
        if !ok {
+               if vType := v.Type(); vType != TypeFunction { // check here to avoid overhead in success case
+                       panic(&ValueError{"Value.Invoke", vType})
+               }
                panic(Error{makeValue(res)})
        }
        return makeValue(res)
@@ -350,17 +377,20 @@ func (v Value) float(method string) float64 {
        return *(*float64)(unsafe.Pointer(&v.ref))
 }
 
-// Float returns the value v as a float64. It panics if v is not a JavaScript number.
+// Float returns the value v as a float64.
+// It panics if v is not a JavaScript number.
 func (v Value) Float() float64 {
        return v.float("Value.Float")
 }
 
-// Int returns the value v truncated to an int. It panics if v is not a JavaScript number.
+// Int returns the value v truncated to an int.
+// It panics if v is not a JavaScript number.
 func (v Value) Int() int {
        return int(v.float("Value.Int"))
 }
 
-// Bool returns the value v as a bool. It panics if v is not a JavaScript boolean.
+// Bool returns the value v as a bool.
+// It panics if v is not a JavaScript boolean.
 func (v Value) Bool() bool {
        switch v.ref {
        case valueTrue.ref:

diff --git a/src/syscall/js/js_test.go b/src/syscall/js/js_test.go
index c14d2cc24c90b8c5d08fe29db33218bc5e00975c..594284faf972bc52d838ec8be563e3413ff0d9a8 100644 (file)
--- a/src/syscall/js/js_test.go
+++ b/src/syscall/js/js_test.go
@@ -202,10 +202,30 @@ func TestLength(t *testing.T) {
        }
 }
 
+func TestGet(t *testing.T) {
+       // positive cases get tested per type
+
+       expectValueError(t, func() {
+               dummys.Get("zero").Get("badField")
+       })
+}
+
+func TestSet(t *testing.T) {
+       // positive cases get tested per type
+
+       expectValueError(t, func() {
+               dummys.Get("zero").Set("badField", 42)
+       })
+}
+
 func TestIndex(t *testing.T) {
        if got := dummys.Get("someArray").Index(1).Int(); got != 42 {
                t.Errorf("got %#v, want %#v", got, 42)
        }
+
+       expectValueError(t, func() {
+               dummys.Get("zero").Index(1)
+       })
 }
 
 func TestSetIndex(t *testing.T) {
@@ -213,6 +233,10 @@ func TestSetIndex(t *testing.T) {
        if got := dummys.Get("someArray").Index(2).Int(); got != 99 {
                t.Errorf("got %#v, want %#v", got, 99)
        }
+
+       expectValueError(t, func() {
+               dummys.Get("zero").SetIndex(2, 99)
+       })
 }
 
 func TestCall(t *testing.T) {
@@ -223,6 +247,13 @@ func TestCall(t *testing.T) {
        if got := dummys.Call("add", js.Global().Call("eval", "40"), 2).Int(); got != 42 {
                t.Errorf("got %#v, want %#v", got, 42)
        }
+
+       expectPanic(t, func() {
+               dummys.Call("zero")
+       })
+       expectValueError(t, func() {
+               dummys.Get("zero").Call("badMethod")
+       })
 }
 
 func TestInvoke(t *testing.T) {
@@ -230,12 +261,20 @@ func TestInvoke(t *testing.T) {
        if got := dummys.Get("add").Invoke(i, 2).Int(); got != 42 {
                t.Errorf("got %#v, want %#v", got, 42)
        }
+
+       expectValueError(t, func() {
+               dummys.Get("zero").Invoke()
+       })
 }
 
 func TestNew(t *testing.T) {
        if got := js.Global().Get("Array").New(42).Length(); got != 42 {
                t.Errorf("got %#v, want %#v", got, 42)
        }
+
+       expectValueError(t, func() {
+               dummys.Get("zero").New()
+       })
 }
 
 func TestInstanceOf(t *testing.T) {
@@ -379,3 +418,23 @@ func TestTruthy(t *testing.T) {
                t.Errorf("got %#v, want %#v", got, want)
        }
 }
+
+func expectValueError(t *testing.T, fn func()) {
+       defer func() {
+               err := recover()
+               if _, ok := err.(*js.ValueError); !ok {
+                       t.Errorf("expected *js.ValueError, got %T", err)
+               }
+       }()
+       fn()
+}
+
+func expectPanic(t *testing.T, fn func()) {
+       defer func() {
+               err := recover()
+               if err == nil {
+                       t.Errorf("expected panic")
+               }
+       }()
+       fn()
+}