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: crypto, hash: add WriteString support #14757

Closed
josharian opened this issue Mar 10, 2016 · 10 comments
Closed

proposal: crypto, hash: add WriteString support #14757

josharian opened this issue Mar 10, 2016 · 10 comments
Projects
Milestone

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Mar 10, 2016

Add WriteString to crypto/sha1, crypto/sha256, hash/crc64, etc. Similar to #11805. Probably easy pickings.

@josharian
Copy link
Contributor Author

@josharian josharian commented Mar 10, 2016

And see CL 20524 for a place this would get used.

@bradfitz bradfitz added this to the Unplanned milestone Apr 9, 2016
@sctb
Copy link
Contributor

@sctb sctb commented Apr 24, 2016

Initial CL submitted. I expect it's not quite there yet; glad to make any changes necessary.

@gopherbot
Copy link

@gopherbot gopherbot commented Apr 24, 2016

CL https://golang.org/cl/22422 mentions this issue.

@minux
Copy link
Member

@minux minux commented May 1, 2016

@josharian
Copy link
Contributor Author

@josharian josharian commented May 4, 2016

I think you're probably right. Before abandoning this, I'd like to do a bit of research in my Go corpus, but that might take a while.

@Scratch-net
Copy link

@Scratch-net Scratch-net commented Oct 17, 2019

Having WriteString might indirectly suggest using it to hash passwords, which is a very, very bad idea.

@FiloSottile FiloSottile changed the title crypto, hash: add WriteString support proposal: crypto, hash: add WriteString support Dec 2, 2019
@FiloSottile FiloSottile added the Proposal label Dec 2, 2019
@FiloSottile FiloSottile modified the milestones: Unplanned, Proposal Dec 2, 2019
@rsc rsc added this to Incoming in Proposals Dec 4, 2019
@rsc
Copy link
Contributor

@rsc rsc commented Dec 17, 2019

I am very skeptical of an unsafe implementation of WriteString here.
Even the one in os.File is not unsafe - it does Write([]byte(s)).
I don't believe crypto should set a precedent that "unsafe is OK if it's for performance",
for others to follow.

@rsc
Copy link
Contributor

@rsc rsc commented Jan 15, 2020

Based on the discussion above (and the general lack of interest over 4 years), this seems like a likely decline.

Leaving open for a week for final comments.

@rsc rsc moved this from Incoming to Likely Decline in Proposals Jan 15, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Jan 22, 2020

No change in consensus, so declined.

@rsc rsc closed this Jan 22, 2020
@rsc rsc moved this from Likely Decline to Accepted in Proposals Jan 22, 2020
@rsc rsc moved this from Accepted to Declined in Proposals Jan 22, 2020
@rsc
Copy link
Contributor

@rsc rsc commented Aug 5, 2020

Over on #38776 we realized that WriteString can be added without requiring use of unsafe, so maybe we should do that. WriteString is now included in #38776.

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
8 participants
You can’t perform that action at this time.