From 721661d9c60ade2fede5c44342aab69f4ef0a571 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <rothberg@redhat.com>
Date: Fri, 3 Dec 2021 10:46:13 +0100
Subject: [PATCH] reference filter: match exact behavior of Docker

The previously inherited behavior from Podman was matching too
aggressively.  Now, the filter matches the exact behavior of
Docker, simplifies the code and is tested directly in libimage.

Context: containers/podman#11905
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>
---
 libimage/filters.go      | 22 ++++++--------
 libimage/filters_test.go | 62 ++++++++++++++++++++++++++++++++++++++++
 libimage/image.go        | 20 +++++++++++++
 3 files changed, 91 insertions(+), 13 deletions(-)
 create mode 100644 libimage/filters_test.go

diff --git a/libimage/filters.go b/libimage/filters.go
index 521af5d06..47b71da80 100644
--- a/libimage/filters.go
+++ b/libimage/filters.go
@@ -3,13 +3,13 @@ package libimage
 import (
 	"context"
 	"fmt"
-	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
 
 	filtersPkg "github.com/containers/common/pkg/filters"
 	"github.com/containers/common/pkg/timetype"
+	"github.com/containers/image/v5/docker/reference"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -160,20 +160,16 @@ func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOp
 
 // filterReference creates a reference filter for matching the specified value.
 func filterReference(value string) filterFunc {
-	// Replacing all '/' with '|' so that filepath.Match() can work '|'
-	// character is not valid in image name, so this is safe.
-	//
-	// TODO: this has been copied from Podman and requires some more review
-	// and especially tests.
-	filter := fmt.Sprintf("*%s*", value)
-	filter = strings.ReplaceAll(filter, "/", "|")
 	return func(img *Image) (bool, error) {
-		if len(value) < 1 {
-			return true, nil
+		refs, err := img.NamesReferences()
+		if err != nil {
+			return false, err
 		}
-		for _, name := range img.Names() {
-			newName := strings.ReplaceAll(name, "/", "|")
-			match, _ := filepath.Match(filter, newName)
+		for _, ref := range refs {
+			match, err := reference.FamiliarMatch(value, ref)
+			if err != nil {
+				return false, err
+			}
 			if match {
 				return true, nil
 			}
diff --git a/libimage/filters_test.go b/libimage/filters_test.go
new file mode 100644
index 000000000..632bd8871
--- /dev/null
+++ b/libimage/filters_test.go
@@ -0,0 +1,62 @@
+package libimage
+
+import (
+	"context"
+	"os"
+	"testing"
+
+	"github.com/containers/common/pkg/config"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFilterReference(t *testing.T) {
+	busyboxLatest := "quay.io/libpod/busybox:latest"
+	alpineLatest := "quay.io/libpod/alpine:latest"
+
+	runtime, cleanup := testNewRuntime(t)
+	defer cleanup()
+	ctx := context.Background()
+
+	pullOptions := &PullOptions{}
+	pullOptions.Writer = os.Stdout
+
+	pulledImages, err := runtime.Pull(ctx, busyboxLatest, config.PullPolicyMissing, pullOptions)
+	require.NoError(t, err)
+	require.Len(t, pulledImages, 1)
+	busybox := pulledImages[0]
+
+	pulledImages, err = runtime.Pull(ctx, alpineLatest, config.PullPolicyMissing, pullOptions)
+	require.NoError(t, err)
+	require.Len(t, pulledImages, 1)
+	alpine := pulledImages[0]
+
+	err = busybox.Tag("localhost/image:tag")
+	require.NoError(t, err)
+	err = alpine.Tag("localhost/image:another-tag")
+	require.NoError(t, err)
+
+	for _, test := range []struct {
+		filter  string
+		matches int
+	}{
+		{"image", 0},
+		{"localhost/image", 2},
+		{"localhost/image:tag", 1},
+		{"localhost/image:another-tag", 1},
+		{"localhost/*", 2},
+		{"localhost/image:*tag", 2},
+		{"busybox", 0},
+		{"alpine", 0},
+		{"quay.io/libpod/busybox", 1},
+		{"quay.io/libpod/alpine", 1},
+		{"quay.io/libpod", 0},
+		{"quay.io/libpod/*", 2},
+	} {
+		listOptions := &ListImagesOptions{
+			Filters: []string{"reference=" + test.filter},
+		}
+		listedImages, err := runtime.ListImages(ctx, nil, listOptions)
+		require.NoError(t, err, "%v", test)
+		require.Len(t, listedImages, test.matches, "%s -> %v", test.filter, listedImages)
+	}
+}
diff --git a/libimage/image.go b/libimage/image.go
index 50dbc9725..661ca159b 100644
--- a/libimage/image.go
+++ b/libimage/image.go
@@ -44,6 +44,8 @@ type Image struct {
 		completeInspectData *ImageData
 		// Corresponding OCI image.
 		ociv1Image *ociv1.Image
+		// Names() parsed into references.
+		namesReferences []reference.Reference
 	}
 }
 
@@ -59,6 +61,7 @@ func (i *Image) reload() error {
 	i.cached.partialInspectData = nil
 	i.cached.completeInspectData = nil
 	i.cached.ociv1Image = nil
+	i.cached.namesReferences = nil
 	return nil
 }
 
@@ -89,6 +92,23 @@ func (i *Image) Names() []string {
 	return i.storageImage.Names
 }
 
+// NamesReferences returns Names() as references.
+func (i *Image) NamesReferences() ([]reference.Reference, error) {
+	if i.cached.namesReferences != nil {
+		return i.cached.namesReferences, nil
+	}
+	refs := make([]reference.Reference, 0, len(i.Names()))
+	for _, name := range i.Names() {
+		ref, err := reference.Parse(name)
+		if err != nil {
+			return nil, err
+		}
+		refs = append(refs, ref)
+	}
+	i.cached.namesReferences = refs
+	return refs, nil
+}
+
 // StorageImage returns the underlying storage.Image.
 func (i *Image) StorageImage() *storage.Image {
 	return i.storageImage