From 1a5acddca2a69bd1a2861bc3f351924ae39a0798 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20M=C3=ADchal?= Date: Wed, 2 Dec 2020 14:10:40 +0100 Subject: [PATCH] Correctly check validity of container name regexp.MatchString() only returns an error if the pattern can't be parsed. In this case, the pattern is a constant string literal, so unless there's a programming mistake, the pattern should always be parsable and there should never be an error. What really needs to be checked is whether the 'containerName' matched the pattern or not. That's indicated by the bool return value 'matched'. https://github.com/containers/toolbox/pull/639 --- src/cmd/create.go | 2 +- src/cmd/enter.go | 2 +- src/cmd/run.go | 2 +- src/pkg/utils/utils.go | 9 +++++++-- test/system/102-create.bats | 2 +- 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/cmd/create.go b/src/cmd/create.go index 06008c9..c48698b 100644 --- a/src/cmd/create.go +++ b/src/cmd/create.go @@ -115,7 +115,7 @@ func create(cmd *cobra.Command, args []string) error { } if container != "" { - if _, err := utils.IsContainerNameValid(container); err != nil { + if !utils.IsContainerNameValid(container) { var builder strings.Builder fmt.Fprintf(&builder, "invalid argument for '%s'\n", containerArg) fmt.Fprintf(&builder, "Container names must match '%s'\n", utils.ContainerNameRegexp) diff --git a/src/cmd/enter.go b/src/cmd/enter.go index 4f25167..6763d5e 100644 --- a/src/cmd/enter.go +++ b/src/cmd/enter.go @@ -86,7 +86,7 @@ func enter(cmd *cobra.Command, args []string) error { if container != "" { nonDefaultContainer = true - if _, err := utils.IsContainerNameValid(container); err != nil { + if !utils.IsContainerNameValid(container) { var builder strings.Builder fmt.Fprintf(&builder, "invalid argument for '%s'\n", containerArg) fmt.Fprintf(&builder, "Container names must match '%s'\n", utils.ContainerNameRegexp) diff --git a/src/cmd/run.go b/src/cmd/run.go index 3fff3a2..054465b 100644 --- a/src/cmd/run.go +++ b/src/cmd/run.go @@ -81,7 +81,7 @@ func run(cmd *cobra.Command, args []string) error { if runFlags.container != "" { nonDefaultContainer = true - if _, err := utils.IsContainerNameValid(runFlags.container); err != nil { + if !utils.IsContainerNameValid(runFlags.container) { var builder strings.Builder fmt.Fprintf(&builder, "invalid argument for '--container'\n") fmt.Fprintf(&builder, "Container names must match '%s'\n", utils.ContainerNameRegexp) diff --git a/src/pkg/utils/utils.go b/src/pkg/utils/utils.go index 0575e80..73ce29b 100644 --- a/src/pkg/utils/utils.go +++ b/src/pkg/utils/utils.go @@ -497,10 +497,15 @@ func PathExists(path string) bool { } // IsContainerNameValid checks if the name of a container matches the right pattern -func IsContainerNameValid(containerName string) (bool, error) { +func IsContainerNameValid(containerName string) bool { pattern := "^" + ContainerNameRegexp + "$" matched, err := regexp.MatchString(pattern, containerName) - return matched, err + if err != nil { + panicMsg := fmt.Sprintf("failed to parse regular expression for container name: %v", err) + panic(panicMsg) + } + + return matched } func IsInsideContainer() bool { diff --git a/test/system/102-create.bats b/test/system/102-create.bats index ecc9bcd..9041c25 100644 --- a/test/system/102-create.bats +++ b/test/system/102-create.bats @@ -16,5 +16,5 @@ load helpers @test "Try to create a container with invalid custom name" { run_toolbox 1 -y create "ßpeci@l.Nam€" - is "${lines[0]}" "Error: failed to create container ßpeci@l.Nam€" "Toolbox should fail to create a container with such name" + is "${lines[0]}" "Error: invalid argument for 'CONTAINER'" "Toolbox should fail to create a container with such name" }