From e29a81efc3df88b451a4a9464724a952d97b4115 Mon Sep 17 00:00:00 2001
From: Jakob Unterwurzacher <jakobunt@gmail.com>
Date: Tue, 5 Jun 2018 21:02:35 +0200
Subject: [PATCH] main: make prefixOArgs errors testable

By returning an error instead of calling os.Exit,
error cases can be tested easily. Error cases
were not tested until now.
---
 cli_args.go           | 19 +++++++++++--------
 cli_args_test.go      | 14 +++++++++++---
 tests/cli/cli_test.go | 12 ++++++++++++
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/cli_args.go b/cli_args.go
index 2f0c936..6253c14 100644
--- a/cli_args.go
+++ b/cli_args.go
@@ -42,16 +42,16 @@ var flagSet *flag.FlagSet
 // prefixOArgs transform options passed via "-o foo,bar" into regular options
 // like "-foo -bar" and prefixes them to the command line.
 // Testcases in TestPrefixOArgs().
-func prefixOArgs(osArgs []string) []string {
+func prefixOArgs(osArgs []string) ([]string, error) {
 	// Need at least 3, example: gocryptfs -o    foo,bar
 	//                               ^ 0    ^ 1    ^ 2
 	if len(osArgs) < 3 {
-		return osArgs
+		return osArgs, nil
 	}
 	// Passing "--" disables "-o" parsing. Ignore element 0 (program name).
 	for _, v := range osArgs[1:] {
 		if v == "--" {
-			return osArgs
+			return osArgs, nil
 		}
 	}
 	// Find and extract "-o foo,bar"
@@ -60,8 +60,7 @@ func prefixOArgs(osArgs []string) []string {
 		if osArgs[i] == "-o" {
 			// Last argument?
 			if i+1 >= len(osArgs) {
-				tlog.Fatal.Printf("The \"-o\" option requires an argument")
-				os.Exit(exitcodes.Usage)
+				return nil, fmt.Errorf("The \"-o\" option requires an argument")
 			}
 			oOpts = strings.Split(osArgs[i+1], ",")
 			// Skip over the arguments to "-o"
@@ -87,16 +86,20 @@ func prefixOArgs(osArgs []string) []string {
 	}
 	// Add other arguments
 	newArgs = append(newArgs, otherArgs...)
-	return newArgs
+	return newArgs, nil
 }
 
 // parseCliOpts - parse command line options (i.e. arguments that start with "-")
 func parseCliOpts() (args argContainer) {
-	os.Args = prefixOArgs(os.Args)
-
 	var err error
 	var opensslAuto string
 
+	os.Args, err = prefixOArgs(os.Args)
+	if err != nil {
+		tlog.Fatal.Println(err)
+		os.Exit(exitcodes.Usage)
+	}
+
 	flagSet = flag.NewFlagSet(tlog.ProgramName, flag.ContinueOnError)
 	flagSet.Usage = helpShort
 	flagSet.BoolVar(&args.debug, "d", false, "")
diff --git a/cli_args_test.go b/cli_args_test.go
index 63a33ef..dd5dcd1 100644
--- a/cli_args_test.go
+++ b/cli_args_test.go
@@ -10,6 +10,8 @@ type testcase struct {
 	i []string
 	// o is the expected output
 	o []string
+	// Do we expect an error?
+	e bool
 }
 
 // TestPrefixOArgs checks that the "-o x,y,z" parsing works correctly.
@@ -61,11 +63,17 @@ func TestPrefixOArgs(t *testing.T) {
 			i: []string{"gocryptfs", "--", "-o", "a"},
 			o: []string{"gocryptfs", "--", "-o", "a"},
 		},
+		// This should error out
+		{
+			i: []string{"gocryptfs", "foo", "bar", "-o"},
+			e: true,
+		},
 	}
 	for _, tc := range testcases {
-		o := prefixOArgs(tc.i)
-		if !reflect.DeepEqual(o, tc.o) {
-			t.Errorf("\n  in=%q\nwant=%q\n got=%q", tc.i, tc.o, o)
+		o, err := prefixOArgs(tc.i)
+		e := (err != nil)
+		if !reflect.DeepEqual(o, tc.o) || e != tc.e {
+			t.Errorf("\n  in=%q\nwant=%q err=%v\n got=%q err=%v", tc.i, tc.o, tc.e, o, e)
 		}
 	}
 }
diff --git a/tests/cli/cli_test.go b/tests/cli/cli_test.go
index 24bb029..8808742 100644
--- a/tests/cli/cli_test.go
+++ b/tests/cli/cli_test.go
@@ -448,3 +448,15 @@ func TestMultipleOperationFlags(t *testing.T) {
 		}
 	}
 }
+
+// Test that a missing argument to "-o" triggers exit code 1.
+// See also cli_args_test.go for comprehensive tests of "-o" parsing.
+func TestMissingOArg(t *testing.T) {
+	cmd := exec.Command(test_helpers.GocryptfsBinary, "foo", "bar", "-o")
+	err := cmd.Run()
+	exitCode := test_helpers.ExtractCmdExitCode(err)
+	if exitCode != exitcodes.Usage {
+		t.Fatalf("this should have failed with code %d, but returned %d",
+			exitcodes.Usage, exitCode)
+	}
+}