Skip to content

Commit

Permalink
topdown: Fix panic in array.slice due to incorrect clamping
Browse files Browse the repository at this point in the history
The built-in function implemented clamping in an odd way. With this
change, the stopIndex will always be [0, len(arr)] and the startIndex
will always be [0, clamped_stopIndex].

Fixes #2320

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
  • Loading branch information
tsandall committed Apr 21, 2020
1 parent dd04920 commit bbcbbe1
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 12 deletions.
23 changes: 11 additions & 12 deletions topdown/array.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,24 +49,23 @@ func builtinArraySlice(a, i, j ast.Value) (ast.Value, error) {
return nil, err
}

// Return empty array if bounds cannot be clamped sensibly.
if (startIndex >= stopIndex) || (startIndex <= 0 && stopIndex <= 0) {
return arr[0:0], nil
// Clamp stopIndex to avoid out-of-range errors. If negative, clamp to zero.
// Otherwise, clamp to length of array.
if stopIndex < 0 {
stopIndex = 0
} else if stopIndex > len(arr) {
stopIndex = len(arr)
}

// Clamp bounds to avoid out-of-range errors.
// Clamp startIndex to avoid out-of-range errors. If negative, clamp to zero.
// Otherwise, clamp to stopIndex to avoid to avoid cases like arr[1:0].
if startIndex < 0 {
startIndex = 0
} else if startIndex > stopIndex {
startIndex = stopIndex
}

if stopIndex > len(arr) {
stopIndex = len(arr)
}

arrb := arr[startIndex:stopIndex]

return arrb, nil

return arr[startIndex:stopIndex], nil
}

func init() {
Expand Down
2 changes: 2 additions & 0 deletions topdown/array_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ func TestTopDownArray(t *testing.T) {
{"slice: stopIndex < startIndex", []string{`p = x { x = array.slice([1,2,3,4,5], 4, 1) }`}, "[]"},
{"slice: clamp startIndex", []string{`p = x { x = array.slice([1,2,3,4,5], -1, 2) }`}, `[1,2]`},
{"slice: clamp stopIndex", []string{`p = x {x = array.slice([1,2,3,4,5], 3, 6) }`}, `[4,5]`},
{"slice: clamp both out of range", []string{"p = x { x = array.slice([], 1000, 2000) }"}, `[]`},
{"slice: clamp both out of range non-empty", []string{"p = x { x = array.slice([1,2,3], 1000, 2000) }"}, `[]`},
}

data := loadSmallTestData()
Expand Down

0 comments on commit bbcbbe1

Please sign in to comment.