From 04e96d05fcb440bca266fe80a0182dd16a005608 Mon Sep 17 00:00:00 2001
From: Daniel J Walsh <dwalsh@redhat.com>
Date: Thu, 28 Sep 2017 18:44:48 +0000
Subject: [PATCH] validateFlags command line options to make sure the user
 entered a value

When a user enters a CLI with a StringFlags or StringSliceFlags and does not add
a value the CLI mistakently takes the next option and uses it as a value.

This usually ends up with an error like not enough options or others.  Some times
it could also succeed, with weird results.  This patch looks for any values that
begin with a "-" and return an error.

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
---
 cmd/kpod/common.go  | 33 +++++++++++++++++++++++++++++++++
 cmd/kpod/diff.go    |  3 +++
 cmd/kpod/export.go  |  3 +++
 cmd/kpod/history.go |  3 +++
 cmd/kpod/images.go  |  3 +++
 cmd/kpod/info.go    |  3 +++
 cmd/kpod/inspect.go |  3 +++
 cmd/kpod/kill.go    |  3 +++
 cmd/kpod/load.go    |  3 +++
 cmd/kpod/logs.go    |  3 +++
 cmd/kpod/mount.go   |  3 +++
 cmd/kpod/ps.go      |  3 +++
 cmd/kpod/pull.go    |  3 +++
 cmd/kpod/push.go    |  3 +++
 cmd/kpod/rename.go  |  3 +++
 cmd/kpod/rm.go      |  3 +++
 cmd/kpod/rmi.go     |  3 +++
 cmd/kpod/save.go    |  3 +++
 cmd/kpod/stats.go   |  3 +++
 cmd/kpod/stop.go    |  3 +++
 20 files changed, 90 insertions(+)

