Skip to content

Commit f7ba82d

Browse files
oioojJay Conrod
authored and
Jay Conrod
committed
cmd/go/internal/web: don't follow an infinite number of redirects
We replaced http.DefaultClient with securityPreservingHTTPClient, but we still need that too many redirects check. This issue introduced by CL 156838. We introduce a special path to test rediret requests in the script test framework. You can specify the number of redirects in the path. $GOPROXY/redirect/<count>/... Redirect request sequence details(count=8): request: $GOPROXY/mod/redirect/8/rsc.io/quote/@v/v1.2.0.mod redirect: $GOPROXY/mod/redirect/7/rsc.io/quote/@v/v1.2.0.mod redirect: $GOPROXY/mod/redirect/6/rsc.io/quote/@v/v1.2.0.mod redirect: $GOPROXY/mod/redirect/5/rsc.io/quote/@v/v1.2.0.mod redirect: $GOPROXY/mod/redirect/4/rsc.io/quote/@v/v1.2.0.mod redirect: $GOPROXY/mod/redirect/3/rsc.io/quote/@v/v1.2.0.mod redirect: $GOPROXY/mod/redirect/2/rsc.io/quote/@v/v1.2.0.mod redirect: $GOPROXY/mod/redirect/1/rsc.io/quote/@v/v1.2.0.mod the last: $GOPROXY/mod/rsc.io/quote/@v/v1.2.0.mod Fixes #39482 Change-Id: I149a3702b2b616069baeef787b2e4b73afc93b0e Reviewed-on: https://go-review.googlesource.com/c/go/+/237177 Run-TryBot: Baokun Lee <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Jay Conrod <[email protected]>
1 parent 5adaa12 commit f7ba82d

File tree

3 files changed

+37
-0
lines changed

3 files changed

+37
-0
lines changed

src/cmd/go/internal/web/http.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ package web
1313

1414
import (
1515
"crypto/tls"
16+
"errors"
1617
"fmt"
1718
"mime"
1819
"net/http"
@@ -47,6 +48,13 @@ var securityPreservingHTTPClient = &http.Client{
4748
lastHop := via[len(via)-1].URL
4849
return fmt.Errorf("redirected from secure URL %s to insecure URL %s", lastHop, req.URL)
4950
}
51+
52+
// Go's http.DefaultClient allows 10 redirects before returning an error.
53+
// The securityPreservingHTTPClient also uses this default policy to avoid
54+
// Go command hangs.
55+
if len(via) >= 10 {
56+
return errors.New("stopped after 10 redirects")
57+
}
5058
return nil
5159
},
5260
}

src/cmd/go/proxy_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,25 @@ func proxyHandler(w http.ResponseWriter, r *http.Request) {
174174
return
175175
}
176176

177+
// Request for $GOPROXY/redirect/<count>/... goes to redirects.
178+
if strings.HasPrefix(path, "redirect/") {
179+
path = path[len("redirect/"):]
180+
if j := strings.Index(path, "/"); j >= 0 {
181+
count, err := strconv.Atoi(path[:j])
182+
if err != nil {
183+
return
184+
}
185+
186+
// The last redirect.
187+
if count <= 1 {
188+
http.Redirect(w, r, fmt.Sprintf("/mod/%s", path[j+1:]), 302)
189+
return
190+
}
191+
http.Redirect(w, r, fmt.Sprintf("/mod/redirect/%d/%s", count-1, path[j+1:]), 302)
192+
return
193+
}
194+
}
195+
177196
// Request for $GOPROXY/sumdb/<name>/supported
178197
// is checking whether it's OK to access sumdb via the proxy.
179198
if path == "sumdb/"+testSumDBName+"/supported" {
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
env GO111MODULE=on
2+
env GOPROXYBASE=$GOPROXY
3+
env GOPROXY=$GOPROXYBASE/redirect/11
4+
env GOSUMDB=off
5+
6+
! go get -d rsc.io/[email protected]
7+
stderr 'stopped after 10 redirects'
8+
9+
env GOPROXY=$GOPROXYBASE/redirect/9
10+
go get -d rsc.io/[email protected]

0 commit comments

Comments
 (0)