warn users again wrapping fmt.Sprintf()
tlimoncelli opened this issue · 0 comments
tlimoncelli commented
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.