Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strings: Map does not handle invalid UTF-8 correctly #26305

Closed
rsc opened this issue Jul 10, 2018 · 3 comments
Closed

strings: Map does not handle invalid UTF-8 correctly #26305

rsc opened this issue Jul 10, 2018 · 3 comments
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Jul 10, 2018

Map's loop does:

for i, c := range s {
	r := mapping(c)
	if r == c {

This is wrong if r is the error rune because s has invalid UTF-8. The loop should check this and flip to rewriting the string in that case.

@rsc rsc added this to the Go1.12 milestone Jul 10, 2018
@ianlancetaylor ianlancetaylor added help wanted NeedsFix The path to resolution is known, but the work has not been done. labels Jul 10, 2018
@rsc
Copy link
Contributor Author

rsc commented Jul 10, 2018

bytes.Map is OK because it is defined to always make a copy.

/cc @robpike

@martisch
Copy link
Contributor

martisch commented Jul 10, 2018

Updated my comment: "This is wrong if r is the error rune because s has invalid UTF-8" the problematic case mentioned here is if c is the error rune due to s having invalid UTF-8 and the mapping of c also produces the error rune as r not leading the string to be rewritten.

It is ok if r is the error rune and c is the error rune because s contained an error rune that maps to an error rune.

@martisch martisch self-assigned this Jul 10, 2018
@gopherbot
Copy link

Change https://golang.org/cl/131495 mentions this issue: strings: correctly handle invalid utf8 sequences in Map

@golang golang locked and limited conversation to collaborators Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants