IsVendor() overmatching paths

I discovered this through our Gitea server flagging files as vendored through its use of enry.IsVendor(). Paths like `oslo_cache/_bmemcache_pool.py` and `playbooks/roles/create-venv/tasks/main.yaml` are inappropriately marked vendored.

My hunch is that the first path is matching https://github.com/go-enry/go-enry/blob/7168084e5e5de38b915b1874528ff73f20a86b69/data/vendor.go#L9
and the second is matching https://github.com/go-enry/go-enry/blob/7168084e5e5de38b915b1874528ff73f20a86b69/data/vendor.go#L110

I've written a little reproducer that removes Gitea from the equation:

```
package main

import "fmt"
import "regexp"
import "github.com/go-enry/go-enry/v2"

func main() {
	input_str1 := "oslo_cache/_bmemcache_pool.py"

	rawregex1, _ := regexp.MatchString(`(^|/)cache/`, input_str1)
	fmt.Println("Raw regex:", rawregex1)

	vendor1 := enry.IsVendor(input_str1)
	fmt.Println("IsVendor:", vendor1)

	input_str2 := "playbooks/roles/create-venv/tasks/main.yaml"

	rawregex2, _ := regexp.MatchString(`(^|/)env/`, input_str2)
	fmt.Println("Raw regex:", rawregex2)

	vendor2 := enry.IsVendor(input_str2)
	fmt.Println("IsVendor:", vendor2)
}
```

When you run this the results are:

```
Raw regex: false
IsVendor: true
Raw regex: false
IsVendor: true
```

What this shows us is that the raw input regexes appear to behave as expected. Neither of our example input strings matches which is what we expect. But when we call `IsVendor()` the result becomes `true`. I suspect that the init function https://github.com/go-enry/go-enry/blob/7168084e5e5de38b915b1874528ff73f20a86b69/utils.go#L139-L246 is either adding rules that collide or introducing some bug to the expanded regex that causes this to happen.

	func init() {
	// We now collate the individual regexps that make up the VendorMatchers to
	// produce a single large regexp which is around twice as fast to test than
	// simply iterating through all the regexps or naïvely collating the
	// regexps.
	//
	// ---
	//
	// data.VendorMatchers here is a slice containing individual regexps that
	// match a vendor file therefore if we want to test if a filename is a
	// Vendor we need to test whether that filename matches one or more of
	// those regexps.
	//
	// Now we could test each matcher in turn using a shortcircuiting test i.e.
	//
	// func IsVendor(filename string) bool {
	// for _, matcher := range data.VendorMatchers {
	// if matcher.Match(filename) {
	// return true
	// }
	// }
	// return false
	// }
	//
	// Or concatentate all these regexps using groups i.e.
	//
	// `(regexp1)\|(regexp2)\|(regexp3)\|...`
	//
	// However both of these are relatively slow and they don't take advantage
	// of the inherent structure within our regexps...
	//
	// If we look at our regexps there are essentially three types of regexp:
	//
	// 1. Those that start with `^`
	// 2. Those that start with `(^\|/)`
	// 3. Others
	//
	// If we collate our regexps into these groups that will significantly
	// reduce the likelihood of backtracking within the regexp trie matcher.
	//
	// A further improvement is to use non-capturing groups as otherwise the
	// regexp parser, whilst matching, will have to allocate slices for
	// matching positions. (A future improvement here could be in the use of
	// enforcing non-capturing groups within the sub-regexps too.)
	//
	// Finally if we sort the segments we can help the matcher build a more
	// efficient matcher and trie.

	// alias the VendorMatchers to simplify things
	matchers := data.VendorMatchers

	// Create three temporary string slices for our three groups above - prefixes removed
	caretStrings := make([]string, 0, 10)
	caretSegmentStrings := make([]string, 0, 10)
	matcherStrings := make([]string, 0, len(matchers))

	// Walk the matchers and check their string representation for each group prefix, remove it and add to the respective group slices
	for _, matcher := range matchers {
	str := matcher.String()
	if str[0] == '^' {
	caretStrings = append(caretStrings, str[1:])
	} else if str[0:5] == "(^\|/)" {
	caretSegmentStrings = append(caretSegmentStrings, str[5:])
	} else {
	matcherStrings = append(matcherStrings, str)
	}
	}

	// Sort the strings within each group - a potential further improvement could be in simplifying within these groups
	sort.Strings(caretSegmentStrings)
	sort.Strings(caretStrings)
	sort.Strings(matcherStrings)

	// Now build the collated regexp
	sb := &strings.Builder{}

	// Start with group 1 - those that started with `^`
	sb.WriteString("(?:^(?:")
	sb.WriteString(caretStrings[0])
	for _, matcher := range caretStrings[1:] {
	sb.WriteString(")\|(?:")
	sb.WriteString(matcher)
	}
	sb.WriteString("))")
	sb.WriteString("\|")

	// Now add group 2 - those that started with `(^\|/)`
	sb.WriteString("(?:(?:^\|/)(?:")
	sb.WriteString(caretSegmentStrings[0])
	for _, matcher := range caretSegmentStrings[1:] {
	sb.WriteString(")\|(?:")
	sb.WriteString(matcher)
	}
	sb.WriteString("))")
	sb.WriteString("\|")

	// Finally add the rest
	sb.WriteString("(?:")
	sb.WriteString(matcherStrings[0])
	for _, matcher := range matcherStrings[1:] {
	sb.WriteString(")\|(?:")
	sb.WriteString(matcher)
	}
	sb.WriteString(")")

	// Compile the whole thing as the isVendorRegExp
	isVendorRegExp = regexp.MustCompile(sb.String())
	}

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

IsVendor() overmatching paths #135

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

IsVendor() overmatching paths #135

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions