cmd/vet: false positive printf detection for URL-encoded / (%2F) in string literal
#29854
Comments
|
The real question is why vet thinks that Println is a printf wrapper when it isn't. If it was in fact a printf wrapper, then the report would be correct, and you should use |
|
I think it's more calling out that this print call may have been intended to be a Printf call, but was inadvertently coded as Println instead, causing the directive to be ignored and any arguments simply appended. And that's a reasonable and fairly useful check. Just kinda falls down when you're using Println to print out URL-encoded characters that mimic actual formatting directives. |
|
You're right, of course. |
|
Perhaps we could teach vet to ignore certain common false positives like @alandonovan @josharian thoughts? |
mvdan
added
the
NeedsDecision
|
Yes, checking for %XX (uppercase hex not starting with zero) might be the simplest workaround for all the URL encoding cases you mention. There will still be occasional false positives, of course. |
|
IMO it shouldn't flag calls that have a single argument, either. It is unlikely that someone uses Println instead of Printf by accident and forgets to provide arguments for the format verbs. |
adonovan
added
the
Analysis
|
just ran into this package main
import "fmt"
func main() {
fmt.Println("http://example.com?at=AAAAAAAAAAAU%2FEmq2DGAxNGYi71fZdi2")
}fix is using Printf, but that's really annoying/pointless: package main
import "fmt"
func main() {
fmt.Printf("%v\n", "http://example.com?at=AAAAAAAAAAAU%2FEmq2DGAxNGYi71fZdi2")
} |
|
Apologies that this little bug languished for 6 years. Fix pending. |
|
Change https://go.dev/cl/650175 mentions this issue: |
|
uh Z is not a hex digit... |
Thanks. It's late here. |
What version of Go are you using (
go version)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env)?go envOutputWhat did you do?
Using the logging library github.com/spf13/jwalterweatherman, I have this line in our logs:
This is supposed to print a URL to console that the user can click to go to the logs. Since the URL includes a slash-separated string as a parameter, those slashes have to be URL-encoded, which for a forward slach is
%2F.What did you expect to see?
No vet error
What did you see instead?
vet fires an error:
Normally this wouldn't be an issue, but as of Go 1.10,
go testnow runsgo vetnatively with a hardcoded list of checks, and the only option is to either convert this to a Printf and doubling the percent signs to escape them, or disablego vetduringgo test, neither of which is a fantastic solution.Recommendations / possible resolutions:
fcall and neglected to include the value to replace it with in the formatted string).go testautomatic run ofgo vet, possibly via a flag like-vetflags="-printf=false". This would also allow any of the other vet-specific flags to be passed down to it.The text was updated successfully, but these errors were encountered: