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

net/http, net/textproto: add a Header method to get multiple values []string, with canonicalized key #34799

Closed
trung opened this issue Oct 9, 2019 · 7 comments
Labels
FeatureRequest FrozenDueToAge help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@trung
Copy link
Contributor

trung commented Oct 9, 2019

In order to get multiple values from a header, we have to use a standard map expression. However, we need to be aware that keys are canonicalized (header["My-Key"]) otherwise header["my-key"] will not return any value. Or need to use textproto.CanonicalMIMEHeaderKey() explicitly.

header.Get(key) allows to get the first value and internally uses textproto.CanonicalMIMEHeaderKey to canonicalize the key before getting the value.

Is there a reason not to provide, something likeheader.GetValues(key), which returns []string?

@smasher164 smasher164 changed the title http/header: allow to get multiple values with canonicalized key net/textproto: allow to get multiple values with canonicalized key Oct 9, 2019
@smasher164 smasher164 changed the title net/textproto: allow to get multiple values with canonicalized key net/textproto: get multiple values with canonicalized key Oct 9, 2019
@smasher164
Copy link
Member

Both the http.Header and textproto.MIMEHeader types specify that "[t]o access multiple values of a key, or to use non-canonical keys, access the map directly." Also, given how simply this function can be written yourself, why is it necessary to include it in the standard library?

func GetValues(h textproto.MIMEHeader, key string) []string {
	if h == nil {
		return nil
	}
	return h[textproto.CanonicalMIMEHeaderKey(key)]
}

@trung
Copy link
Contributor Author

trung commented Oct 9, 2019

Thanks @smasher164 for the explanation.

When using http.Client to invoke an HTTP API endpoint, all keys in the http.Response.Header are canonicalized automatically although the API server returns a non-canonicalized header key.

I thought it would be useful to provide such method so developers don't have to worry about case sensitivity when providing header key to get multiple values just like getting a single value.

I'm cool with having a utility function to perform the same. Hope no other developers would hit the similar confusion as I did.

@bradfitz
Copy link
Contributor

bradfitz commented Oct 9, 2019

I'd prefer to change the representation of net/http (and probably net/textproto) Header to not be a map and instead be a struct containing an internal map (along with other stuff), and enforce that we only store canonicalized keys.

That's part of #23707.

That said, a new method for now sounds fine. GetValues is a little weird in that the existing method Get isn't GetValue. GetAll? GetMulti? Just Values? (we prefer to avoid Get in general)

@andybons andybons added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 10, 2019
@andybons andybons added this to the Unplanned milestone Oct 10, 2019
@smasher164
Copy link
Member

I'm in favor of calling it Values, since it reads nicely when called as header.Values(key).

@bradfitz
Copy link
Contributor

I like Values, but the downside is that it doesn't sort next to Get in the godoc. Not the end of the world, I suppose.

@odeke-em odeke-em changed the title net/textproto: get multiple values with canonicalized key net/http, net/textproto: add a Header method to get multiple values []string, with canonicalized key Oct 10, 2019
@gopherbot
Copy link

Change https://golang.org/cl/200760 mentions this issue: net/http, net/textproto: add a Header method to get multiple values []string, with canonicalized key

@gopherbot
Copy link

Change https://golang.org/cl/217129 mentions this issue: doc/go1.14: mention net/http.Header.Values, net/textproto.MIMEHeader.Values

gopherbot pushed a commit that referenced this issue Jan 31, 2020
…Values

These methods are new in Go 1.14.

Updates #34799
Updates #36878

Change-Id: I063f5cb4638d7e0716e6ce2a8c3fffced51bbd34
Reviewed-on: https://go-review.googlesource.com/c/go/+/217129
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@golang golang locked and limited conversation to collaborators Jan 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FeatureRequest 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

5 participants