From 96bbdc4abc5ee1f6f5d28c8d8dd705588fdfbc22 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Wed, 19 Aug 2026 07:00:53 +0100 Subject: [PATCH 1/4] fix: keep credentials on the host the request started from Header and BasicAuth restored credentials on every hop, including the ones the client stripped when a redirect left the original host, so a 302 to another host received them. Credentials are now set while the chain stays on the original host or one of its subdomains, matching how the standard client decides it, and SecretHeader gives the same protection to a custom header carrying a secret. --- README.md | 15 ++- middleware/header.go | 124 +++++++++++++++++++++++-- middleware/header_test.go | 191 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 320 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index c0320fc..2179f27 100644 --- a/README.md +++ b/README.md @@ -31,10 +31,11 @@ _Please note: this is not a replacement for `http.Client`, but rather a companio *Built-in middlewares:* - `Header` - appends user-defined headers to all requests. +- `SecretHeader` - appends a user-defined header carrying a credential, kept on the original host only - `MaxConcurrent` - sets maximum concurrency - `Retry` - sets retry on errors and status codes - `JSON` - sets headers `"Content-Type": "application/json"` and `"Accept": "application/json"` -- `BasicAuth(user, passwd string)` - adds HTTP Basic Authentication +- `BasicAuth(user, passwd string)` - adds HTTP Basic Authentication, kept on the original host only *Interfaces for external middlewares:* @@ -55,6 +56,16 @@ See examples of the usage in [_example](https://github.com/go-pkgz/requester/tre ```go rq := requester.New(http.Client{}, middleware.Header("X-Auth", "123456789")) ``` + +Headers carrying credentials, i.e. `Authorization`, `Www-Authenticate`, `Cookie`, `Cookie2`, `Proxy-Authorization` and `Proxy-Authenticate`, are dropped as soon as a followed redirect leaves the host the request started from, the same way `http.Client` treats them. Any other header is set on every hop. Hosts are compared as they are written, so the unicode and the punycode form of an internationalised host count as two hosts and the credential stays behind. + +### SecretHeader middleware + +`SecretHeader` adds a header the same way `Header` does, but treats it as a credential regardless of its name. The header is set while the redirect chain stays on the host the request started from, or on one of its subdomains, and dropped once the chain leaves it, a value the caller set on the request itself included. Use it for custom headers carrying a secret: + +```go +rq := requester.New(http.Client{}, middleware.SecretHeader("X-Auth", "123456789")) +``` ### MaxConcurrent middleware `MaxConcurrent` middleware can be used to limit the concurrency of a given requester and limit overall concurrency for multiple requesters. For the first case, `MaxConcurrent(N)` should be created in the requester chain of middlewares. For example, `rq := requester.New(http.Client{Timeout: 3 * time.Second}, middleware.MaxConcurrent(8))`. To make it global, `MaxConcurrent` should be created once, outside the chain, and passed into each requester. For example: @@ -189,6 +200,8 @@ rq := requester.New(http.Client{}, middleware.JSON) rq := requester.New(http.Client{}, middleware.BasicAuth("user", "passwd")) ``` +Credentials are set while the redirect chain stays on the host the request started from, or on one of its subdomains, and dropped once the chain leaves it. + ---- ### Logging middleware interface diff --git a/middleware/header.go b/middleware/header.go index fc2524e..5c9b5c1 100644 --- a/middleware/header.go +++ b/middleware/header.go @@ -1,18 +1,24 @@ package middleware import ( + "encoding/base64" "net/http" + "strings" ) -// Header middleware adds a header to request +// Header middleware adds a header to request. Headers carrying credentials, i.e. Authorization, Www-Authenticate, +// Cookie, Cookie2, Proxy-Authorization and Proxy-Authenticate, are dropped as soon as a redirect leaves the host the +// request started from, the same way the standard client treats them. Any other header, a custom one carrying a +// secret included, is set on every hop, see SecretHeader for the protected version. func Header(key, value string) func(http.RoundTripper) http.RoundTripper { - return func(next http.RoundTripper) http.RoundTripper { - fn := func(req *http.Request) (*http.Response, error) { - req.Header.Set(key, value) - return next.RoundTrip(req) - } - return RoundTripperFunc(fn) - } + return headerHandler(key, value, credentialHeader(key)) +} + +// SecretHeader middleware adds a header carrying a credential to request. The header is set while the redirect chain +// stays on the host the request started from, or on one of its subdomains, and dropped once the chain leaves it, +// including a value the caller set on the request itself. +func SecretHeader(key, value string) func(http.RoundTripper) http.RoundTripper { + return headerHandler(key, value, true) } // JSON sets Content-Type and Accept headers to json @@ -25,13 +31,113 @@ func JSON(next http.RoundTripper) http.RoundTripper { return RoundTripperFunc(fn) } -// BasicAuth middleware adds basic auth to request +// BasicAuth middleware adds basic auth to request. Credentials are set while the redirect chain stays on the host +// the request started from, or on one of its subdomains, and dropped once the chain leaves it. func BasicAuth(user, passwd string) func(http.RoundTripper) http.RoundTripper { return func(next http.RoundTripper) http.RoundTripper { + value := "Basic " + base64.StdEncoding.EncodeToString([]byte(user+":"+passwd)) fn := func(req *http.Request) (*http.Response, error) { + if !onOriginalHost(req) { + dropHeader(req.Header, "Authorization", value) + return next.RoundTrip(req) + } req.SetBasicAuth(user, passwd) return next.RoundTrip(req) } return RoundTripperFunc(fn) } } + +// headerHandler makes the middleware setting a single header, secret ones only while the request is on its original host +func headerHandler(key, value string, secret bool) func(http.RoundTripper) http.RoundTripper { + return func(next http.RoundTripper) http.RoundTripper { + fn := func(req *http.Request) (*http.Response, error) { + if secret && !onOriginalHost(req) { + dropHeader(req.Header, key, value) + return next.RoundTrip(req) + } + req.Header.Set(key, value) + return next.RoundTrip(req) + } + return RoundTripperFunc(fn) + } +} + +// dropHeader removes the credential from the request going to a host outside of the original one. +// +// The client copies the headers of the original request to every hop, dropping the credential ones on the way out, +// so for a key it doesn't recognise as a credential the whole header goes, the copy of the caller's own value +// included. For the keys it does recognise, values on this hop can belong to the destination instead, set by the +// cookie jar or by a CheckRedirect hook, so only the value the middleware sets is removed. +func dropHeader(header http.Header, key, value string) { + if !credentialHeader(key) { + header.Del(key) + return + } + + vals := header.Values(key) + kept := make([]string, 0, len(vals)) + for _, v := range vals { + if v != value { + kept = append(kept, v) + } + } + if len(kept) == 0 { + header.Del(key) + return + } + header[http.CanonicalHeaderKey(key)] = kept +} + +// credentialHeader reports if the header carries credentials, the set matching the one the standard client +// strips on a redirect to another host +func credentialHeader(key string) bool { + switch http.CanonicalHeaderKey(key) { + case "Authorization", "Www-Authenticate", "Cookie", "Cookie2", "Proxy-Authorization", "Proxy-Authenticate": + return true + } + return false +} + +// onOriginalHost reports if the request is still on the host the redirect chain started from, or on one of its +// subdomains. A request outside of a redirect chain is always on its own host. Once the chain left the original +// host the result stays negative for the rest of it, matching the standard client. +// +// The chain is walked over Response.Request, set by the standard transport and expected from any custom one. +// A redirect the origin can't be established for is treated as a hop away from it, and so is a hop between the +// unicode and the punycode form of the same internationalised host, which is compared as it is written. +func onOriginalHost(req *http.Request) bool { + if req.Response == nil { // not a redirect + return true + } + + origin := req + for origin.Response != nil { + if origin.Response.Request == nil { // broken chain, the origin is unknown + return false + } + origin = origin.Response.Request + } + + originHost := strings.ToLower(origin.URL.Hostname()) + for r := req; r != origin; r = r.Response.Request { + if !domainOrSubdomain(strings.ToLower(r.URL.Hostname()), originHost) { + return false + } + } + return true +} + +// domainOrSubdomain reports whether sub is the same domain as parent or a subdomain of it +func domainOrSubdomain(sub, parent string) bool { + if sub == parent { + return true + } + if strings.ContainsAny(sub, ":%") { // IPv6 address or a zone, never a hostname + return false + } + if !strings.HasSuffix(sub, parent) { + return false + } + return sub[len(sub)-len(parent)-1] == '.' +} diff --git a/middleware/header_test.go b/middleware/header_test.go index 0f1735d..2676544 100644 --- a/middleware/header_test.go +++ b/middleware/header_test.go @@ -1,7 +1,13 @@ package middleware import ( + "context" + "fmt" + "io" + "net" "net/http" + "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -169,3 +175,188 @@ func TestHeader_EdgeCases(t *testing.T) { require.NoError(t, err) }) } + +func TestHeader_Redirects(t *testing.T) { + // chain makes a request as the standard client does it for a redirect, i.e. every hop + // pointing to the response of the previous one + chain := func(t *testing.T, urls ...string) *http.Request { + t.Helper() + var prev *http.Request + for _, u := range urls { + req, err := http.NewRequest("GET", u, http.NoBody) + require.NoError(t, err) + if prev != nil { + req.Response = &http.Response{StatusCode: 302, Request: prev} + } + prev = req + } + return prev + } + + tbl := []struct { + name string + urls []string + kept bool + }{ + {"no redirect", []string{"http://example.com/a"}, true}, + {"same host", []string{"http://example.com/a", "http://example.com/b"}, true}, + {"subdomain of the original host", []string{"http://example.com/a", "http://sub.example.com/b"}, true}, + {"another port on the original host", []string{"http://example.com/a", "http://example.com:8443/b"}, true}, + {"host in a different case", []string{"http://example.com/a", "http://EXAMPLE.com/b"}, true}, + {"scheme change on the original host", []string{"http://example.com/a", "https://example.com/b"}, true}, + {"another host", []string{"http://example.com/a", "http://attacker.com/b"}, false}, + {"parent of the original host", []string{"http://sub.example.com/a", "http://example.com/b"}, false}, + {"host with the original as a suffix", []string{"http://example.com/a", "http://notexample.com/b"}, false}, + {"back on the original host after leaving it", + []string{"http://example.com/a", "http://attacker.com/b", "http://example.com/c"}, false}, + {"two hops on the original host", + []string{"http://example.com/a", "http://sub.example.com/b", "http://example.com/c"}, true}, + } + + for _, tt := range tbl { + t.Run(tt.name, func(t *testing.T) { + var got http.Header + rmock := &mocks.RoundTripper{RoundTripFunc: func(r *http.Request) (*http.Response, error) { + got = r.Header.Clone() + return &http.Response{StatusCode: 200}, nil + }} + + req := chain(t, tt.urls...) + // the client copies non-sensitive headers to the next hop, the middleware's own value must not survive it + req.Header.Set("Authorization", "Basic dXNlcjpwYXNzd2Q=") + req.Header.Set("X-Auth", "secret") + + h := BasicAuth("user", "passwd")(SecretHeader("X-Auth", "secret")(Header("X-Trace", "t1")(rmock))) + _, err := h.RoundTrip(req) + require.NoError(t, err) + + assert.Equal(t, "t1", got.Get("X-Trace"), "plain header goes on every hop") + if tt.kept { + assert.Equal(t, "Basic dXNlcjpwYXNzd2Q=", got.Get("Authorization")) + assert.Equal(t, "secret", got.Get("X-Auth")) + return + } + assert.Empty(t, got.Get("Authorization")) + assert.Empty(t, got.Get("X-Auth")) + }) + } + + t.Run("values of the destination kept on another host", func(t *testing.T) { + var got http.Header + rmock := &mocks.RoundTripper{RoundTripFunc: func(r *http.Request) (*http.Response, error) { + got = r.Header.Clone() + return &http.Response{StatusCode: 200}, nil + }} + + req := chain(t, "http://example.com/a", "http://attacker.com/b") + req.Header.Set("Authorization", "Basic dXNlcjpwYXNzd2Q=") // the middleware's own value + req.Header.Add("Authorization", "Bearer for-the-destination") + req.Header.Set("Cookie", "set=by-the-jar") + + h := BasicAuth("user", "passwd")(SecretHeader("Cookie", "set=by-the-middleware")(rmock)) + _, err := h.RoundTrip(req) + require.NoError(t, err) + + assert.Equal(t, []string{"Bearer for-the-destination"}, got.Values("Authorization")) + assert.Equal(t, []string{"set=by-the-jar"}, got.Values("Cookie")) + }) + + t.Run("caller value of a secret header dropped on another host", func(t *testing.T) { + var got http.Header + rmock := &mocks.RoundTripper{RoundTripFunc: func(r *http.Request) (*http.Response, error) { + got = r.Header.Clone() + return &http.Response{StatusCode: 200}, nil + }} + + // the client copies headers it doesn't treat as credentials from the original request to every hop + req := chain(t, "http://example.com/a", "http://attacker.com/b") + req.Header.Set("X-Auth", "set-by-the-caller") + + _, err := SecretHeader("X-Auth", "set-by-the-middleware")(rmock).RoundTrip(req) + require.NoError(t, err) + assert.Empty(t, got.Values("X-Auth")) + }) + + t.Run("redirect with unknown origin", func(t *testing.T) { + var got http.Header + rmock := &mocks.RoundTripper{RoundTripFunc: func(r *http.Request) (*http.Response, error) { + got = r.Header.Clone() + return &http.Response{StatusCode: 200}, nil + }} + + req, err := http.NewRequest("GET", "http://example.com/b", http.NoBody) + require.NoError(t, err) + req.Response = &http.Response{StatusCode: 302} // transport left Request unset, the origin can't be established + + _, err = BasicAuth("user", "passwd")(rmock).RoundTrip(req) + require.NoError(t, err) + assert.Empty(t, got.Get("Authorization")) + }) + + t.Run("credential header set through Header", func(t *testing.T) { + var got http.Header + rmock := &mocks.RoundTripper{RoundTripFunc: func(r *http.Request) (*http.Response, error) { + got = r.Header.Clone() + return &http.Response{StatusCode: 200}, nil + }} + + h := Header("authorization", "Bearer t")(Header("Cookie", "s=1")(rmock)) + _, err := h.RoundTrip(chain(t, "http://example.com/a", "http://attacker.com/b")) + require.NoError(t, err) + assert.Empty(t, got.Get("Authorization")) + assert.Empty(t, got.Get("Cookie")) + }) +} + +func TestHeader_RedirectsThroughClient(t *testing.T) { + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, err := fmt.Fprintf(w, "%s|%s|%s", r.Header.Get("Authorization"), r.Header.Get("X-Auth"), r.Header.Get("X-Trace")) + assert.NoError(t, err) + })) + defer target.Close() + + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "http://"+strings.TrimPrefix(r.URL.Path, "/to/")+"/target", http.StatusFound) + })) + defer origin.Close() + + // the middleware decides on the hostname, so the test hostnames resolve to the local servers + hosts := map[string]string{ + "origin.example:80": origin.Listener.Addr().String(), + "attacker.example:80": target.Listener.Addr().String(), + "sub.origin.example:80": target.Listener.Addr().String(), + } + base := &http.Transport{DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + if mapped, ok := hosts[addr]; ok { + addr = mapped + } + return (&net.Dialer{}).DialContext(ctx, network, addr) + }} + client := http.Client{Transport: BasicAuth("user", "passwd")(SecretHeader("X-Auth", "secret")(Header("X-Trace", "t1")(base)))} + + got := func(t *testing.T, redirectTo string) (auth, secret, trace string) { + t.Helper() + resp, err := client.Get("http://origin.example/to/" + redirectTo) + require.NoError(t, err) + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + parts := strings.Split(string(body), "|") + require.Len(t, parts, 3) + return parts[0], parts[1], parts[2] + } + + t.Run("credentials dropped on another host", func(t *testing.T) { + auth, secret, trace := got(t, "attacker.example") + assert.Empty(t, auth) + assert.Empty(t, secret) + assert.Equal(t, "t1", trace, "plain header still goes through") + }) + + t.Run("credentials kept on a subdomain of the original host", func(t *testing.T) { + auth, secret, trace := got(t, "sub.origin.example") + assert.Equal(t, "Basic dXNlcjpwYXNzd2Q=", auth) + assert.Equal(t, "secret", secret) + assert.Equal(t, "t1", trace) + }) +} From 22e1014de334a6ce11f39b361d99e067b1583cc1 Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Wed, 19 Aug 2026 07:54:21 +0100 Subject: [PATCH 2/4] fix: leave credential headers of the destination alone Off the original host the client copies no credential header from the original request, so what the middleware sees there belongs to the destination, set by a CheckRedirect hook or by the cookie jar. Recognised keys are now left untouched instead of being matched by value, which erased a destination value equal to the configured one. A header the client doesn't recognise as a credential is still removed whole, since that one does get copied to every hop. The middleware fills the request in on the response when the transport below leaves it unset, so a redirect through such a transport keeps its credentials on the original host. --- README.md | 10 +++--- middleware/header.go | 65 ++++++++++++++++----------------------- middleware/header_test.go | 37 ++++++++++++++++++---- 3 files changed, 62 insertions(+), 50 deletions(-) diff --git a/README.md b/README.md index 2179f27..0c3edcb 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ _Please note: this is not a replacement for `http.Client`, but rather a companio http.Client{Timeout: 5*time.Second}, // set http client requester.MaxConcurrent(8), // maximum number of concurrent requests requester.JSON, // set json headers - requester.Header("X-AUTH", "123456789"),// set some auth header + requester.SecretHeader("X-AUTH", "123456789"), // set some auth header, kept on the original host and its subdomains requester.Logger(requester.StdLogger), // enable logging to stdout ) @@ -54,14 +54,14 @@ See examples of the usage in [_example](https://github.com/go-pkgz/requester/tre `Header` middleware adds user-defined headers to all requests. It expects a map of headers to be added. For example: ```go -rq := requester.New(http.Client{}, middleware.Header("X-Auth", "123456789")) +rq := requester.New(http.Client{}, middleware.Header("X-Trace", "6f1a2b")) ``` -Headers carrying credentials, i.e. `Authorization`, `Www-Authenticate`, `Cookie`, `Cookie2`, `Proxy-Authorization` and `Proxy-Authenticate`, are dropped as soon as a followed redirect leaves the host the request started from, the same way `http.Client` treats them. Any other header is set on every hop. Hosts are compared as they are written, so the unicode and the punycode form of an internationalised host count as two hosts and the credential stays behind. +Headers carrying credentials, i.e. `Authorization`, `Www-Authenticate`, `Cookie`, `Cookie2`, `Proxy-Authorization` and `Proxy-Authenticate`, are not set once a followed redirect leaves the host the request started from, the same way `http.Client` treats them. Any other header is set on every hop. Hosts are compared as they are written, so the unicode and the punycode form of an internationalised host count as two hosts and the credential stays behind. ### SecretHeader middleware -`SecretHeader` adds a header the same way `Header` does, but treats it as a credential regardless of its name. The header is set while the redirect chain stays on the host the request started from, or on one of its subdomains, and dropped once the chain leaves it, a value the caller set on the request itself included. Use it for custom headers carrying a secret: +`SecretHeader` adds a header the same way `Header` does, but treats it as a credential regardless of its name. The header is set while the redirect chain stays on the host the request started from, or on one of its subdomains, and removed once the chain leaves it. Since `http.Client` copies a header it doesn't recognise as a credential to every hop, the removal covers the whole header, so a value the caller set on the request, or one a `CheckRedirect` hook set for the destination, goes with it. Use it for custom headers carrying a secret: ```go rq := requester.New(http.Client{}, middleware.SecretHeader("X-Auth", "123456789")) @@ -200,7 +200,7 @@ rq := requester.New(http.Client{}, middleware.JSON) rq := requester.New(http.Client{}, middleware.BasicAuth("user", "passwd")) ``` -Credentials are set while the redirect chain stays on the host the request started from, or on one of its subdomains, and dropped once the chain leaves it. +Credentials are set while the redirect chain stays on the host the request started from, or on one of its subdomains, and left out once the chain leaves it. A credential the client itself puts in for the destination, through a `CheckRedirect` hook or a cookie jar, is not touched. ---- diff --git a/middleware/header.go b/middleware/header.go index 5c9b5c1..ae29238 100644 --- a/middleware/header.go +++ b/middleware/header.go @@ -1,13 +1,12 @@ package middleware import ( - "encoding/base64" "net/http" "strings" ) // Header middleware adds a header to request. Headers carrying credentials, i.e. Authorization, Www-Authenticate, -// Cookie, Cookie2, Proxy-Authorization and Proxy-Authenticate, are dropped as soon as a redirect leaves the host the +// Cookie, Cookie2, Proxy-Authorization and Proxy-Authenticate, are not set once a redirect leaves the host the // request started from, the same way the standard client treats them. Any other header, a custom one carrying a // secret included, is set on every hop, see SecretHeader for the protected version. func Header(key, value string) func(http.RoundTripper) http.RoundTripper { @@ -15,7 +14,7 @@ func Header(key, value string) func(http.RoundTripper) http.RoundTripper { } // SecretHeader middleware adds a header carrying a credential to request. The header is set while the redirect chain -// stays on the host the request started from, or on one of its subdomains, and dropped once the chain leaves it, +// stays on the host the request started from, or on one of its subdomains, and removed once the chain leaves it, // including a value the caller set on the request itself. func SecretHeader(key, value string) func(http.RoundTripper) http.RoundTripper { return headerHandler(key, value, true) @@ -32,17 +31,16 @@ func JSON(next http.RoundTripper) http.RoundTripper { } // BasicAuth middleware adds basic auth to request. Credentials are set while the redirect chain stays on the host -// the request started from, or on one of its subdomains, and dropped once the chain leaves it. +// the request started from, or on one of its subdomains, and left out once the chain leaves it. func BasicAuth(user, passwd string) func(http.RoundTripper) http.RoundTripper { return func(next http.RoundTripper) http.RoundTripper { - value := "Basic " + base64.StdEncoding.EncodeToString([]byte(user+":"+passwd)) fn := func(req *http.Request) (*http.Response, error) { - if !onOriginalHost(req) { - dropHeader(req.Header, "Authorization", value) - return next.RoundTrip(req) + // off the original host the client copies no Authorization from the original request, so anything in + // the header belongs to the destination, put in by a CheckRedirect hook, and is left as it is + if onOriginalHost(req) { + req.SetBasicAuth(user, passwd) } - req.SetBasicAuth(user, passwd) - return next.RoundTrip(req) + return roundTrip(next, req) } return RoundTripperFunc(fn) } @@ -53,40 +51,29 @@ func headerHandler(key, value string, secret bool) func(http.RoundTripper) http. return func(next http.RoundTripper) http.RoundTripper { fn := func(req *http.Request) (*http.Response, error) { if secret && !onOriginalHost(req) { - dropHeader(req.Header, key, value) - return next.RoundTrip(req) + // the client copies headers it doesn't recognise as credentials from the original request to every + // hop, so the caller's own value of such a key goes as well. For the recognised ones it copies + // nothing off the original host and what is there belongs to the destination, left alone + if !credentialHeader(key) { + req.Header.Del(key) + } + return roundTrip(next, req) } req.Header.Set(key, value) - return next.RoundTrip(req) + return roundTrip(next, req) } return RoundTripperFunc(fn) } } -// dropHeader removes the credential from the request going to a host outside of the original one. -// -// The client copies the headers of the original request to every hop, dropping the credential ones on the way out, -// so for a key it doesn't recognise as a credential the whole header goes, the copy of the caller's own value -// included. For the keys it does recognise, values on this hop can belong to the destination instead, set by the -// cookie jar or by a CheckRedirect hook, so only the value the middleware sets is removed. -func dropHeader(header http.Header, key, value string) { - if !credentialHeader(key) { - header.Del(key) - return - } - - vals := header.Values(key) - kept := make([]string, 0, len(vals)) - for _, v := range vals { - if v != value { - kept = append(kept, v) - } - } - if len(kept) == 0 { - header.Del(key) - return +// roundTrip passes the request down the chain and fills the request in on the response if the transport below left +// it unset, keeping the chain onOriginalHost walks complete for any custom transport +func roundTrip(next http.RoundTripper, req *http.Request) (*http.Response, error) { + resp, err := next.RoundTrip(req) + if resp != nil && resp.Request == nil { + resp.Request = req } - header[http.CanonicalHeaderKey(key)] = kept + return resp, err //nolint:wrapcheck // the transport's error goes through the middleware as it is } // credentialHeader reports if the header carries credentials, the set matching the one the standard client @@ -103,9 +90,9 @@ func credentialHeader(key string) bool { // subdomains. A request outside of a redirect chain is always on its own host. Once the chain left the original // host the result stays negative for the rest of it, matching the standard client. // -// The chain is walked over Response.Request, set by the standard transport and expected from any custom one. -// A redirect the origin can't be established for is treated as a hop away from it, and so is a hop between the -// unicode and the punycode form of the same internationalised host, which is compared as it is written. +// The chain is walked over Response.Request, which the middleware fills in for the transport below it. A redirect +// the origin still can't be established for is treated as a hop away from it, and so is a hop between the unicode +// and the punycode form of the same internationalised host, which is compared as it is written. func onOriginalHost(req *http.Request) bool { if req.Response == nil { // not a redirect return true diff --git a/middleware/header_test.go b/middleware/header_test.go index 2676544..ad6f2e7 100644 --- a/middleware/header_test.go +++ b/middleware/header_test.go @@ -222,8 +222,7 @@ func TestHeader_Redirects(t *testing.T) { }} req := chain(t, tt.urls...) - // the client copies non-sensitive headers to the next hop, the middleware's own value must not survive it - req.Header.Set("Authorization", "Basic dXNlcjpwYXNzd2Q=") + // the client copies headers it doesn't treat as credentials to the next hop, X-Auth among them req.Header.Set("X-Auth", "secret") h := BasicAuth("user", "passwd")(SecretHeader("X-Auth", "secret")(Header("X-Trace", "t1")(rmock))) @@ -248,17 +247,20 @@ func TestHeader_Redirects(t *testing.T) { return &http.Response{StatusCode: 200}, nil }} + // the client copies no credential header to a host outside of the original one, so these belong to the + // destination, put in by a CheckRedirect hook or by the cookie jar, values equal to the configured ones + // included req := chain(t, "http://example.com/a", "http://attacker.com/b") - req.Header.Set("Authorization", "Basic dXNlcjpwYXNzd2Q=") // the middleware's own value + req.Header.Set("Authorization", "Basic dXNlcjpwYXNzd2Q=") req.Header.Add("Authorization", "Bearer for-the-destination") - req.Header.Set("Cookie", "set=by-the-jar") + req.Header.Set("Cookie", "set=by-the-middleware") h := BasicAuth("user", "passwd")(SecretHeader("Cookie", "set=by-the-middleware")(rmock)) _, err := h.RoundTrip(req) require.NoError(t, err) - assert.Equal(t, []string{"Bearer for-the-destination"}, got.Values("Authorization")) - assert.Equal(t, []string{"set=by-the-jar"}, got.Values("Cookie")) + assert.Equal(t, []string{"Basic dXNlcjpwYXNzd2Q=", "Bearer for-the-destination"}, got.Values("Authorization")) + assert.Equal(t, []string{"set=by-the-middleware"}, got.Values("Cookie")) }) t.Run("caller value of a secret header dropped on another host", func(t *testing.T) { @@ -277,6 +279,29 @@ func TestHeader_Redirects(t *testing.T) { assert.Empty(t, got.Values("X-Auth")) }) + t.Run("transport leaving the request out of the response", func(t *testing.T) { + var got http.Header + base := RoundTripperFunc(func(r *http.Request) (*http.Response, error) { + if r.URL.Path == "/a" { // the response carries no Request, as a custom transport may leave it + return &http.Response{ + StatusCode: http.StatusFound, + Header: http.Header{"Location": []string{"http://example.com/b"}}, + Body: io.NopCloser(strings.NewReader("")), + }, nil + } + got = r.Header.Clone() + return &http.Response{StatusCode: 200, Body: io.NopCloser(strings.NewReader(""))}, nil + }) + + client := http.Client{Transport: BasicAuth("user", "passwd")(base)} + resp, err := client.Get("http://example.com/a") + require.NoError(t, err) + require.NoError(t, resp.Body.Close()) + + // the middleware filled the request in on the way back, so the origin of the second hop is known + assert.Equal(t, "Basic dXNlcjpwYXNzd2Q=", got.Get("Authorization")) + }) + t.Run("redirect with unknown origin", func(t *testing.T) { var got http.Header rmock := &mocks.RoundTripper{RoundTripFunc: func(r *http.Request) (*http.Response, error) { From 4f259e84353db67da21517a53d0591a4fc93ecbb Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Wed, 19 Aug 2026 07:45:27 +0100 Subject: [PATCH 3/4] chore: update example dependencies lcw to v1.2.0, which drops hashicorp/errwrap and go-multierror and brings go-redis v9.18.0 and go.uber.org/atomic. The go directive stays at 1.23, so go-redis stops at v9.18.0: v9.19.0 needs go 1.24 and the current x/sys needs 1.25. --- _example/go.mod | 7 +++---- _example/go.sum | 29 ++++++++++++++--------------- 2 files changed, 17 insertions(+), 19 deletions(-) diff --git a/_example/go.mod b/_example/go.mod index 6084d1c..060e9ea 100644 --- a/_example/go.mod +++ b/_example/go.mod @@ -3,7 +3,7 @@ module github.com/go-pkgz/requester/_example go 1.23 require ( - github.com/go-pkgz/lcw v1.1.0 + github.com/go-pkgz/lcw v1.2.0 github.com/go-pkgz/repeater/v2 v2.2.0 github.com/go-pkgz/requester v1.0.0 ) @@ -12,10 +12,9 @@ require ( github.com/cespare/xxhash/v2 v2.3.0 // indirect github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f // indirect github.com/google/uuid v1.6.0 // indirect - github.com/hashicorp/errwrap v1.1.0 // indirect - github.com/hashicorp/go-multierror v1.1.1 // indirect github.com/hashicorp/golang-lru v1.0.2 // indirect - github.com/redis/go-redis/v9 v9.17.3 // indirect + github.com/redis/go-redis/v9 v9.18.0 // indirect + go.uber.org/atomic v1.11.0 // indirect ) replace github.com/go-pkgz/requester => ../ diff --git a/_example/go.sum b/_example/go.sum index 8850085..1129f17 100644 --- a/_example/go.sum +++ b/_example/go.sum @@ -1,7 +1,5 @@ -github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a h1:HbKu58rmZpUGpz5+4FfNmIU+FmZg2P3Xaj2v2bfNWmk= -github.com/alicebob/gopher-json v0.0.0-20200520072559-a9ecdc9d1d3a/go.mod h1:SGnFV6hVsYE877CKEZ6tDNTjaSXYUk6QqoIK6PrAtcc= -github.com/alicebob/miniredis/v2 v2.31.1 h1:7XAt0uUg3DtwEKW5ZAGa+K7FZV2DdKQo5K/6TTnfX8Y= -github.com/alicebob/miniredis/v2 v2.31.1/go.mod h1:UB/T2Uztp7MlFSDakaX1sTXUv5CASoprx0wulRT6HBg= +github.com/alicebob/miniredis/v2 v2.38.0 h1:nZAzCR+Lj+Vxk4ZXzm2NuKq2O33RXj1XxJ2e2uP9jiw= +github.com/alicebob/miniredis/v2 v2.38.0/go.mod h1:TcL7YfarKPGDAthEtl5NBeHZfeUQj6OXMm/+iu5cLMM= github.com/bsm/ginkgo/v2 v2.12.0 h1:Ny8MWAHyOepLGlLKYmXG4IEkioBysk6GpaRTLC8zwWs= github.com/bsm/ginkgo/v2 v2.12.0/go.mod h1:SwYbGRRDovPVboqFv0tPTcG1sN61LM1Z4ARdbAV9g4c= github.com/bsm/gomega v1.27.10 h1:yeMWxP2pV2fG3FgAODIY8EiRE3dy0aeFYt4l7wh6yKA= @@ -10,24 +8,25 @@ github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UF github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f h1:lO4WD4F/rVNCu3HqELle0jiPLLBs70cWOduZpkS1E78= github.com/dgryski/go-rendezvous v0.0.0-20200823014737-9f7001d12a5f/go.mod h1:cuUVRXasLTGF7a8hSLbxyZXjz+1KgoB3wDUb6vlszIc= -github.com/go-pkgz/lcw v1.1.0 h1:hDJdQJZf4iw19a7cTgQvF6Poz/L7mL4E7FgG5jGs4lA= -github.com/go-pkgz/lcw v1.1.0/go.mod h1:zwT7RSxFskQsHWHJezYq6n0iTn0n4yIYRDijXq3awM0= +github.com/go-pkgz/lcw v1.2.0 h1:P4qr2srBKuZZRPKY7xB1CJYMRVMV6vnKSTqlsuN349A= +github.com/go-pkgz/lcw v1.2.0/go.mod h1:DbA7qyDxoL7ZWKscm6KpxDYbex59bs/PXZ00WdNil2U= github.com/go-pkgz/repeater/v2 v2.2.0 h1:8nZR/NaknmLfx2YMHbr78u9OL4Xj+8+romm9dz4FpMg= github.com/go-pkgz/repeater/v2 v2.2.0/go.mod h1:RgX5vUbLKq7PV82QUDP5pFbQS1os4Z+U9XzKymK23A8= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= github.com/google/uuid v1.6.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo= -github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= -github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I= -github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= -github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= -github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/hashicorp/golang-lru v1.0.2 h1:dV3g9Z/unq5DpblPpw+Oqcv4dU/1omnb4Ok8iPY6p1c= github.com/hashicorp/golang-lru v1.0.2/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4= -github.com/redis/go-redis/v9 v9.17.3 h1:fN29NdNrE17KttK5Ndf20buqfDZwGNgoUr9qjl1DQx4= -github.com/redis/go-redis/v9 v9.17.3/go.mod h1:u410H11HMLoB+TP67dz8rL9s6QW2j76l0//kSOd3370= +github.com/klauspost/cpuid/v2 v2.0.9 h1:lgaqFMSdTdQYdZ04uHyN2d/eKdOMyi2YLSvlQIBFYa4= +github.com/klauspost/cpuid/v2 v2.0.9/go.mod h1:FInQzS24/EEf25PyTYn52gqo7WaD8xa0213Md/qVLRg= +github.com/redis/go-redis/v9 v9.18.0 h1:pMkxYPkEbMPwRdenAzUNyFNrDgHx9U+DrBabWNfSRQs= +github.com/redis/go-redis/v9 v9.18.0/go.mod h1:k3ufPphLU5YXwNTUcCRXGxUoF1fqxnhFQmscfkCoDA0= github.com/stretchr/testify v1.12.0 h1:K6Mr6jO9JICuend/5xzTM03ydSV3vdNRYAdPSukj8uI= github.com/stretchr/testify v1.12.0/go.mod h1:bOYBZb5qJ00vPzWfIqBUZPaxK8jWiXc6d3ErP4Ca9Gw= -github.com/yuin/gopher-lua v1.1.0 h1:BojcDhfyDWgU2f2TOzYK/g5p2gxMrku8oupLDqlnSqE= -github.com/yuin/gopher-lua v1.1.0/go.mod h1:GBR0iDaNXjAgGg9zfCvksxSRnQx76gclCIb7kdAd1Pw= +github.com/yuin/gopher-lua v1.1.1 h1:kYKnWBjvbNP4XLT3+bPEwAXJx262OhaHDWDVOPjL46M= +github.com/yuin/gopher-lua v1.1.1/go.mod h1:GBR0iDaNXjAgGg9zfCvksxSRnQx76gclCIb7kdAd1Pw= +github.com/zeebo/xxh3 v1.0.2 h1:xZmwmqxHZA8AI603jOQ0tMqmBr9lPeFwGg6d+xy9DC0= +github.com/zeebo/xxh3 v1.0.2/go.mod h1:5NWz9Sef7zIDm2JHfFlcQvNekmcEl9ekUZQQKCYaDcA= +go.uber.org/atomic v1.11.0 h1:ZvwS0R+56ePWxUNi+Atn9dWONBPp/AUETXlHW0DxSjE= +go.uber.org/atomic v1.11.0/go.mod h1:LUxbIzbOniOlMKjJjyPfpl4v+PKK2cNJn91OQbhoJI0= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= From ea1b31eab5cb474bbfaf074fd7d566e0d5f718fe Mon Sep 17 00:00:00 2001 From: Dmitry Verkhoturov Date: Wed, 19 Aug 2026 07:59:17 +0100 Subject: [PATCH 4/4] ci: limit the workflow token to reading the repository The job checks out, builds, lints and posts coverage to coveralls, none of which writes to the repository, so the token gets contents: read as the rest of the org does it. --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 66d9563..f780bd2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -6,6 +6,9 @@ on: tags: ["**"] pull_request: +permissions: + contents: read + jobs: build: runs-on: ubuntu-latest