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

@@ -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

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++ {

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) {

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) {