diff --git a/cmd/kpod/common.go b/cmd/kpod/common.go
index 1c1503e2..f77b3fd1 100644
--- a/cmd/kpod/common.go
+++ b/cmd/kpod/common.go
@@ -2,6 +2,8 @@ package main
 
 import (
 	"os"
+	"reflect"
+	"regexp"
 	"strings"
 
 	is "github.com/containers/image/storage"
@@ -100,3 +102,34 @@ func splitCamelCase(src string) string {
 	entries := camelcase.Split(src)
 	return strings.Join(entries, " ")
 }
+
+// validateFlags searches for StringFlags or StringSlice flags that never had
+// a value set.  This commonly occurs when the CLI mistakenly takes the next
+// option and uses it as a value.
+func validateFlags(c *cli.Context, flags []cli.Flag) error {
+	for _, flag := range flags {
+		switch reflect.TypeOf(flag).String() {
+		case "cli.StringSliceFlag":
+			{
+				f := flag.(cli.StringSliceFlag)
+				name := strings.Split(f.Name, ",")
+				val := c.StringSlice(name[0])
+				for _, v := range val {
+					if ok, _ := regexp.MatchString("^-.+", v); ok {
+						return errors.Errorf("option --%s requires a value", name[0])
+					}
+				}
+			}
+		case "cli.StringFlag":
+			{
+				f := flag.(cli.StringFlag)
+				name := strings.Split(f.Name, ",")
+				val := c.String(name[0])
+				if ok, _ := regexp.MatchString("^-.+", val); ok {
+					return errors.Errorf("option --%s requires a value", name[0])
+				}
+			}
+		}
+	}
+	return nil
+}
diff --git a/cmd/kpod/diff.go b/cmd/kpod/diff.go
index fa8c1d56..7e4f8c81 100644
--- a/cmd/kpod/diff.go
+++ b/cmd/kpod/diff.go
@@ -77,6 +77,9 @@ func diffCmd(c *cli.Context) error {
 	if len(c.Args()) != 1 {
 		return errors.Errorf("container, layer, or image name must be specified: kpod diff [options [...]] ID-NAME")
 	}
+	if err := validateFlags(c, diffFlags); err != nil {
+		return err
+	}
 	config, err := getConfig(c)
 	if err != nil {
 		return errors.Wrapf(err, "could not get config")
diff --git a/cmd/kpod/export.go b/cmd/kpod/export.go
index 53eb5080..94f05ce1 100644
--- a/cmd/kpod/export.go
+++ b/cmd/kpod/export.go
@@ -48,6 +48,9 @@ func exportCmd(c *cli.Context) error {
 		return errors.Errorf("too many arguments given, need 1 at most.")
 	}
 	container := args[0]
+	if err := validateFlags(c, exportFlags); err != nil {
+		return err
+	}
 
 	config, err := getConfig(c)
 	if err != nil {
diff --git a/cmd/kpod/history.go b/cmd/kpod/history.go
index f85cd134..2eff7a97 100644
--- a/cmd/kpod/history.go
+++ b/cmd/kpod/history.go
@@ -85,6 +85,9 @@ var (
 )
 
 func historyCmd(c *cli.Context) error {
+	if err := validateFlags(c, historyFlags); err != nil {
+		return err
+	}
 	config, err := getConfig(c)
 	if err != nil {
 		return errors.Wrapf(err, "Could not get config")
diff --git a/cmd/kpod/images.go b/cmd/kpod/images.go
index e1eb7660..535fdf34 100644
--- a/cmd/kpod/images.go
+++ b/cmd/kpod/images.go
@@ -52,6 +52,9 @@ var (
 )
 
 func imagesCmd(c *cli.Context) error {
+	if err := validateFlags(c, imagesFlags); err != nil {
+		return err
+	}
 	config, err := getConfig(c)
 	if err != nil {
 		return errors.Wrapf(err, "Could not get config")
diff --git a/cmd/kpod/info.go b/cmd/kpod/info.go
index b5400e93..22ca74c7 100644
--- a/cmd/kpod/info.go
+++ b/cmd/kpod/info.go
@@ -36,6 +36,9 @@ var (
 )
 
 func infoCmd(c *cli.Context) error {
+	if err := validateFlags(c, infoFlags); err != nil {
+		return err
+	}
 	info := map[string]interface{}{}
 
 	infoGivers := []infoGiverFunc{
diff --git a/cmd/kpod/inspect.go b/cmd/kpod/inspect.go
index d3411d3d..45e9d7e1 100644
--- a/cmd/kpod/inspect.go
+++ b/cmd/kpod/inspect.go
@@ -49,6 +49,9 @@ func inspectCmd(c *cli.Context) error {
 	if len(args) > 1 {
 		return errors.Errorf("too many arguments specified")
 	}
+	if err := validateFlags(c, inspectFlags); err != nil {
+		return err
+	}
 
 	itemType := c.String("type")
 	size := c.Bool("size")
diff --git a/cmd/kpod/kill.go b/cmd/kpod/kill.go
index bdce1875..8a550003 100644
--- a/cmd/kpod/kill.go
+++ b/cmd/kpod/kill.go
@@ -35,6 +35,9 @@ func killCmd(c *cli.Context) error {
 	if len(args) == 0 {
 		return errors.Errorf("specify one or more containers to kill")
 	}
+	if err := validateFlags(c, killFlags); err != nil {
+		return err
+	}
 	config, err := getConfig(c)
 	if err != nil {
 		return errors.Wrapf(err, "could not get config")
diff --git a/cmd/kpod/load.go b/cmd/kpod/load.go
index 6fba9eae..97401290 100644
--- a/cmd/kpod/load.go
+++ b/cmd/kpod/load.go
@@ -45,6 +45,9 @@ func loadCmd(c *cli.Context) error {
 	if len(args) > 1 {
 		return errors.New("too many arguments. Requires exactly 1")
 	}
+	if err := validateFlags(c, loadFlags); err != nil {
+		return err
+	}
 
 	runtime, err := getRuntime(c)
 	if err != nil {
diff --git a/cmd/kpod/logs.go b/cmd/kpod/logs.go
index 0f5fd8fa..60be4792 100644
--- a/cmd/kpod/logs.go
+++ b/cmd/kpod/logs.go
@@ -46,6 +46,9 @@ func logsCmd(c *cli.Context) error {
 	if len(args) != 1 {
 		return errors.Errorf("'kpod logs' requires exactly one container name/ID")
 	}
+	if err := validateFlags(c, logsFlags); err != nil {
+		return err
+	}
 	container := c.Args().First()
 	var opts libkpod.LogOptions
 	opts.Details = c.Bool("details")
diff --git a/cmd/kpod/mount.go b/cmd/kpod/mount.go
index 7b46ff14..a711bede 100644
--- a/cmd/kpod/mount.go
+++ b/cmd/kpod/mount.go
@@ -64,6 +64,9 @@ func mountCmd(c *cli.Context) error {
 	if len(args) > 1 {
 		return errors.Errorf("too many arguments specified")
 	}
+	if err := validateFlags(c, mountFlags); err != nil {
+		return err
+	}
 	config, err := getConfig(c)
 	if err != nil {
 		return errors.Wrapf(err, "Could not get config")
diff --git a/cmd/kpod/ps.go b/cmd/kpod/ps.go
index b8bc7f0e..2cd99433 100644
--- a/cmd/kpod/ps.go
+++ b/cmd/kpod/ps.go
@@ -143,6 +143,9 @@ var (
 )
 
 func psCmd(c *cli.Context) error {
+	if err := validateFlags(c, psFlags); err != nil {
+		return err
+	}
 	config, err := getConfig(c)
 	if err != nil {
 		return errors.Wrapf(err, "could not get config")
diff --git a/cmd/kpod/pull.go b/cmd/kpod/pull.go
index e7e78cea..02ce01f0 100644
--- a/cmd/kpod/pull.go
+++ b/cmd/kpod/pull.go
@@ -129,6 +129,9 @@ func pullCmd(c *cli.Context) error {
 		logrus.Errorf("too many arguments. Requires exactly 1")
 		return nil
 	}
+	if err := validateFlags(c, pullFlags); err != nil {
+		return err
+	}
 	image := args[0]
 	srcRef, err := alltransports.ParseImageName(image)
 	if err != nil {
diff --git a/cmd/kpod/push.go b/cmd/kpod/push.go
index ffc477c0..a019f54a 100644
--- a/cmd/kpod/push.go
+++ b/cmd/kpod/push.go
@@ -68,6 +68,9 @@ func pushCmd(c *cli.Context) error {
 	if len(args) < 2 {
 		return errors.New("kpod push requires exactly 2 arguments")
 	}
+	if err := validateFlags(c, pushFlags); err != nil {
+		return err
+	}
 	srcName := c.Args().Get(0)
 	destName := c.Args().Get(1)
 
diff --git a/cmd/kpod/rename.go b/cmd/kpod/rename.go
index fe1f08d9..b638856e 100644
--- a/cmd/kpod/rename.go
+++ b/cmd/kpod/rename.go
@@ -23,6 +23,9 @@ func renameCmd(c *cli.Context) error {
 	if len(c.Args()) != 2 {
 		return errors.Errorf("Rename requires a src container name/ID and a dest container name")
 	}
+	if err := validateFlags(c, renameFlags); err != nil {
+		return err
+	}
 
 	config, err := getConfig(c)
 	if err != nil {
diff --git a/cmd/kpod/rm.go b/cmd/kpod/rm.go
index bed72642..69f68302 100644
--- a/cmd/kpod/rm.go
+++ b/cmd/kpod/rm.go
@@ -33,6 +33,9 @@ func rmCmd(c *cli.Context) error {
 	if len(args) == 0 {
 		return errors.Errorf("specify one or more containers to remove")
 	}
+	if err := validateFlags(c, rmFlags); err != nil {
+		return err
+	}
 
 	config, err := getConfig(c)
 	if err != nil {
diff --git a/cmd/kpod/rmi.go b/cmd/kpod/rmi.go
index ba2b1ea0..fc4056d7 100644
--- a/cmd/kpod/rmi.go
+++ b/cmd/kpod/rmi.go
@@ -28,6 +28,9 @@ var (
 )
 
 func rmiCmd(c *cli.Context) error {
+	if err := validateFlags(c, rmiFlags); err != nil {
+		return err
+	}
 
 	force := false
 	if c.IsSet("force") {
diff --git a/cmd/kpod/save.go b/cmd/kpod/save.go
index b1413a7e..f04ba463 100644
--- a/cmd/kpod/save.go
+++ b/cmd/kpod/save.go
@@ -47,6 +47,9 @@ func saveCmd(c *cli.Context) error {
 	if len(args) == 0 {
 		return errors.Errorf("need at least 1 argument")
 	}
+	if err := validateFlags(c, saveFlags); err != nil {
+		return err
+	}
 
 	runtime, err := getRuntime(c)
 	if err != nil {
diff --git a/cmd/kpod/stats.go b/cmd/kpod/stats.go
index b3ea6879..0bf2e263 100644
--- a/cmd/kpod/stats.go
+++ b/cmd/kpod/stats.go
@@ -62,6 +62,9 @@ var (
 )
 
 func statsCmd(c *cli.Context) error {
+	if err := validateFlags(c, statsFlags); err != nil {
+		return err
+	}
 	config, err := getConfig(c)
 	if err != nil {
 		return errors.Wrapf(err, "could not read config")
diff --git a/cmd/kpod/stop.go b/cmd/kpod/stop.go
index 49ef483c..06b26bb9 100644
--- a/cmd/kpod/stop.go
+++ b/cmd/kpod/stop.go
@@ -42,6 +42,9 @@ func stopCmd(c *cli.Context) error {
 	if len(args) < 1 {
 		return errors.Errorf("you must provide at least one container name or id")
 	}
+	if err := validateFlags(c, stopFlags); err != nil {
+		return err
+	}
 
 	config, err := getConfig(c)
 	if err != nil {