Repository: sonatype-nexus-community/nancy (go)

Issue:
panic when passing -q flag
I mistakenly tried to use `-q` instead of `-quiet` (forgetting the usage). Instead of being told of the bad flag, nancy instead panicked.

```
panic: runtime error: slice bounds out of range [:5] with length 2

goroutine 1 [running]:
github.com/sonatype-nexus-community/nancy/logger.init.0()
	/home/jake/go/pkg/mod/github.com/sonatype-nexus-community/nancy@v0.1.11/logger/logger.go:35 +0x1c5
```

Forgive my deletion of the template; it seemed like a feature request template whereas this is a bug.

I'm on `v0.1.11`.

cc @bhamail / @DarthHater


Pull request:
Fix panic when passing invalid flag. 
allow logging init() to succeed when invalid flag is passed.

It relates to the following issue #s:
* Fixes #100

cc @bhamail / @DarthHater


Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 3a15656..3685b00 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -24,8 +24,8 @@ jobs:
       - run: go mod download
       - run: go mod tidy
       - run: go get -u github.com/jstemmer/go-junit-report
+      - run: go build
       - run: go test ./... -v 2>&1 -p=1 | go-junit-report > $TEST_RESULTS/gotest/report.xml
-      - run: go build 
       - run: cd packages/testdata && ../../nancy Gopkg.lock && cd -
       - run: ./nancy go.sum
       - run: go list -m all | ./nancy
diff --git a/logger/logger.go b/logger/logger.go
index e8baa04..78aab8b 100644
--- a/logger/logger.go
+++ b/logger/logger.go
@@ -17,23 +17,30 @@ package logger
 
 import (
 	"fmt"
-	"os"
-	"path"
-
 	"github.com/sirupsen/logrus"
 	"github.com/sonatype-nexus-community/nancy/types"
+	"os"
+	"path"
+	"strings"
 )
 
-// DefaultLogFile can be overriden to use a different file name for upstream consumers
-var DefaultLogFile = "nancy.combined.log"
+const DefaultLogFilename = "nancy.combined.log"
+const TestLogfilename = "nancy.test.log"
+
+// DefaultLogFile can be overridden to use a different file name for upstream consumers
+var DefaultLogFile = DefaultLogFilename
 
 // LogLady can be obtained from outside the package, the name is a reference to the brilliant
 // actress in Twin Peaks
 var LogLady = logrus.New()
 
 func init() {
-	if len(os.Args) > 1 && os.Args[1] != "iq" && os.Args[1][:5] == "-test" {
-		DefaultLogFile = "nancy.test.log"
+	doInit(os.Args)
+}
+
+func doInit(args []string) {
+	if useTestLogFile(args) {
+		DefaultLogFile = TestLogfilename
 	}
 	file, err := os.OpenFile(GetLogFileLocation(), os.O_CREATE|os.O_APPEND|os.O_WRONLY, 0666)
 	if err != nil {
@@ -45,6 +52,31 @@ func init() {
 	LogLady.Formatter = &logrus.JSONFormatter{}
 }
 
+func stringPrefixInSlice(a string, list []string) bool {
+	for _, b := range list {
+		if strings.Contains(b, a) {
+			return true
+		}
+	}
+	return false
+}
+
+func stringInSlice(a string, list []string) bool {
+	for _, b := range list {
+		if b == a {
+			return true
+		}
+	}
+	return false
+}
+
+func useTestLogFile(args []string) bool {
+	if stringPrefixInSlice("-test.", args) && !stringInSlice("-iq", args) {
+		return true
+	}
+	return false
+}
+
 // GetLogFileLocation will return the location on disk of the log file
 func GetLogFileLocation() (result string) {
 	result, _ = os.UserHomeDir()


Test patch:
diff --git a/logger/logger_test.go b/logger/logger_test.go
index de5449a..7cad301 100644
--- a/logger/logger_test.go
+++ b/logger/logger_test.go
@@ -17,6 +17,7 @@ package logger
 
 import (
 	"encoding/json"
+	"github.com/stretchr/testify/assert"
 	"io/ioutil"
 	"os"
 	"strings"
@@ -30,8 +31,8 @@ func TestLogger(t *testing.T) {
 	// Do initial write to have a file
 	LogLady.Debug("Test")
 
-	if !strings.Contains(GetLogFileLocation(), "nancy.test.log") {
-		t.Error("Nancy test file not in log file location")
+	if !strings.Contains(GetLogFileLocation(), TestLogfilename) {
+		t.Errorf("Nancy test file not in log file location. args: %+v", os.Args)
 	}
 
 	err := os.Truncate(GetLogFileLocation(), 0)
@@ -64,3 +65,37 @@ type LogTest struct {
 	Msg   string `json:"msg"`
 	Time  string `json:"time"`
 }
+
+func setupTestCase(t *testing.T) func(t *testing.T) {
+	origDefault := DefaultLogFile
+	t.Logf("setup test case, origDefault: %+v", origDefault)
+	DefaultLogFile = DefaultLogFilename
+	return func(t *testing.T) {
+		t.Logf("teardown test case, origDefault: %+v", origDefault)
+		DefaultLogFile = origDefault
+	}
+}
+
+func TestInit(t *testing.T) {
+	teardownTestCase := setupTestCase(t)
+	defer teardownTestCase(t)
+
+	doInit([]string{"yadda", "-test.v"})
+	assert.Equal(t, TestLogfilename, DefaultLogFile)
+}
+
+func TestInitIQ(t *testing.T) {
+	teardownTestCase := setupTestCase(t)
+	defer teardownTestCase(t)
+
+	doInit([]string{"yadda", "-iq", "-test.v"})
+	assert.Equal(t, DefaultLogFilename, DefaultLogFile)
+}
+
+func TestInitDefault(t *testing.T) {
+	teardownTestCase := setupTestCase(t)
+	defer teardownTestCase(t)
+
+	doInit([]string{"yadda", "-yadda"})
+	assert.Equal(t, DefaultLogFilename, DefaultLogFile)
+}
diff --git a/main_test.go b/main_test.go
new file mode 100644
index 0000000..dc930aa
--- /dev/null
+++ b/main_test.go
@@ -0,0 +1,32 @@
+// Copyright 2020 Sonatype Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package main
+
+import (
+	"fmt"
+	"os/exec"
+	"strings"
+	"testing"
+)
+
+func TestBadArgs(t *testing.T) {
+	var err error
+	cmd := exec.Command("./nancy", "-bad")
+	out, err := cmd.CombinedOutput()
+	sout := string(out) // because out is []byte
+	if err != nil && !strings.Contains(sout, "flag provided but not defined: -bad") {
+		fmt.Println(sout) // so we can see the full output
+		t.Errorf("%v", err)
+	}
+}