From 66c253cbf70f5dcf75d47be9558a18096a39bc58 Mon Sep 17 00:00:00 2001
From: "Kai Qiang Wu(Kennan)" <wkqwu@cn.ibm.com>
Date: Mon, 4 Jan 2016 08:37:01 +0000
Subject: [PATCH] Fix volume filter validation

Fixes: #18890
This fix add same filter validation logic as images. We should
add such check to make sure filters work make sense to end-users

Right now, we keep old use 1 as filter, but in long term, it should
be have same interface checking as images, it could be improved in
other patches.

Signed-off-by: Kai Qiang Wu(Kennan) <wkqwu@cn.ibm.com>
---
 daemon/list.go                            | 24 +++++++++++++++++++----
 integration-cli/docker_cli_volume_test.go | 24 +++++++++++++++++++++++
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/daemon/list.go b/daemon/list.go
index f262fbe821..4bc844a4cb 100644
--- a/daemon/list.go
+++ b/daemon/list.go
@@ -15,6 +15,10 @@ import (
 	"github.com/docker/go-connections/nat"
 )
 
+var acceptedVolumeFilterTags = map[string]bool{
+	"dangling": true,
+}
+
 // iterationAction represents possible outcomes happening during the container iteration.
 type iterationAction int
 
@@ -410,20 +414,32 @@ func (daemon *Daemon) transformContainer(container *container.Container, ctx *li
 // Volumes lists known volumes, using the filter to restrict the range
 // of volumes returned.
 func (daemon *Daemon) Volumes(filter string) ([]*types.Volume, []string, error) {
-	var volumesOut []*types.Volume
+	var (
+		volumesOut   []*types.Volume
+		danglingOnly = false
+	)
 	volFilters, err := filters.FromParam(filter)
 	if err != nil {
 		return nil, nil, err
 	}
 
-	filterUsed := volFilters.Include("dangling") &&
-		(volFilters.ExactMatch("dangling", "true") || volFilters.ExactMatch("dangling", "1"))
+	if err := volFilters.Validate(acceptedVolumeFilterTags); err != nil {
+		return nil, nil, err
+	}
+
+	if volFilters.Include("dangling") {
+		if volFilters.ExactMatch("dangling", "true") || volFilters.ExactMatch("dangling", "1") {
+			danglingOnly = true
+		} else if !volFilters.ExactMatch("dangling", "false") && !volFilters.ExactMatch("dangling", "0") {
+			return nil, nil, fmt.Errorf("Invalid filter 'dangling=%s'", volFilters.Get("dangling"))
+		}
+	}
 
 	volumes, warnings, err := daemon.volumes.List()
 	if err != nil {
 		return nil, nil, err
 	}
-	if filterUsed {
+	if danglingOnly {
 		volumes = daemon.volumes.FilterByUsed(volumes)
 	}
 	for _, v := range volumes {
diff --git a/integration-cli/docker_cli_volume_test.go b/integration-cli/docker_cli_volume_test.go
index 7d1683a7ee..7c08e393aa 100644
--- a/integration-cli/docker_cli_volume_test.go
+++ b/integration-cli/docker_cli_volume_test.go
@@ -117,6 +117,30 @@ func (s *DockerSuite) TestVolumeCliLsFilterDangling(c *check.C) {
 	c.Assert(out, checker.Contains, "testnotinuse1\n", check.Commentf("expected volume 'testnotinuse1' in output"))
 	c.Assert(out, check.Not(checker.Contains), "testisinuse1\n", check.Commentf("volume 'testisinuse1' in output, but not expected"))
 	c.Assert(out, check.Not(checker.Contains), "testisinuse2\n", check.Commentf("volume 'testisinuse2' in output, but not expected"))
+
+	out, _ = dockerCmd(c, "volume", "ls", "--filter", "dangling=1")
+	// Filter "dangling" volumes; only "dangling" (unused) volumes should be in the output, dangling also accept 1
+	c.Assert(out, checker.Contains, "testnotinuse1\n", check.Commentf("expected volume 'testnotinuse1' in output"))
+	c.Assert(out, check.Not(checker.Contains), "testisinuse1\n", check.Commentf("volume 'testisinuse1' in output, but not expected"))
+	c.Assert(out, check.Not(checker.Contains), "testisinuse2\n", check.Commentf("volume 'testisinuse2' in output, but not expected"))
+
+	out, _ = dockerCmd(c, "volume", "ls", "--filter", "dangling=0")
+	// dangling=0 is same as dangling=false case
+	c.Assert(out, checker.Contains, "testnotinuse1\n", check.Commentf("expected volume 'testnotinuse1' in output"))
+	c.Assert(out, checker.Contains, "testisinuse1\n", check.Commentf("expected volume 'testisinuse1' in output"))
+	c.Assert(out, checker.Contains, "testisinuse2\n", check.Commentf("expected volume 'testisinuse2' in output"))
+}
+
+func (s *DockerSuite) TestVolumeCliLsErrorWithInvalidFilterName(c *check.C) {
+	out, _, err := dockerCmdWithError("volume", "ls", "-f", "FOO=123")
+	c.Assert(err, checker.NotNil)
+	c.Assert(out, checker.Contains, "Invalid filter")
+}
+
+func (s *DockerSuite) TestVolumeCliLsWithIncorrectFilterValue(c *check.C) {
+	out, _, err := dockerCmdWithError("volume", "ls", "-f", "dangling=invalid")
+	c.Assert(err, check.NotNil)
+	c.Assert(out, checker.Contains, "Invalid filter")
 }
 
 func (s *DockerSuite) TestVolumeCliRm(c *check.C) {