Merge pull request #2311 from alliasgher/fix-boolwithinverse-string-panic

flag: prevent BoolWithInverseFlag.String from panicking without a tab

Author: dearchap

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:])
 }
 

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)
+	}
+}