diff --git a/pkg/policy/match.go b/pkg/policy/match.go index ed18587..3f6cf0b 100644 --- a/pkg/policy/match.go +++ b/pkg/policy/match.go @@ -38,54 +38,43 @@ func matchStatement(statement Statement, node ipld.Node) bool { switch statement.Kind() { case KindEqual: if s, ok := statement.(equality); ok { - one, many, err := s.selector.Select(node) + res, err := s.selector.Select(node) if err != nil { return false } - if one != nil { - return datamodel.DeepEqual(s.value, one) - } - if many != nil { - for _, n := range many { - if eq := datamodel.DeepEqual(s.value, n); eq { - return true - } - } - } - - return false + return datamodel.DeepEqual(s.value, res) } case KindGreaterThan: if s, ok := statement.(equality); ok { - one, _, err := s.selector.Select(node) - if err != nil || one == nil { + res, err := s.selector.Select(node) + if err != nil { return false } - return isOrdered(s.value, one, gt) + return isOrdered(s.value, res, gt) } case KindGreaterThanOrEqual: if s, ok := statement.(equality); ok { - one, _, err := s.selector.Select(node) - if err != nil || one == nil { + res, err := s.selector.Select(node) + if err != nil { return false } - return isOrdered(s.value, one, gte) + return isOrdered(s.value, res, gte) } case KindLessThan: if s, ok := statement.(equality); ok { - one, _, err := s.selector.Select(node) - if err != nil || one == nil { + res, err := s.selector.Select(node) + if err != nil { return false } - return isOrdered(s.value, one, lt) + return isOrdered(s.value, res, lt) } case KindLessThanOrEqual: if s, ok := statement.(equality); ok { - one, _, err := s.selector.Select(node) - if err != nil || one == nil { + res, err := s.selector.Select(node) + if err != nil { return false } - return isOrdered(s.value, one, lte) + return isOrdered(s.value, res, lte) } case KindNot: if s, ok := statement.(negation); ok { @@ -116,24 +105,32 @@ func matchStatement(statement Statement, node ipld.Node) bool { } case KindLike: if s, ok := statement.(wildcard); ok { - one, _, err := s.selector.Select(node) - if err != nil || one == nil { - return false - } - v, err := one.AsString() + res, err := s.selector.Select(node) if err != nil { return false } + v, err := res.AsString() + if err != nil { + return false // not a string + } return s.pattern.Match(v) } case KindAll: if s, ok := statement.(quantifier); ok { - _, many, err := s.selector.Select(node) - if err != nil || many == nil { + res, err := s.selector.Select(node) + if err != nil { return false } - for _, n := range many { - ok := matchStatement(s.statement, n) + it := res.ListIterator() + if it == nil { + return false // not a list + } + for !it.Done() { + _, v, err := it.Next() + if err != nil { + return false + } + ok := matchStatement(s.statement, v) if !ok { return false } @@ -142,25 +139,24 @@ func matchStatement(statement Statement, node ipld.Node) bool { } case KindAny: if s, ok := statement.(quantifier); ok { - one, many, err := s.selector.Select(node) + res, err := s.selector.Select(node) if err != nil { return false } - if one != nil { - ok := matchStatement(s.statement, one) + it := res.ListIterator() + if it == nil { + return false // not a list + } + for !it.Done() { + _, v, err := it.Next() + if err != nil { + return false + } + ok := matchStatement(s.statement, v) if ok { return true } } - if many != nil { - for _, n := range many { - ok := matchStatement(s.statement, n) - if ok { - return true - } - } - } - return false } } diff --git a/pkg/policy/selector/selector.go b/pkg/policy/selector/selector.go index f60aa5c..09d6dc4 100644 --- a/pkg/policy/selector/selector.go +++ b/pkg/policy/selector/selector.go @@ -21,9 +21,8 @@ type Selector []segment // Select perform the selection described by the selector on the input IPLD DAG. // Select can return: // - exactly one matched IPLD node -// - an error that can be checked with IsResolutionErr(err) to distinguish not being able to resolve to a node, -// or another type of error. -// - nil and no error, if the selector couldn't match on an optional segment (with ?). +// - a resolutionerr error if not being able to resolve to a node +// - nil and no errors, if the selector couldn't match on an optional segment (with ?). func (s Selector) Select(subject ipld.Node) (ipld.Node, error) { return resolve(s, subject, nil) } @@ -108,8 +107,8 @@ func resolve(sel Selector, subject ipld.Node, at []string) (ipld.Node, error) { case cur == nil || cur.Kind() == datamodel.Kind_Null: if seg.Optional() { // build an empty list - n, err := qp.BuildList(basicnode.Prototype.Any, 0, func(_ datamodel.ListAssembler) {}) - return n, err + n, _ := qp.BuildList(basicnode.Prototype.Any, 0, func(_ datamodel.ListAssembler) {}) + return n, nil } return nil, newResolutionError(fmt.Sprintf("can not iterate over kind: %s", kindString(cur)), at) @@ -119,16 +118,24 @@ func resolve(sel Selector, subject ipld.Node, at []string) (ipld.Node, error) { case cur.Kind() == datamodel.Kind_Map: // iterators on maps collect the values - return qp.BuildList(basicnode.Prototype.Any, cur.Length(), func(l datamodel.ListAssembler) { + nd, err := qp.BuildList(basicnode.Prototype.Any, cur.Length(), func(l datamodel.ListAssembler) { it := cur.MapIterator() for !it.Done() { _, v, err := it.Next() if err != nil { - panic(err) // recovered by BuildList + // recovered by BuildList + // Error is bubbled up, but should never occur as we already checked the type, + // and are using the iterator correctly. + // This is verified with fuzzing. + panic(err) } qp.ListEntry(l, qp.Node(v)) } }) + if err != nil { + panic("should never happen") + } + return nd, nil default: return nil, newResolutionError(fmt.Sprintf("can not iterate over kind: %s", kindString(cur)), at) @@ -144,9 +151,7 @@ func resolve(sel Selector, subject ipld.Node, at []string) (ipld.Node, error) { case cur.Kind() == datamodel.Kind_Map: n, err := cur.LookupByString(seg.Field()) if err != nil { - if !isMissing(err) { - return nil, err - } + // the only possible error is missing field as we already check the type if seg.Optional() { cur = nil } else { @@ -187,7 +192,7 @@ func resolve(sel Selector, subject ipld.Node, at []string) (ipld.Node, error) { return nil, newResolutionError(fmt.Sprintf("can not slice on kind: %s", kindString(cur)), at) } - if start < 0 || end < start || end > length { + if start < 0 || end < start || end >= length { err := newResolutionError(fmt.Sprintf("slice out of bounds: [%d:%d]", start, end), at) return nil, errIfNotOptional(seg, err) } @@ -198,13 +203,16 @@ func resolve(sel Selector, subject ipld.Node, at []string) (ipld.Node, error) { for i := start; i <= end; i++ { item, err := cur.LookupByIndex(i) if err != nil { - panic(err) // recovered by BuildList + // recovered by BuildList + // Error is bubbled up, but should never occur as we already checked the type and boundaries + // This is verified with fuzzing. + panic(err) } qp.ListEntry(l, qp.Node(item)) } }) if err != nil { - return nil, errIfNotOptional(seg, err) + panic("should never happen") } cur = sliced case datamodel.Kind_Bytes: diff --git a/pkg/policy/selector/selector_test.go b/pkg/policy/selector/selector_test.go index f5470ca..fe53070 100644 --- a/pkg/policy/selector/selector_test.go +++ b/pkg/policy/selector/selector_test.go @@ -263,6 +263,10 @@ func FuzzParseAndSelect(f *testing.F) { } // look for panic() - _, _ = sel.Select(node) + _, err = sel.Select(node) + if err != nil && !IsResolutionErr(err) { + // not normal, we should only have resolution errors + t.Fatal(err) + } }) }