From cd5309355e25dda4a33bcf5c931aae5a15f9de94 Mon Sep 17 00:00:00 2001 From: Richard Musiol Date: Mon, 25 Mar 2019 00:31:41 +0100 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick --- src/syscall/js/js.go | 42 ++++++++++++++++++++++++---- src/syscall/js/js_test.go | 59 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 6 deletions(-) diff --git a/src/syscall/js/js.go b/src/syscall/js/js.go index 0893db022d..bccf188fa5 100644 --- 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 c14d2cc24c..594284faf9 100644 --- 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() +} -- 2.50.0