httputil: copy some headers over redirects #2896
Conversation
httputil/redirect17.go
Outdated
|
|
||
| func fixupHeadersForRedirect(req *http.Request, via []*http.Request) { | ||
| // preserve some headers across redirects | ||
| // to the CDN |
There was a problem hiding this comment.
This line merges well into the prior one. </nitpick>
httputil/redirect.go
Outdated
| "net/http" | ||
| ) | ||
|
|
||
| func checkRedirect(req *http.Request, via []*http.Request) error { |
There was a problem hiding this comment.
Feels a bit small to be on its own file.
| @@ -0,0 +1,37 @@ | |||
| // +build !go1.8 | |||
httputil/redirect17.go
Outdated
| // (this is done automatically, slightly more cleanly, from 1.8) | ||
| for k, v := range via[0].Header { | ||
| switch http.CanonicalHeaderKey(k) { | ||
| case "Authorization", "Www-Authenticate", "Cookie", "Cookie2": |
There was a problem hiding this comment.
Cookie2?
It would be nice to know why some fields are here and some are not. I'm not a HTTP guru to check this.
httputil/redirect17.go
Outdated
| for k, v := range via[0].Header { | ||
| switch http.CanonicalHeaderKey(k) { | ||
| case "Authorization", "Www-Authenticate", "Cookie", "Cookie2": | ||
| // whistle innocently |
There was a problem hiding this comment.
Given zygas comment, maybe we can give a slightly more elaborate comment here :) ? Maybe something like // Do not copy authorisation headers across redirects to not leak them and whistle innocently
httputil/redirect17.go
Outdated
| // preserve some headers across redirects (needed for the CDN) | ||
| // (this is done automatically, slightly more cleanly, from 1.8) | ||
| for k, v := range via[0].Header { | ||
| switch http.CanonicalHeaderKey(k) { |
There was a problem hiding this comment.
Can we please use strings.ToLower here instead? There's no benefit in having the canonical representation here, and it's very error prone to have to type exactly what the method returns for us to not have a bug ("Www" below feels very tempting to replace by "WWW").
niemeyer
changed the title
chipaca
force-pushed
the
headers-over-redirects
branch
from
66052e2 to
5da72b1
Compare
No description provided.