From ebd4367dda4f9a77b58518798913bf7635bddb20 Mon Sep 17 00:00:00 2001 From: Michael Manganiello Date: Fri, 12 May 2023 09:51:47 -0300 Subject: [PATCH] Fix false positive in ContainsAll function As the `ContainsAll` is working with a match counter, it could return a false positive when the `haystack` slice contains duplicate elements. This can be checked with the included testing scenario, with `haystack = [1, 1]` and `needles = [1, 2]`. Iterating over the haystack to check for items to be present in needles will increase the match counter to 2, even if `2` is not present in the first slice. --- util/util.go | 14 ++++++-------- util/util_test.go | 8 +++++++- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/util/util.go b/util/util.go index 33fa34ee..84177d9f 100644 --- a/util/util.go +++ b/util/util.go @@ -6,7 +6,6 @@ import ( "encoding/json" "errors" "fmt" - "golang.org/x/time/rate" "io" "math/rand" "net/netip" @@ -17,6 +16,8 @@ import ( "sync" "time" + "golang.org/x/time/rate" + "github.com/gabriel-vasile/mimetype" "golang.org/x/term" ) @@ -67,15 +68,12 @@ func ContainsIP(haystack []netip.Prefix, needle netip.Addr) bool { // ContainsAll returns true if all needles are contained in haystack func ContainsAll[T comparable](haystack []T, needles []T) bool { - matches := 0 - for _, s := range haystack { - for _, needle := range needles { - if s == needle { - matches++ - } + for _, needle := range needles { + if !Contains(haystack, needle) { + return false } } - return matches == len(needles) + return true } // SplitNoEmpty splits a string using strings.Split, but filters out empty strings diff --git a/util/util_test.go b/util/util_test.go index 5717c5cc..49a24126 100644 --- a/util/util_test.go +++ b/util/util_test.go @@ -2,7 +2,6 @@ package util import ( "errors" - "golang.org/x/time/rate" "io" "net/netip" "os" @@ -11,6 +10,8 @@ import ( "testing" "time" + "golang.org/x/time/rate" + "github.com/stretchr/testify/require" ) @@ -49,6 +50,11 @@ func TestContains(t *testing.T) { require.False(t, Contains(s, 3)) } +func TestContainsAll(t *testing.T) { + require.True(t, ContainsAll([]int{1, 2, 3}, []int{2, 3})) + require.False(t, ContainsAll([]int{1, 1}, []int{1, 2})) +} + func TestContainsIP(t *testing.T) { require.True(t, ContainsIP([]netip.Prefix{netip.MustParsePrefix("fd00::/8"), netip.MustParsePrefix("1.1.0.0/16")}, netip.MustParseAddr("1.1.1.1"))) require.True(t, ContainsIP([]netip.Prefix{netip.MustParsePrefix("fd00::/8"), netip.MustParsePrefix("1.1.0.0/16")}, netip.MustParseAddr("fd12:1234:5678::9876")))