Skip to content

Commit

Permalink
fix: array writes
Browse files Browse the repository at this point in the history
Fix array writes not being persisted by passing in writeable
reflect.Value when available instead of .Interface() which looses that
property.

Also:
* Correct typo of writeable.
* Use value.IsValid() instead of comparison with zero entry.

Fixes #386
  • Loading branch information
stevenh committed Nov 26, 2022
1 parent b6f2991 commit 4382e69
Show file tree
Hide file tree
Showing 10 changed files with 79 additions and 34 deletions.
4 changes: 2 additions & 2 deletions builtin_object.go
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ func builtinObject_isFrozen(call FunctionCall) Value {
result := true
object.enumerate(true, func(name string) bool {
property := object.getProperty(name)
if property.configurable() || property.writable() {
if property.configurable() || property.writeable() {
result = false
}
return true
Expand All @@ -243,7 +243,7 @@ func builtinObject_freeze(call FunctionCall) Value {
if object := object._object(); object != nil {
object.enumerate(true, func(name string) bool {
if property, update := object.getOwnProperty(name), false; nil != property {
if property.isDataDescriptor() && property.writable() {
if property.isDataDescriptor() && property.writeable() {
property.writeOff()
update = true
}
Expand Down
37 changes: 37 additions & 0 deletions issue_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -899,3 +899,40 @@ func Test_issue390(t *testing.T) {
require.NoError(t, err)
require.Equal(t, int64(1), rows)
}

type testSetType struct {
String string
Array [1]string
Slice []string
}

func TestIssue386(t *testing.T) {
var msg testSetType
msg.String = "string"
msg.Slice = []string{"slice"}
msg.Array[0] = "array"

vm := New()
err := vm.Set("msg", &msg)
require.NoError(t, err)

tests := map[string]string{
"string": `
msg.TypeMessage = 'something';
msg.TypeMessage;`,
"array": `
msg.Array[0] = 'something';
msg.Array[0]`,
"slice": `
msg.Slice[0] = 'something';
msg.Slice[0]`,
}

for name, code := range tests {
t.Run(name, func(t *testing.T) {
val, err := vm.Run(code)
require.NoError(t, err)
require.Equal(t, "something", val.String())
})
}
}
12 changes: 6 additions & 6 deletions object_class.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func _objectCanPut(self *_object, name string) (canPut bool, property *_property
if property != nil {
switch propertyValue := property.value.(type) {
case Value:
canPut = property.writable()
canPut = property.writeable()
return
case _propertyGetSet:
setter = propertyValue[1]
Expand All @@ -232,7 +232,7 @@ func _objectCanPut(self *_object, name string) (canPut bool, property *_property
if !self.extensible {
return false, nil, nil
}
return property.writable(), nil, nil
return property.writeable(), nil, nil
case _propertyGetSet:
setter = propertyValue[1]
canPut = setter != nil
Expand Down Expand Up @@ -364,10 +364,10 @@ func objectDefineOwnProperty(self *_object, name string, descriptor _property, t
} else if isDataDescriptor && descriptor.isDataDescriptor() {
// DataDescriptor <=> DataDescriptor
if !configurable {
if !property.writable() && descriptor.writable() {
if !property.writeable() && descriptor.writeable() {
goto Reject
}
if !property.writable() {
if !property.writeable() {
if descriptor.value != nil && !sameValue(value, descriptor.value.(Value)) {
goto Reject
}
Expand Down Expand Up @@ -422,7 +422,7 @@ func objectDefineOwnProperty(self *_object, name string, descriptor _property, t
mode0 := property.mode
if mode1&0200 != 0 {
if descriptor.isDataDescriptor() {
mode1 &= ^0200 // Turn off "writable" missing
mode1 &= ^0200 // Turn off "writeable" missing
mode1 |= (mode0 & 0100)
}
}
Expand All @@ -432,7 +432,7 @@ func objectDefineOwnProperty(self *_object, name string, descriptor _property, t
if mode1&02 != 0 {
mode1 |= (mode0 & 01)
}
mode1 &= 0311 // 0311 to preserve the non-setting on "writable"
mode1 &= 0311 // 0311 to preserve the non-setting on "writeable"
}
self._write(name, value1, mode1)
}
Expand Down
8 changes: 4 additions & 4 deletions property.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ type _property struct {
mode _propertyMode
}

func (self _property) writable() bool {
func (self _property) writeable() bool {
return self.mode&modeWriteMask == modeWriteMask&modeOnMask
}

Expand Down Expand Up @@ -138,8 +138,8 @@ func toPropertyDescriptor(rt *_runtime, value Value) (descriptor _property) {
}
}

if objectDescriptor.hasProperty("writable") {
if objectDescriptor.get("writable").bool() {
if objectDescriptor.hasProperty("writeable") {
if objectDescriptor.get("writeable").bool() {
descriptor.writeOn()
} else {
descriptor.writeOff()
Expand Down Expand Up @@ -199,7 +199,7 @@ func (self *_runtime) fromPropertyDescriptor(descriptor _property) *_object {
object := self.newObject()
if descriptor.isDataDescriptor() {
object.defineProperty("value", descriptor.value.(Value), 0111, false)
object.defineProperty("writable", toValue_bool(descriptor.writable()), 0111, false)
object.defineProperty("writeable", toValue_bool(descriptor.writeable()), 0111, false)
} else if descriptor.isAccessorDescriptor() {
getSet := descriptor.value.(_propertyGetSet)
get := Value{}
Expand Down
9 changes: 9 additions & 0 deletions runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,11 @@ func (self *_runtime) convertCallParameter(v Value, t reflect.Type) (reflect.Val
}

func (self *_runtime) toValue(value interface{}) Value {
rv, ok := value.(reflect.Value)
if ok {
value = rv.Interface()
}

switch value := value.(type) {
case Value:
return value
Expand Down Expand Up @@ -666,6 +671,10 @@ func (self *_runtime) toValue(value interface{}) Value {
default:
{
value := reflect.ValueOf(value)
if ok {
// Use passed in rv which may be writeable.
value = rv
}

switch value.Kind() {
case reflect.Ptr:
Expand Down
6 changes: 3 additions & 3 deletions type_array.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ func arrayDefineOwnProperty(self *_object, name string, descriptor _property, th
if newLength > length {
return objectDefineOwnProperty(self, name, descriptor, throw)
}
if !lengthProperty.writable() {
if !lengthProperty.writeable() {
goto Reject
}
newWritable := true
if descriptor.mode&0700 == 0 {
// If writable is off
// If writeable is off
newWritable = false
descriptor.mode |= 0100
}
Expand All @@ -88,7 +88,7 @@ func arrayDefineOwnProperty(self *_object, name string, descriptor _property, th
objectDefineOwnProperty(self, name, descriptor, false)
}
} else if index := stringToArrayIndex(name); index >= 0 {
if index >= int64(length) && !lengthProperty.writable() {
if index >= int64(length) && !lengthProperty.writeable() {
goto Reject
}
if !objectDefineOwnProperty(self, strconv.FormatInt(index, 10), descriptor, false) {
Expand Down
12 changes: 6 additions & 6 deletions type_go_array.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,19 @@ func (runtime *_runtime) newGoArrayObject(value reflect.Value) *_object {

type _goArrayObject struct {
value reflect.Value
writable bool
writeable bool
propertyMode _propertyMode
}

func _newGoArrayObject(value reflect.Value) *_goArrayObject {
writable := value.Kind() == reflect.Ptr // The Array is addressable (like a Slice)
writeable := value.CanSet() // The Array is addressable (like a Slice)
mode := _propertyMode(0010)
if writable {
if writeable {
mode = 0110
}
self := &_goArrayObject{
value: value,
writable: writable,
writeable: writeable,
propertyMode: mode,
}
return self
Expand Down Expand Up @@ -122,7 +122,7 @@ func goArrayDefineOwnProperty(self *_object, name string, descriptor _property,
return self.runtime.typeErrorResult(throw)
} else if index := stringToArrayIndex(name); index >= 0 {
object := self.value.(*_goArrayObject)
if object.writable {
if object.writeable {
if self.value.(*_goArrayObject).setValue(index, descriptor.value.(Value)) {
return true
}
Expand All @@ -142,7 +142,7 @@ func goArrayDelete(self *_object, name string, throw bool) bool {
index := stringToArrayIndex(name)
if index >= 0 {
object := self.value.(*_goArrayObject)
if object.writable {
if object.writeable {
indexValue, exists := object.getValueIndex(index)
if exists {
indexValue.Set(reflect.Zero(reflect.Indirect(object.value).Type().Elem()))
Expand Down
6 changes: 3 additions & 3 deletions type_go_map.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ func goMapGetOwnProperty(self *_object, name string) *_property {
object := self.value.(*_goMapObject)
value := object.value.MapIndex(object.toKey(name))
if value.IsValid() {
return &_property{self.runtime.toValue(value.Interface()), 0111}
return &_property{self.runtime.toValue(value), 0111}
}

// Other methods
if method := self.value.(*_goMapObject).value.MethodByName(name); (method != reflect.Value{}) {
if method := self.value.(*_goMapObject).value.MethodByName(name); !method.IsZero() {
return &_property{
value: self.runtime.toValue(method.Interface()),
value: self.runtime.toValue(method),
mode: 0110,
}
}
Expand Down
11 changes: 5 additions & 6 deletions type_go_slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,11 @@ func goSliceGetOwnProperty(self *_object, name string) *_property {
}

// .0, .1, .2, ...
index := stringToArrayIndex(name)
if index >= 0 {
if index := stringToArrayIndex(name); index >= 0 {
value := Value{}
reflectValue, exists := self.value.(*_goSliceObject).getValue(index)
if exists {
value = self.runtime.toValue(reflectValue.Interface())
value = self.runtime.toValue(reflectValue)
}
return &_property{
value: value,
Expand All @@ -68,9 +67,9 @@ func goSliceGetOwnProperty(self *_object, name string) *_property {
}

// Other methods
if method := self.value.(*_goSliceObject).value.MethodByName(name); (method != reflect.Value{}) {
if method := self.value.(*_goSliceObject).value.MethodByName(name); !method.IsValid() {
return &_property{
value: self.runtime.toValue(method.Interface()),
value: self.runtime.toValue(method),
mode: 0110,
}
}
Expand Down Expand Up @@ -106,7 +105,7 @@ func goSliceDefineOwnProperty(self *_object, name string, descriptor _property,

func goSliceDelete(self *_object, name string, throw bool) bool {
// length
if name == "length" {
if name == propertyLength {
return self.runtime.typeErrorResult(throw)
}

Expand Down
8 changes: 4 additions & 4 deletions type_go_struct.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ func (self _goStructObject) getValue(name string) reflect.Value {
}

if validGoStructName(name) {
// Do not reveal hidden or unexported fields
if field := reflect.Indirect(self.value).FieldByName(name); (field != reflect.Value{}) {
// Do not reveal hidden or unexported fields.
if field := reflect.Indirect(self.value).FieldByName(name); !field.IsValid() {
return field
}

if method := self.value.MethodByName(name); (method != reflect.Value{}) {
if method := self.value.MethodByName(name); !method.IsValid() {
return method
}
}
Expand Down Expand Up @@ -81,7 +81,7 @@ func goStructGetOwnProperty(self *_object, name string) *_property {
object := self.value.(*_goStructObject)
value := object.getValue(name)
if value.IsValid() {
return &_property{self.runtime.toValue(value.Interface()), 0110}
return &_property{self.runtime.toValue(value), 0110}
}

return objectGetOwnProperty(self, name)
Expand Down

0 comments on commit 4382e69

Please sign in to comment.