Browse code

Remove slash prefix when matching name filters (Fixes #37453)

* Regex name filters were display undesired behavior due to
names containing the trailing slash when being compared
* Adjusted filterByNameIDMatches and includeContainerInList to
strip the slash prefix before doing name comparisons
* Added test case and helper functions for the test to list_test
* Force failed tests during development to ensure there were
no false positives

Signed-off-by: Chris White <me@cwprogram.com>

Chris White authored on 2018/07/15 19:34:18
Showing 2 changed files
... ...
@@ -146,7 +146,7 @@ func (daemon *Daemon) filterByNameIDMatches(view container.View, ctx *listContex
146 146
 				continue
147 147
 			}
148 148
 			for _, eachName := range idNames {
149
-				if ctx.filters.Match("name", eachName) {
149
+				if ctx.filters.Match("name", strings.TrimPrefix(eachName, "/")) {
150 150
 					matches[id] = true
151 151
 				}
152 152
 			}
... ...
@@ -429,7 +429,7 @@ func includeContainerInList(container *container.Snapshot, ctx *listContext) ite
429 429
 	}
430 430
 
431 431
 	// Do not include container if the name doesn't match
432
-	if !ctx.filters.Match("name", container.Name) {
432
+	if !ctx.filters.Match("name", strings.TrimPrefix(container.Name, "/")) {
433 433
 		return excludeContainer
434 434
 	}
435 435
 
... ...
@@ -1,15 +1,82 @@
1 1
 package daemon
2 2
 
3 3
 import (
4
+	"io/ioutil"
5
+	"os"
6
+	"path/filepath"
7
+	"strings"
4 8
 	"testing"
5 9
 
6 10
 	"github.com/docker/docker/api/types"
11
+	containertypes "github.com/docker/docker/api/types/container"
7 12
 	"github.com/docker/docker/api/types/filters"
8 13
 	"github.com/docker/docker/container"
14
+	"github.com/docker/docker/image"
15
+	"github.com/opencontainers/go-digest"
16
+	"github.com/pborman/uuid"
9 17
 	"gotest.tools/assert"
10 18
 	is "gotest.tools/assert/cmp"
11 19
 )
12 20
 
21
+var root string
22
+
23
+func TestMain(m *testing.M) {
24
+	var err error
25
+	root, err = ioutil.TempDir("", "docker-container-test-")
26
+	if err != nil {
27
+		panic(err)
28
+	}
29
+	defer os.RemoveAll(root)
30
+
31
+	os.Exit(m.Run())
32
+}
33
+
34
+// This sets up a container with a name so that name filters
35
+// work against it. It takes in a pointer to Daemon so that
36
+// minor operations are not repeated by the caller
37
+func setupContainerWithName(t *testing.T, name string, daemon *Daemon) *container.Container {
38
+	var (
39
+		id              = uuid.New()
40
+		computedImageID = digest.FromString(id)
41
+		cRoot           = filepath.Join(root, id)
42
+	)
43
+	if err := os.MkdirAll(cRoot, 0755); err != nil {
44
+		t.Fatal(err)
45
+	}
46
+
47
+	c := container.NewBaseContainer(id, cRoot)
48
+	// these are for passing includeContainerInList
49
+	c.Name = name
50
+	c.Running = true
51
+	c.HostConfig = &containertypes.HostConfig{}
52
+
53
+	// these are for passing the refreshImage reducer
54
+	c.ImageID = image.IDFromDigest(computedImageID)
55
+	c.Config = &containertypes.Config{
56
+		Image: computedImageID.String(),
57
+	}
58
+
59
+	// this is done here to avoid requiring these
60
+	// operations n x number of containers in the
61
+	// calling function
62
+	daemon.containersReplica.Save(c)
63
+	daemon.reserveName(id, name)
64
+
65
+	return c
66
+}
67
+
68
+func containerListContainsName(containers []*types.Container, name string) bool {
69
+	for _, container := range containers {
70
+		for _, containerName := range container.Names {
71
+			if strings.TrimPrefix(containerName, "/") == name {
72
+				return true
73
+			}
74
+		}
75
+	}
76
+
77
+	return false
78
+}
79
+
13 80
 func TestListInvalidFilter(t *testing.T) {
14 81
 	db, err := container.NewViewDB()
15 82
 	assert.Assert(t, err == nil)
... ...
@@ -24,3 +91,35 @@ func TestListInvalidFilter(t *testing.T) {
24 24
 	})
25 25
 	assert.Assert(t, is.Error(err, "Invalid filter 'invalid'"))
26 26
 }
27
+
28
+func TestNameFilter(t *testing.T) {
29
+	db, err := container.NewViewDB()
30
+	assert.Assert(t, err == nil)
31
+	d := &Daemon{
32
+		containersReplica: db,
33
+	}
34
+
35
+	var (
36
+		one   = setupContainerWithName(t, "a1", d)
37
+		two   = setupContainerWithName(t, "a2", d)
38
+		three = setupContainerWithName(t, "b1", d)
39
+	)
40
+
41
+	// moby/moby #37453 - ^ regex not working due to prefix slash
42
+	// not being stripped
43
+	containerList, err := d.Containers(&types.ContainerListOptions{
44
+		Filters: filters.NewArgs(filters.Arg("name", "^a")),
45
+	})
46
+	assert.Assert(t, err == nil)
47
+	assert.Assert(t, is.Len(containerList, 2))
48
+	assert.Assert(t, containerListContainsName(containerList, one.Name))
49
+	assert.Assert(t, containerListContainsName(containerList, two.Name))
50
+
51
+	// Same as above but make sure it works for exact names
52
+	containerList, err = d.Containers(&types.ContainerListOptions{
53
+		Filters: filters.NewArgs(filters.Arg("name", "b1")),
54
+	})
55
+	assert.Assert(t, err == nil)
56
+	assert.Assert(t, is.Len(containerList, 1))
57
+	assert.Assert(t, containerListContainsName(containerList, three.Name))
58
+}