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: net/http: add scheme constants #40587

Closed
ainar-g opened this issue Aug 5, 2020 · 10 comments
Closed

proposal: net/http: add scheme constants #40587

ainar-g opened this issue Aug 5, 2020 · 10 comments

Comments

@ainar-g
Copy link
Contributor

ainar-g commented Aug 5, 2020

Over the years I've noticed quite a lot of “magic” strings in HTTP-related code, primarily "http" and "https". There are a lot of examples in net/http/... as well as in cmd/....

So, I propose to add these constants to net/http (or, perhaps, net/url?):

const (
	Scheme = "http"
	Secure = "https"
)

Example usage:

var u = &url.URL{
	Scheme: http.Scheme,
	Host:   host,
	Path:   path.Join("users", userID),
}
if u.Scheme != http.Secure {
	return errors.New("insecure api")
}
@gopherbot gopherbot added this to the Proposal milestone Aug 5, 2020
@seankhliao
Copy link
Member

I think Scheme and Secure are not obvious to the reader, if anything I would prefer plain HTTP and HTTPS though it stutters

@ianlancetaylor ianlancetaylor added this to Incoming in Proposals (old) Aug 5, 2020
@ainar-g
Copy link
Contributor Author

ainar-g commented Aug 15, 2020

@seankhliao My reasoning was similar to that of sort.Interface. Then again, we do have log.Log, so I don't really have a strong opinion about the exact naming.

@davecheney
Copy link
Contributor

I don’t think these constants add anything for the reader. The literals http and https are descriptive, we’ll know, and unlikely to change at this point. A constant which represented these literals would be identical, except would be capitalised, which is misleading as HTTP schemes are not capitalised.

@ainar-g
Copy link
Contributor Author

ainar-g commented Aug 15, 2020

@davecheney My main concern here are possible misspellings. I've already seen http being misspelled as htp in a private project. Luckily, the tests caught it, but still.

@bradfitz
Copy link
Contributor

In addition to the stutter, writing http.HTTP and http.HTTPS is 3 characters longer than "http" and "https".

People felt strongly about adding the HTTP method constants too but in practice they're not used because "GET" looks better than http.MethodGet. I feel like this would be the same.

As such, I'm not in favor of this.

@ainar-g
Copy link
Contributor Author

ainar-g commented Aug 20, 2020

@bradfitz

[…] in practice they're not used because "GET" looks better than http.MethodGet. I feel like this would be the same.

That is not my experience, to be honest. Unless the code base is older than the named constants, I have almost always seen the constants preferred to literals.

As for the stuttering, my original proposal doesn't have that issue.

@davecheney
Copy link
Contributor

As for the stuttering, my original proposal doesn't have that issue

Accepted, but the names http.Scheme and http.Secure don’t describe their contents; the former sounds like the name of a type and the latter sounds like some kind of boolean

@ainar-g
Copy link
Contributor Author

ainar-g commented Sep 1, 2020

@davecheney I guess they could be http.SchemePlain and  http.SchemeSecure, just like we now have http.MethodGet and http.MethodPost. But that's way more typing.

I would hope that packages that implement other protocols would add their schemes in a similar fashion, so that we could have, say, ftp.Scheme and git.Scheme.

@rsc rsc moved this from Incoming to Active in Proposals (old) Oct 7, 2020
@rsc
Copy link
Contributor

rsc commented Oct 7, 2020

Based on the discussion, this seems like a likely decline.

@rsc rsc moved this from Active to Likely Decline in Proposals (old) Oct 7, 2020
@rsc
Copy link
Contributor

rsc commented Oct 14, 2020

No change in consensus, so declined.

@rsc rsc closed this as completed Oct 14, 2020
@rsc rsc moved this from Likely Decline to Declined in Proposals (old) Oct 14, 2020
@golang golang locked and limited conversation to collaborators Oct 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

No branches or pull requests

6 participants