Skip to content
This repository has been archived by the owner on May 9, 2021. It is now read-only.

Check for invalid UTF-8 encoded strings in cutsets #252

Closed
dsnet opened this issue Oct 26, 2016 · 4 comments
Closed

Check for invalid UTF-8 encoded strings in cutsets #252

dsnet opened this issue Oct 26, 2016 · 4 comments

Comments

@dsnet
Copy link
Member

dsnet commented Oct 26, 2016

The (strings|bytes).(IndexAny|LastIndexAny|ContainsAny|Trim|TrimLeft|TrimRight) functions take an argument for a set of UTF-8 encoded characters to match on. In production code, I have seen incorrect uses of these functions where the user treats chars as simply a list of bytes, rather UTF-8 encoded characters.

Most likely, any invalid chars input is due to user error. The following is probably not what a user expects:

fmt.Printf("%q\n", strings.Trim("\x80test\xff", "\xff"))

This currently prints test, while the user expectation is probably \x80test.

@dominikh
Copy link
Member

@dsnet Is golint the right place for this check? AFAIK, golint is about style, not correctness. I'm currently trying to decide between sending a PR to golint and adding the check to my own staticcheck.

@dsnet
Copy link
Member Author

dsnet commented Nov 12, 2016

I'm actually not sure. You are right that this is an issue about correctness, so it doesn't quite fit as a lint check either. According to golang/go#17780, the criterion for a vet check is: correctness, frequency, and precision.

When I looked over a large number of Go code inside Google, this mishap only occurred a few dozen times, so it probably doesn't hit the frequency requirements. If you want to add it to staticcheck, that would sound great.

dominikh added a commit to dominikh/go-staticcheck that referenced this issue Nov 13, 2016
Inspired by golang/lint#252

Idea-By: Joe Tsai <joetsai@digital-static.net>
@dominikh
Copy link
Member

I've added the check to staticcheck. Running it against my corpus also yielded a very low number of results, so most likely not suitable for vet.

@dsnet
Copy link
Member Author

dsnet commented Nov 14, 2016

I'm going to close this issue since the feature request fits neither vet nor lint. Thanks @dominikh for adding it to staticcheck.

@dsnet dsnet closed this as completed Nov 14, 2016
dominikh added a commit to dominikh/go-tools that referenced this issue Jan 24, 2017
Inspired by golang/lint#252

Idea-By: Joe Tsai <joetsai@digital-static.net>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants