From f369c102c42f38b4dab92796ab3a567ebd84e0c9 Mon Sep 17 00:00:00 2001 From: Zachary Gershman Date: Wed, 21 Feb 2018 08:41:48 -0800 Subject: [PATCH 1/3] net/http: optionally add headers on redirect Fixes #19973 --- src/net/http/client.go | 65 +++++++++++++++++++++++-------------- src/net/http/client_test.go | 43 +++++++++++++----------- 2 files changed, 64 insertions(+), 44 deletions(-) diff --git a/src/net/http/client.go b/src/net/http/client.go index a02c805f38ec8f..58085b44df7fa3 100644 --- a/src/net/http/client.go +++ b/src/net/http/client.go @@ -75,6 +75,15 @@ type Client struct { // which is to stop after 10 consecutive requests. CheckRedirect func(req *Request, via []*Request) error + // HeadersPolicy specifies which headers will be sent on redirect. + // + // HeadersPolicy is called once per header to determine if that + // header should be sent along in a redirect request. + // + // If HeadersPolicy is nil, all headers will be forwarded on all + // subsequent redirects. + HeadersPolicy func(string) bool + // Jar specifies the cookie jar. // // The Jar is used to insert relevant cookies into every @@ -689,7 +698,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.HeadersPolicy) { req.Header[k] = vv } } @@ -698,6 +708,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") @@ -840,30 +879,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. // diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go index eea3b16fb3bd5b..6b4483e1fed762 100644 --- a/src/net/http/client_test.go +++ b/src/net/http/client_test.go @@ -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) From eefe49948ad2df7b9af2fea124ec4f1214a03e20 Mon Sep 17 00:00:00 2001 From: Zachary Gershman Date: Mon, 11 Mar 2019 08:39:21 -0700 Subject: [PATCH 2/3] net/http: Rename HeadersPolicy to RedirectHeader --- src/net/http/client.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/net/http/client.go b/src/net/http/client.go index 58085b44df7fa3..57bfc34ac9353c 100644 --- a/src/net/http/client.go +++ b/src/net/http/client.go @@ -75,14 +75,14 @@ type Client struct { // which is to stop after 10 consecutive requests. CheckRedirect func(req *Request, via []*Request) error - // HeadersPolicy specifies which headers will be sent on redirect. + // RedirectHeader specifies which headers will be sent on redirect. // - // HeadersPolicy is called once per header to determine if that + // RedirectHeader is called once per header to determine if that // header should be sent along in a redirect request. // - // If HeadersPolicy is nil, all headers will be forwarded on all + // If RedirectHeader is nil, all headers will be forwarded on all // subsequent redirects. - HeadersPolicy func(string) bool + RedirectHeader func(headerKey string) bool // Jar specifies the cookie jar. // @@ -699,7 +699,7 @@ func (c *Client) makeHeadersCopier(ireq *Request) func(*Request) { // (at least the safe ones). for k, vv := range ireqhdr { chk := CanonicalHeaderKey(k) - if shouldCopyHeaderOnRedirect(chk, preq.URL, req.URL, c.HeadersPolicy) { + if shouldCopyHeaderOnRedirect(chk, preq.URL, req.URL, c.RedirectHeader) { req.Header[k] = vv } } From dedd217f8dbf936aef280f7b4c824a0ae6ba521d Mon Sep 17 00:00:00 2001 From: Zachary Gershman Date: Wed, 17 Apr 2019 09:07:01 -0700 Subject: [PATCH 3/3] net/http: adds redirect header documentation --- src/net/http/client.go | 4 ++++ src/net/http/client_test.go | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/src/net/http/client.go b/src/net/http/client.go index 57bfc34ac9353c..096cf1757d4ed8 100644 --- a/src/net/http/client.go +++ b/src/net/http/client.go @@ -82,6 +82,10 @@ type Client struct { // // 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. diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go index 6b4483e1fed762..1ca574dc4d5fd6 100644 --- a/src/net/http/client_test.go +++ b/src/net/http/client_test.go @@ -1692,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)