fatih/color

warn users again wrapping fmt.Sprintf()

tlimoncelli opened this issue · 0 comments

I recently saw this being submitted in a PR:

return color.RedString(fmt.Sprintf("- DELETE %s", a))

That works, but this is better:

return color.RedString("- DELETE %s", a)

If someone could force user-input so that a := "%v" then color.RedString() would try to interpret the string as a format string, which would be an error. I suspect this could be leveraged to be a security issue.

The README.md should be clear that color.FOOString(fmt.Sprintf(a, b)) is an anti-pattern, and that only color.FOOString(a,b) is needed.

IMHO any function that accepts a format string should have a name that ends with f, such as color.RedStringf(). I realize this is a breaking change, and I hope you consider this change in v2.