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

proposal: spec: disallow Hangul filler codepoints in Go identifiers #40717

Closed
dolmen opened this issue Aug 12, 2020 · 6 comments
Closed

proposal: spec: disallow Hangul filler codepoints in Go identifiers #40717

dolmen opened this issue Aug 12, 2020 · 6 comments
Labels
Projects
Milestone

Comments

@dolmen
Copy link

@dolmen dolmen commented Aug 12, 2020

The Hangul filler codepoints (U+115F, U+1160, U+3164) are rendered as zero-width white space as specified by the Unicode standard. And they are allowed in Go identifiers.
https://twitter.com/omengue/status/1293476660268998656
They are only useful for obfuscation. Despites that would be a breaking change I propose to forbid them in Go 1 spec as I only see nasty uses.

What version of Go are you using (go version)?

$ go version
1.4.6

Does this issue reproduce with the latest release?

yes

What did you do?

var ᅟ, ᅠ, ㅤ = 0, 1, 2

Go Playground: https://play.golang.org/p/bOmUvRfC3Co

What did you expect to see?

Compile time failure.

What did you see instead?

Compiles fine.

@gopherbot gopherbot added this to the Proposal milestone Aug 12, 2020
@gopherbot gopherbot added the Proposal label Aug 12, 2020
@davecheney
Copy link
Contributor

@davecheney davecheney commented Aug 12, 2020

Apart from code golf, can these been used maliciously?

@dolmen
Copy link
Author

@dolmen dolmen commented Aug 12, 2020

@davecheney I see much potential for confusion in code review by hijacking names of basic types:

Go playground: https://play.golang.org/p/EYIrCh9XtI_u

func main() {
        var v interface{}
	fmt.Println(v)
	v = interfaceᅟ{}
	fmt.Println(v)
}

type interfacestruct{}

Output:

<nil>
{}
@davecheney
Copy link
Contributor

@davecheney davecheney commented Aug 12, 2020

I'd hope that widespread use of this style of identifier would be self correcting via natural selection.

Said another way, rather than changing the language, can this problem be solved by telling people that this isn't a good way to write code?

@martisch
Copy link
Contributor

@martisch martisch commented Aug 12, 2020

Relevant issue about homographs attacks with go/vet: #20115 Seems this would be a similar use case.

Referencing rsc in #20209 (comment)

I think we agree that the first step is to add a vet check and only after building experience with it think about actual language restrictions (or not). Marking this proposal-hold until there is a proposal (a short list would be fine) of what a vet "unicode" check would check.

@rsc rsc added this to Incoming in Proposals Aug 12, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Aug 12, 2020

The Hangul filler codepoints (U+115F, U+1160, U+3164) are rendered as zero-width white space as specified by the Unicode standard.

Looks like you missed U+FFA0.

But where in the Unicode standard does it say these are zero-width spaces?
They are classified as Lo (Letter, other).

I do see that they (including U+FFA0) have the Default_Ignorable_Code_Point property, and they are the only Letter-classified code points that do. So we should probably include them in the vet checks being contemplated in #20115.
Those vet checks are on hold for higher priority work.

Also, at least in the font I'm using, there's a clear indication that dodgy Unicode is happening:

Screen Shot 2020-08-12 at 10 35 52 AM

@rsc rsc changed the title Proposal: disallow Hangul filler codepoints in Go identifiers proposal: spec: disallow Hangul filler codepoints in Go identifiers Aug 12, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Aug 12, 2020

I suggest closing this as a duplicate of #20115.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Declined
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.