diff --git a/errors.go b/errors.go index f365a57990..ffd6471f23 100644 --- a/errors.go +++ b/errors.go @@ -150,10 +150,12 @@ func HandleExitCoder(err error) { } if exitErr, ok := err.(ExitCoder); ok { - if _, ok := exitErr.(ErrorFormatter); ok { - _, _ = fmt.Fprintf(ErrWriter, "%+v\n", err) - } else { - _, _ = fmt.Fprintln(ErrWriter, err) + if msg := err.Error(); msg != "" { + if _, ok := exitErr.(ErrorFormatter); ok { + _, _ = fmt.Fprintf(ErrWriter, "%+v\n", err) + } else { + _, _ = fmt.Fprintln(ErrWriter, err) + } } OsExiter(exitErr.ExitCode()) return diff --git a/errors_test.go b/errors_test.go index 35aaab54d4..6a497c58f5 100644 --- a/errors_test.go +++ b/errors_test.go @@ -232,3 +232,28 @@ func TestErrRequiredFlags_Error(t *testing.T) { expectedMsg = "Required flag \"flag1\" not set" assert.Equal(t, expectedMsg, err.Error()) } + +// TestHandleExitCoder_EmptyMessage is a regression test for +// https://github.com/urfave/cli/issues/2263. +// HandleExitCoder must not print an empty line to ErrWriter when the ExitCoder +// message is empty (e.g. cli.Exit("", code)). +func TestHandleExitCoder_EmptyMessage(t *testing.T) { + called := false + + OsExiter = func(rc int) { + if !called { + called = true + } + } + ErrWriter = &bytes.Buffer{} + + defer func() { + OsExiter = fakeOsExiter + ErrWriter = fakeErrWriter + }() + + HandleExitCoder(Exit("", 2)) + + assert.True(t, called) + assert.Empty(t, ErrWriter.(*bytes.Buffer).String(), "expected no output for empty-message exit") +} diff --git a/flag_bool_with_inverse.go b/flag_bool_with_inverse.go index 272dd98fec..680fe2c64e 100644 --- a/flag_bool_with_inverse.go +++ b/flag_bool_with_inverse.go @@ -179,6 +179,14 @@ func (bif *BoolWithInverseFlag) String() string { prefix = "-" } + // Guard against a FlagStringer that returns a string without a tab (e.g. + // a custom stringer or the default stringer when the flag does not + // implement DocGenerationFlag). In that case treat the entire output as + // the tab-delimited suffix so the slice never goes out of bounds. + if i < 0 { + i = 0 + } + return fmt.Sprintf("%s[%s]%s%s", prefix, bif.inversePrefix(), bif.Name, out[i:]) } diff --git a/flag_bool_with_inverse_test.go b/flag_bool_with_inverse_test.go index 981494586b..fbb7e43ff8 100644 --- a/flag_bool_with_inverse_test.go +++ b/flag_bool_with_inverse_test.go @@ -520,3 +520,26 @@ func TestBoolWithInverseFlag_SatisfiesVisibleFlagInterface(t *testing.T) { _ = f.IsVisible() } + +// TestBoolWithInverseFlagStringNoPanicWithNoTabStringer is a regression test for +// https://github.com/urfave/cli/issues/2303. +// BoolWithInverseFlag.String() panicked with "slice bounds out of range [-1:]" +// when the FlagStringer returned a string without a tab character. +func TestBoolWithInverseFlagStringNoPanicWithNoTabStringer(t *testing.T) { + orig := FlagStringer + defer func() { FlagStringer = orig }() + + FlagStringer = func(f Flag) string { + return "no tab here" + } + + flag := &BoolWithInverseFlag{ + Name: "verbose", + } + + // Must not panic. + got := flag.String() + if !strings.Contains(got, "verbose") { + t.Errorf("expected String() to contain the flag name, got %q", got) + } +}