From f7ba82d68f90e20aa9e6aa973cb6f12321abec71 Mon Sep 17 00:00:00 2001
From: Baokun Lee <nototon@gmail.com>
Date: Tue, 9 Jun 2020 23:03:35 +0800
Subject: [PATCH] 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 <nototon@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
---
 src/cmd/go/internal/web/http.go               |  8 ++++++++
 src/cmd/go/proxy_test.go                      | 19 +++++++++++++++++++
 .../script/mod_get_too_many_redirects.txt     | 10 ++++++++++
 3 files changed, 37 insertions(+)
 create mode 100644 src/cmd/go/testdata/script/mod_get_too_many_redirects.txt

diff --git a/src/cmd/go/internal/web/http.go b/src/cmd/go/internal/web/http.go
index beb80c505dc..e0509808d67 100644
--- a/src/cmd/go/internal/web/http.go
+++ b/src/cmd/go/internal/web/http.go
@@ -13,6 +13,7 @@ package web
 
 import (
 	"crypto/tls"
+	"errors"
 	"fmt"
 	"mime"
 	"net/http"
@@ -47,6 +48,13 @@ var securityPreservingHTTPClient = &http.Client{
 			lastHop := via[len(via)-1].URL
 			return fmt.Errorf("redirected from secure URL %s to insecure URL %s", lastHop, req.URL)
 		}
+
+		// Go's http.DefaultClient allows 10 redirects before returning an error.
+		// The securityPreservingHTTPClient also uses this default policy to avoid
+		// Go command hangs.
+		if len(via) >= 10 {
+			return errors.New("stopped after 10 redirects")
+		}
 		return nil
 	},
 }
diff --git a/src/cmd/go/proxy_test.go b/src/cmd/go/proxy_test.go
index 8214488a59a..2a4d2935b38 100644
--- a/src/cmd/go/proxy_test.go
+++ b/src/cmd/go/proxy_test.go
@@ -174,6 +174,25 @@ func proxyHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Request for $GOPROXY/redirect/<count>/... goes to redirects.
+	if strings.HasPrefix(path, "redirect/") {
+		path = path[len("redirect/"):]
+		if j := strings.Index(path, "/"); j >= 0 {
+			count, err := strconv.Atoi(path[:j])
+			if err != nil {
+				return
+			}
+
+			// The last redirect.
+			if count <= 1 {
+				http.Redirect(w, r, fmt.Sprintf("/mod/%s", path[j+1:]), 302)
+				return
+			}
+			http.Redirect(w, r, fmt.Sprintf("/mod/redirect/%d/%s", count-1, path[j+1:]), 302)
+			return
+		}
+	}
+
 	// Request for $GOPROXY/sumdb/<name>/supported
 	// is checking whether it's OK to access sumdb via the proxy.
 	if path == "sumdb/"+testSumDBName+"/supported" {
diff --git a/src/cmd/go/testdata/script/mod_get_too_many_redirects.txt b/src/cmd/go/testdata/script/mod_get_too_many_redirects.txt
new file mode 100644
index 00000000000..9cbe0d279de
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_too_many_redirects.txt
@@ -0,0 +1,10 @@
+env GO111MODULE=on
+env GOPROXYBASE=$GOPROXY
+env GOPROXY=$GOPROXYBASE/redirect/11
+env GOSUMDB=off
+
+! go get -d rsc.io/quote@v1.2.0
+stderr 'stopped after 10 redirects'
+
+env GOPROXY=$GOPROXYBASE/redirect/9
+go get -d rsc.io/quote@v1.2.0