From 4e4e89c89421ec459fb32e42d4839c7593f7c582 Mon Sep 17 00:00:00 2001 From: Robert Krimen Date: Wed, 20 Feb 2013 15:16:46 -0800 Subject: [PATCH] Fix bug in String.slice with a (go) slice bounds violation Basically, check that start is less than length before trying to take a slice. Also, rename valueToArrayIndex to valueToRangeIndex to reflect the fact that it does not always return a value that is valid index (value could be length) --- Makefile | 1 + builtin.go | 44 +++------------------------------- otto_.go | 65 +++++++++++++++++++++++++++++++++++++------------- string_test.go | 1 + 4 files changed, 53 insertions(+), 58 deletions(-) diff --git a/Makefile b/Makefile index 3526eebd..a3a90de6 100644 --- a/Makefile +++ b/Makefile @@ -44,6 +44,7 @@ TEST := -v --run _underscore_objects TEST := -v --run _underscore_ TEST := -v --run _parseFloat TEST := -v --run _underscore_utility +TEST := -v --run Array_splice TEST := . test: test-i diff --git a/builtin.go b/builtin.go index baeda127..155f92e7 100644 --- a/builtin.go +++ b/builtin.go @@ -654,7 +654,7 @@ func builtinString_slice(call FunctionCall) Value { length := uint(len(target)) start, end := rangeStartEnd(call.ArgumentList, length, false) - if 0 >= end-start { + if start >= length || end-start <= 0 { return toValue("") } return toValue(target[start:end]) @@ -665,8 +665,6 @@ func builtinString_substring(call FunctionCall) Value { target := toString(call.This) length := uint(len(target)) - //start := valueToArrayIndex(call.Argument(0), size, false) - //end := valueToArrayIndex(call.Argument(1), size, false) start, end := rangeStartEnd(call.ArgumentList, length, true) if start > end { start, end = end, start @@ -879,48 +877,12 @@ func builtinArray_joinNative(valueArray []Value, separator string) string { return strings.Join(stringList, separator) } -func rangeStartEnd(source []Value, size uint, negativeIsZero bool) (start, end uint) { - start = valueToArrayIndex(valueOfArrayIndex(source, 0), size, negativeIsZero) - if len(source) == 1 { - // If there is only the start argument, then end = size - end = size - return - } - - // Assuming the argument is undefined... - end = size - endValue := valueOfArrayIndex(source, 1) - if !endValue.IsUndefined() { - // Which it is not, so get the value as an array index - end = valueToArrayIndex(endValue, size, negativeIsZero) - } - return -} - -func rangeStartLength(source []Value, size uint) (start, length int64) { - start = int64(valueToArrayIndex(valueOfArrayIndex(source, 0), size, false)) - - // Assume the second argument is missing or undefined - length = int64(size) - if len(source) == 1 { - // If there is only the start argument, then length = size - return - } - - lengthValue := valueOfArrayIndex(source, 1) - if !lengthValue.IsUndefined() { - // Which it is not, so get the value as an array index - length = toInteger(lengthValue) - } - return -} - func builtinArray_splice(call FunctionCall) Value { thisObject := call.thisObject() length := uint(toUint32(thisObject.get("length"))) - start := valueToArrayIndex(call.Argument(0), length, false) - deleteCount := valueToArrayIndex(call.Argument(1), length-start, true) + start := valueToRangeIndex(call.Argument(0), length, false) + deleteCount := valueToRangeIndex(call.Argument(1), length-start, true) valueArray := make([]Value, deleteCount) for index := uint(0); index < deleteCount; index++ { diff --git a/otto_.go b/otto_.go index e07d6d4a..26491268 100644 --- a/otto_.go +++ b/otto_.go @@ -60,30 +60,61 @@ func valueOfArrayIndex(list []Value, index int) Value { return UndefinedValue() } -func valueToArrayIndex(indexValue Value, length uint, negativeIsZero bool) uint { +// A range index can be anything from 0 up to length. It is NOT safe to use as an index +// to an array, but is useful for slicing and in some ECMA algorithms. +func valueToRangeIndex(indexValue Value, length uint, negativeIsZero bool) uint { index := toIntegerFloat(indexValue) - if !negativeIsZero { - if 0 > length { - return uint(index) - } - if 0 > index { - index = math.Max(index+float64(length), 0) - } else { - index = math.Min(index, float64(length)) - } - return uint(index) - } - { + if negativeIsZero { index := uint(math.Max(index, 0)) - if 0 > length { - return index - } // minimum(index, length) - if index > length { + if index >= length { return length } return index } + + if index < 0 { + index = math.Max(index+float64(length), 0) + } else { + index = math.Min(index, float64(length)) + } + return uint(index) +} + +func rangeStartEnd(array []Value, size uint, negativeIsZero bool) (start, end uint) { + start = valueToRangeIndex(valueOfArrayIndex(array, 0), size, negativeIsZero) + if len(array) == 1 { + // If there is only the start argument, then end = size + end = size + return + } + + // Assuming the argument is undefined... + end = size + endValue := valueOfArrayIndex(array, 1) + if !endValue.IsUndefined() { + // Which it is not, so get the value as an array index + end = valueToRangeIndex(endValue, size, negativeIsZero) + } + return +} + +func rangeStartLength(source []Value, size uint) (start, length int64) { + start = int64(valueToRangeIndex(valueOfArrayIndex(source, 0), size, false)) + + // Assume the second argument is missing or undefined + length = int64(size) + if len(source) == 1 { + // If there is only the start argument, then length = size + return + } + + lengthValue := valueOfArrayIndex(source, 1) + if !lengthValue.IsUndefined() { + // Which it is not, so get the value as an array index + length = toInteger(lengthValue) + } + return } func boolFields(input string) (result map[string]bool) { diff --git a/string_test.go b/string_test.go index d79bc1e9..f96a2c25 100644 --- a/string_test.go +++ b/string_test.go @@ -169,6 +169,7 @@ func TestString_slice(t *testing.T) { test(`"abc".slice(0,11)`, "abc") test(`"abc".slice(0,-1)`, "ab") test(`"abc".slice(-1,11)`, "c") + test(`abc = "abc"; abc.slice(abc.length+1, 0)`, "") } func TestString_substring(t *testing.T) {