Skip to content

Commit

Permalink
fix(selectors): more permissive with slice "from" underflow
Browse files Browse the repository at this point in the history
  • Loading branch information
rvagg committed Jul 5, 2023
1 parent f6610a1 commit d4d5de8
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 33 deletions.
2 changes: 1 addition & 1 deletion .ipld
53 changes: 24 additions & 29 deletions traversal/selector/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,24 @@ type Slice struct {
To int64
}

func sliceBounds(from, to, length int64) (bool, int64, int64) {
if to < 0 {
to = length + to
} else if length < to {
to = length
}
if from < 0 {
from = length + from
if from < 0 {
from = 0
}
}
if from > to || from >= length {
return false, 0, 0
}
return true, from, to
}

func (s Slice) Slice(n datamodel.Node) (datamodel.Node, error) {
var from, to int64
switch n.Kind() {
Expand All @@ -41,24 +59,11 @@ func (s Slice) Slice(n datamodel.Node) (datamodel.Node, error) {
return nil, err
}

to = s.To
from = s.From
length := int64(len(str))

if to < 0 {
to = length + to
} else if length < to {
to = length
}
if from < 0 {
from = length + from
} else if length < from {
from = length
}
if from < 0 || to < 0 || from > to || from >= length {
var match bool
match, from, to = sliceBounds(s.From, s.To, int64(len(str)))
if !match {
return nil, nil
}

return basicnode.NewString(str[from:to]), nil
case datamodel.Kind_Bytes:
to = s.To
Expand Down Expand Up @@ -91,21 +96,11 @@ func (s Slice) Slice(n datamodel.Node) (datamodel.Node, error) {
length = int64(len(bytes))
}

if to < 0 {
to = length + to
} else if length < to {
to = length
}
if from < 0 {
from = length + from
} else if length < from {
from = length
}

if from < 0 || to < 0 || from > to || from >= length {
var match bool
match, from, to = sliceBounds(from, to, length)
if !match {
return nil, nil
}

if rdr != nil {
sr := io.NewSectionReader(&readerat{rdr, 0}, from, to-from)
return basicnode.NewBytesFromReader(sr), nil
Expand Down
20 changes: 17 additions & 3 deletions traversal/selector/matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package selector_test
import (
"fmt"
"math"
"regexp"
"testing"

qt "github.com/frankban/quicktest"
Expand Down Expand Up @@ -78,9 +79,11 @@ func TestSubsetMatch(t *testing.T) {
{-int64(len(expectedString)), math.MaxInt64, expectedString, true},
{math.MaxInt64 - 1, math.MaxInt64, "", false},
{int64(len(expectedString)), math.MaxInt64, "", false},
{-1000, -100, "", false},
{-1, -2, "", false},
{-1, -1, "", true}, // matches empty node
{-1, -2, "", false}, // To < From, no match
{-1, -1, "", true}, // To==From, match zero bytes
{-1000, -100, "", false}, // From undeflow, adjusted to 0, To underflow, not adjusted, To < From, no match
{-100, -1000, "", false}, // From undeflow, adjusted to 0, To underflow, adjusted to 0, To < From, no match
{-1000, 1000, expectedString, true}, // From undeflow, adjusted to 0, To overflow, adjusted to len, match all
} {
for _, variant := range nodes {
t.Run(fmt.Sprintf("%s[%d:%d]", variant.name, tc.from, tc.to), func(t *testing.T) {
Expand Down Expand Up @@ -123,4 +126,15 @@ func TestSubsetMatch(t *testing.T) {
})
}
}

// when both are positive, we can validate ranges up-front
t.Run("invalid range", func(t *testing.T) {
selNode, err := mkRangeSelector(1000, 100)
qt.Assert(t, err, qt.IsNil)
re, err := regexp.Compile("from.*less than or equal to.*to")
qt.Assert(t, err, qt.IsNil)
ss, err := selector.ParseSelector(selNode)
qt.Assert(t, ss, qt.IsNil)
qt.Assert(t, err, qt.ErrorMatches, re)
})
}

0 comments on commit d4d5de8

Please sign in to comment.