Skip to content

net/http: optionally add headers on redirect #24004

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 44 additions & 25 deletions src/net/http/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,19 @@ type Client struct {
// which is to stop after 10 consecutive requests.
CheckRedirect func(req *Request, via []*Request) error

// RedirectHeader specifies which headers will be sent on redirect.
//
// RedirectHeader is called once per header to determine if that
// header should be sent along in a redirect request.
//
// If RedirectHeader is nil, all headers will be forwarded on all
// subsequent redirects.
//
// Headers "Authorization", "Www-Authenticate", "Cookie",
// and "Cookie2" bypass this function and are sent along
// on all redirects.
RedirectHeader func(headerKey string) bool

// Jar specifies the cookie jar.
//
// The Jar is used to insert relevant cookies into every
Expand Down Expand Up @@ -689,7 +702,8 @@ func (c *Client) makeHeadersCopier(ireq *Request) func(*Request) {
// Copy the initial request's Header values
// (at least the safe ones).
for k, vv := range ireqhdr {
if shouldCopyHeaderOnRedirect(k, preq.URL, req.URL) {
chk := CanonicalHeaderKey(k)
if shouldCopyHeaderOnRedirect(chk, preq.URL, req.URL, c.RedirectHeader) {
req.Header[k] = vv
}
}
Expand All @@ -698,6 +712,35 @@ func (c *Client) makeHeadersCopier(ireq *Request) func(*Request) {
}
}

func shouldCopyHeaderOnRedirect(canonicalHeaderKey string, initial, dest *url.URL, headersPolicy func(string) bool) bool {
switch canonicalHeaderKey {
case "Authorization", "Www-Authenticate", "Cookie", "Cookie2":
// Permit sending auth/cookie headers from "foo.com"
// to "sub.foo.com".

// Note that we don't send all cookies to subdomains
// automatically. This function is only used for
// Cookies set explicitly on the initial outgoing
// client request. Cookies automatically added via the
// CookieJar mechanism continue to follow each
// cookie's scope as set by Set-Cookie. But for
// outgoing requests with the Cookie header set
// directly, we don't know their scope, so we assume
// it's for *.domain.com.

ihost := canonicalAddr(initial)
dhost := canonicalAddr(dest)
return isDomainOrSubdomain(dhost, ihost)
}

// All other headers are copied:
if headersPolicy == nil {
return true
}

return headersPolicy(canonicalHeaderKey)
}

func defaultCheckRedirect(req *Request, via []*Request) error {
if len(via) >= 10 {
return errors.New("stopped after 10 redirects")
Expand Down Expand Up @@ -840,30 +883,6 @@ func (b *cancelTimerBody) Close() error {
return err
}

func shouldCopyHeaderOnRedirect(headerKey string, initial, dest *url.URL) bool {
switch CanonicalHeaderKey(headerKey) {
case "Authorization", "Www-Authenticate", "Cookie", "Cookie2":
// Permit sending auth/cookie headers from "foo.com"
// to "sub.foo.com".

// Note that we don't send all cookies to subdomains
// automatically. This function is only used for
// Cookies set explicitly on the initial outgoing
// client request. Cookies automatically added via the
// CookieJar mechanism continue to follow each
// cookie's scope as set by Set-Cookie. But for
// outgoing requests with the Cookie header set
// directly, we don't know their scope, so we assume
// it's for *.domain.com.

ihost := canonicalAddr(initial)
dhost := canonicalAddr(dest)
return isDomainOrSubdomain(dhost, ihost)
}
// All other headers are copied:
return true
}

// isDomainOrSubdomain reports whether sub is a subdomain (or exact
// match) of the parent domain.
//
Expand Down
45 changes: 25 additions & 20 deletions src/net/http/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1651,30 +1651,35 @@ func TestClientAltersCookiesOnRedirect(t *testing.T) {
// Part of Issue 4800
func TestShouldCopyHeaderOnRedirect(t *testing.T) {
tests := []struct {
header string
initialURL string
destURL string
want bool
header string
initialURL string
destURL string
headerPolicy func(string) bool
want bool
}{
{"User-Agent", "http://foo.com/", "http://bar.com/", true},
{"X-Foo", "http://foo.com/", "http://bar.com/", true},
// No policy, header always included:
{"User-Agent", "http://foo.com/", "http://bar.com/", nil, true},
{"X-Foo", "http://foo.com/", "http://bar.com/", nil, true},

// Policy provided, do not include header:
{"User-Agent", "", "", func(string) bool { return false }, true},

// Sensitive headers:
{"cookie", "http://foo.com/", "http://bar.com/", false},
{"cookie2", "http://foo.com/", "http://bar.com/", false},
{"authorization", "http://foo.com/", "http://bar.com/", false},
{"www-authenticate", "http://foo.com/", "http://bar.com/", false},
{"cookie", "http://foo.com/", "http://bar.com/", nil, false},
{"cookie2", "http://foo.com/", "http://bar.com/", nil, false},
{"authorization", "http://foo.com/", "http://bar.com/", nil, false},
{"www-authenticate", "http://foo.com/", "http://bar.com/", nil, false},

// But subdomains should work:
{"www-authenticate", "http://foo.com/", "http://foo.com/", true},
{"www-authenticate", "http://foo.com/", "http://sub.foo.com/", true},
{"www-authenticate", "http://foo.com/", "http://notfoo.com/", false},
{"www-authenticate", "http://foo.com/", "https://foo.com/", false},
{"www-authenticate", "http://foo.com:80/", "http://foo.com/", true},
{"www-authenticate", "http://foo.com:80/", "http://sub.foo.com/", true},
{"www-authenticate", "http://foo.com:443/", "https://foo.com/", true},
{"www-authenticate", "http://foo.com:443/", "https://sub.foo.com/", true},
{"www-authenticate", "http://foo.com:1234/", "http://foo.com/", false},
{"www-authenticate", "http://foo.com/", "http://foo.com/", nil, true},
{"www-authenticate", "http://foo.com/", "http://sub.foo.com/", nil, true},
{"www-authenticate", "http://foo.com/", "http://notfoo.com/", nil, false},
{"www-authenticate", "http://foo.com/", "https://foo.com/", nil, false},
{"www-authenticate", "http://foo.com:80/", "http://foo.com/", nil, true},
{"www-authenticate", "http://foo.com:80/", "http://sub.foo.com/", nil, true},
{"www-authenticate", "http://foo.com:443/", "https://foo.com/", nil, true},
{"www-authenticate", "http://foo.com:443/", "https://sub.foo.com/", nil, true},
{"www-authenticate", "http://foo.com:1234/", "http://foo.com/", nil, false},
}
for i, tt := range tests {
u0, err := url.Parse(tt.initialURL)
Expand All @@ -1687,7 +1692,7 @@ func TestShouldCopyHeaderOnRedirect(t *testing.T) {
t.Errorf("%d. dest URL %q parse error: %v", i, tt.destURL, err)
continue
}
got := Export_shouldCopyHeaderOnRedirect(tt.header, u0, u1)
got := Export_shouldCopyHeaderOnRedirect(tt.header, u0, u1, tt.headerPolicy)
if got != tt.want {
t.Errorf("%d. shouldCopyHeaderOnRedirect(%q, %q => %q) = %v; want %v",
i, tt.header, tt.initialURL, tt.destURL, got, tt.want)
Expand Down