From 8a4e508830203eb45b736fa778c9f1636cb89336 Mon Sep 17 00:00:00 2001 From: Lifubang Date: Sun, 2 Sep 2018 21:43:19 +0800 Subject: [PATCH 1/2] fixes: no name check in docker container command Signed-off-by: Lifubang --- cli/command/container/kill.go | 5 +++++ cli/command/container/rm.go | 5 +++++ cli/command/container/stop.go | 5 +++++ cli/names.go | 19 +++++++++++++++++++ cli/names/names.go | 11 +++++++++++ 5 files changed, 45 insertions(+) create mode 100644 cli/names.go create mode 100644 cli/names/names.go diff --git a/cli/command/container/kill.go b/cli/command/container/kill.go index feedbc01114b..e83a580bd77b 100644 --- a/cli/command/container/kill.go +++ b/cli/command/container/kill.go @@ -26,6 +26,11 @@ func NewKillCommand(dockerCli command.Cli) *cobra.Command { Short: "Kill one or more running containers", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + for _, name := range args { + if !cli.CheckContainerName(name) { + return fmt.Errorf("container name %s is invalid", name) + } + } opts.containers = args return runKill(dockerCli, &opts) }, diff --git a/cli/command/container/rm.go b/cli/command/container/rm.go index 2dcd4b6ace94..cf0be9c51b9c 100644 --- a/cli/command/container/rm.go +++ b/cli/command/container/rm.go @@ -29,6 +29,11 @@ func NewRmCommand(dockerCli command.Cli) *cobra.Command { Short: "Remove one or more containers", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + for _, name := range args { + if !cli.CheckContainerName(name) { + return fmt.Errorf("container name %s is invalid", name) + } + } opts.containers = args return runRm(dockerCli, &opts) }, diff --git a/cli/command/container/stop.go b/cli/command/container/stop.go index e299175436fa..13ddcfd5bc59 100644 --- a/cli/command/container/stop.go +++ b/cli/command/container/stop.go @@ -28,6 +28,11 @@ func NewStopCommand(dockerCli command.Cli) *cobra.Command { Short: "Stop one or more running containers", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { + for _, name := range args { + if !cli.CheckContainerName(name) { + return fmt.Errorf("container name %s is invalid", name) + } + } opts.containers = args opts.timeChanged = cmd.Flags().Changed("time") return runStop(dockerCli, &opts) diff --git a/cli/names.go b/cli/names.go new file mode 100644 index 000000000000..4b7c687125c9 --- /dev/null +++ b/cli/names.go @@ -0,0 +1,19 @@ +package cli + +import ( + "strings" + + "github.com/docker/cli/cli/names" +) + +var ( + validContainerNamePattern = names.RestrictedNamePattern +) + +// CheckContainerName check container's name is valid or not +func CheckContainerName(name string) bool { + if len(name) == 0 { + return false + } + return validContainerNamePattern.MatchString(strings.TrimPrefix(name, "/")) +} diff --git a/cli/names/names.go b/cli/names/names.go new file mode 100644 index 000000000000..1d77e3ab14ac --- /dev/null +++ b/cli/names/names.go @@ -0,0 +1,11 @@ +package names + +import ( + "regexp" +) + +// RestrictedNameChars collects the characters allowed to represent a name, normally used to validate container and volume names. +const RestrictedNameChars = `[a-zA-Z0-9][a-zA-Z0-9_.-]` + +// RestrictedNamePattern is a regular expression to validate names against the collection of restricted characters. +var RestrictedNamePattern = regexp.MustCompile(`^` + RestrictedNameChars + `+$`) From 518c3e61bcdffa8aff92ed19ced6bfe025255959 Mon Sep 17 00:00:00 2001 From: Lifubang Date: Mon, 3 Sep 2018 20:02:48 +0800 Subject: [PATCH 2/2] code optimization after review Signed-off-by: Lifubang --- cli/command/container/kill.go | 6 ++---- cli/command/container/rm.go | 6 ++---- cli/command/container/stop.go | 6 ++---- cli/names.go | 24 +++++++++++++++++------- cli/names/names.go | 11 ----------- 5 files changed, 23 insertions(+), 30 deletions(-) delete mode 100644 cli/names/names.go diff --git a/cli/command/container/kill.go b/cli/command/container/kill.go index e83a580bd77b..f1e8d8820eed 100644 --- a/cli/command/container/kill.go +++ b/cli/command/container/kill.go @@ -26,10 +26,8 @@ func NewKillCommand(dockerCli command.Cli) *cobra.Command { Short: "Kill one or more running containers", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - for _, name := range args { - if !cli.CheckContainerName(name) { - return fmt.Errorf("container name %s is invalid", name) - } + if err := cli.CheckContainerNames(args...); err != nil { + return err } opts.containers = args return runKill(dockerCli, &opts) diff --git a/cli/command/container/rm.go b/cli/command/container/rm.go index cf0be9c51b9c..3e738dbcf9bd 100644 --- a/cli/command/container/rm.go +++ b/cli/command/container/rm.go @@ -29,10 +29,8 @@ func NewRmCommand(dockerCli command.Cli) *cobra.Command { Short: "Remove one or more containers", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - for _, name := range args { - if !cli.CheckContainerName(name) { - return fmt.Errorf("container name %s is invalid", name) - } + if err := cli.CheckContainerNames(args...); err != nil { + return err } opts.containers = args return runRm(dockerCli, &opts) diff --git a/cli/command/container/stop.go b/cli/command/container/stop.go index 13ddcfd5bc59..7fd4f11dab11 100644 --- a/cli/command/container/stop.go +++ b/cli/command/container/stop.go @@ -28,10 +28,8 @@ func NewStopCommand(dockerCli command.Cli) *cobra.Command { Short: "Stop one or more running containers", Args: cli.RequiresMinArgs(1), RunE: func(cmd *cobra.Command, args []string) error { - for _, name := range args { - if !cli.CheckContainerName(name) { - return fmt.Errorf("container name %s is invalid", name) - } + if err := cli.CheckContainerNames(args...); err != nil { + return err } opts.containers = args opts.timeChanged = cmd.Flags().Changed("time") diff --git a/cli/names.go b/cli/names.go index 4b7c687125c9..aca492093c38 100644 --- a/cli/names.go +++ b/cli/names.go @@ -1,19 +1,29 @@ package cli import ( + "fmt" + "regexp" "strings" - - "github.com/docker/cli/cli/names" ) var ( - validContainerNamePattern = names.RestrictedNamePattern + validContainerNamePattern = regexp.MustCompile("^[a-zA-Z0-9][a-zA-Z0-9_.-]+$") ) // CheckContainerName check container's name is valid or not -func CheckContainerName(name string) bool { - if len(name) == 0 { - return false +func CheckContainerName(name string) error { + if !validContainerNamePattern.MatchString(strings.TrimPrefix(name, "/")) { + return fmt.Errorf("container name %s is invalid", name) + } + return nil +} + +// CheckContainerNames check containers' name is valid or not +func CheckContainerNames(names ...string) error { + for _, name := range names { + if err := CheckContainerName(name); err != nil { + return err + } } - return validContainerNamePattern.MatchString(strings.TrimPrefix(name, "/")) + return nil } diff --git a/cli/names/names.go b/cli/names/names.go deleted file mode 100644 index 1d77e3ab14ac..000000000000 --- a/cli/names/names.go +++ /dev/null @@ -1,11 +0,0 @@ -package names - -import ( - "regexp" -) - -// RestrictedNameChars collects the characters allowed to represent a name, normally used to validate container and volume names. -const RestrictedNameChars = `[a-zA-Z0-9][a-zA-Z0-9_.-]` - -// RestrictedNamePattern is a regular expression to validate names against the collection of restricted characters. -var RestrictedNamePattern = regexp.MustCompile(`^` + RestrictedNameChars + `+$`)