cmd/go/internal/web: include snippets of plain-text server responses in error detail

For the server response to be displayed, the response must be served
as type text/plain with charset us-ascii or utf-8, and must consist of
only graphic characters and whitespace.

We truncate the server response at the first blank line or after 8
lines or a fixed number of characters, and tab-indent (if multiple
lines) to ensure that the response is offset from ordinary go command
output.

Fixes #30748

Change-Id: I0bc1d734737e456e3251aee2252463b6355e8c97
Reviewed-on: https://go-review.googlesource.com/c/go/+/189783
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>

src/cmd/go/internal/web/api.go

@@ -10,12 +10,15 @@
 package web
 
 import (
+	"bytes"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"net/url"
 	"os"
 	"strings"
+	"unicode"
+	"unicode/utf8"
 )
 
 // SecurityMode specifies whether a function should make network
@@ -34,9 +37,32 @@ type HTTPError struct {
 	URL        string // redacted
 	Status     string
 	StatusCode int
+	Err        error  // underlying error, if known
+	Detail     string // limited to maxErrorDetailLines and maxErrorDetailBytes
 }
 
+const (
+	maxErrorDetailLines = 8
+	maxErrorDetailBytes = maxErrorDetailLines * 81
+)
+
 func (e *HTTPError) Error() string {
+	if e.Detail != "" {
+		detailSep := " "
+		if strings.ContainsRune(e.Detail, '\n') {
+			detailSep = "\n\t"
+		}
+		return fmt.Sprintf("reading %s: %v\n\tserver response:%s%s", e.URL, e.Status, detailSep, e.Detail)
+	}
+
+	if err := e.Err; err != nil {
+		if pErr, ok := e.Err.(*os.PathError); ok && strings.HasSuffix(e.URL, pErr.Path) {
+			// Remove the redundant copy of the path.
+			err = pErr.Err
+		}
+		return fmt.Sprintf("reading %s: %v", e.URL, err)
+	}
+
 	return fmt.Sprintf("reading %s: %v", e.URL, e.Status)
 }
 
@@ -44,6 +70,10 @@ func (e *HTTPError) Is(target error) bool {
 	return target == os.ErrNotExist && (e.StatusCode == 404 || e.StatusCode == 410)
 }
 
+func (e *HTTPError) Unwrap() error {
+	return e.Err
+}
+
 // GetBytes returns the body of the requested resource, or an error if the
 // response status was not http.StatusOK.
 //
@@ -69,16 +99,69 @@ type Response struct {
 	Status     string
 	StatusCode int
 	Header     map[string][]string
-	Body       io.ReadCloser
+	Body       io.ReadCloser // Either the original body or &errorDetail.
+
+	fileErr     error
+	errorDetail errorDetailBuffer
 }
 
 // Err returns an *HTTPError corresponding to the response r.
-// It returns nil if the response r has StatusCode 200 or 0 (unset).
+// If the response r has StatusCode 200 or 0 (unset), Err returns nil.
+// Otherwise, Err may read from r.Body in order to extract relevant error detail.
 func (r *Response) Err() error {
 	if r.StatusCode == 200 || r.StatusCode == 0 {
 		return nil
 	}
-	return &HTTPError{URL: r.URL, Status: r.Status, StatusCode: r.StatusCode}
+
+	return &HTTPError{
+		URL:        r.URL,
+		Status:     r.Status,
+		StatusCode: r.StatusCode,
+		Err:        r.fileErr,
+		Detail:     r.formatErrorDetail(),
+	}
+}
+
+// formatErrorDetail converts r.errorDetail (a prefix of the output of r.Body)
+// into a short, tab-indented summary.
+func (r *Response) formatErrorDetail() string {
+	if r.Body != &r.errorDetail {
+		return "" // Error detail collection not enabled.
+	}
+
+	// Ensure that r.errorDetail has been populated.
+	_, _ = io.Copy(ioutil.Discard, r.Body)
+
+	s := r.errorDetail.buf.String()
+	if !utf8.ValidString(s) {
+		return "" // Don't try to recover non-UTF-8 error messages.
+	}
+	for _, r := range s {
+		if !unicode.IsGraphic(r) && !unicode.IsSpace(r) {
+			return "" // Don't let the server do any funny business with the user's terminal.
+		}
+	}
+
+	var detail strings.Builder
+	for i, line := range strings.Split(s, "\n") {
+		if strings.TrimSpace(line) == "" {
+			break // Stop at the first blank line.
+		}
+		if i > 0 {
+			detail.WriteString("\n\t")
+		}
+		if i >= maxErrorDetailLines {
+			detail.WriteString("[Truncated: too many lines.]")
+			break
+		}
+		if detail.Len()+len(line) > maxErrorDetailBytes {
+			detail.WriteString("[Truncated: too long.]")
+			break
+		}
+		detail.WriteString(line)
+	}
+
+	return detail.String()
 }
 
 // Get returns the body of the HTTP or HTTPS resource specified at the given URL.
@@ -131,3 +214,39 @@ func Join(u *url.URL, path string) *url.URL {
 	j.RawPath = strings.TrimSuffix(u.RawPath, "/") + "/" + strings.TrimPrefix(path, "/")
 	return &j
 }
+
+// An errorDetailBuffer is an io.ReadCloser that copies up to
+// maxErrorDetailLines into a buffer for later inspection.
+type errorDetailBuffer struct {
+	r        io.ReadCloser
+	buf      strings.Builder
+	bufLines int
+}
+
+func (b *errorDetailBuffer) Close() error {
+	return b.r.Close()
+}
+
+func (b *errorDetailBuffer) Read(p []byte) (n int, err error) {
+	n, err = b.r.Read(p)
+
+	// Copy the first maxErrorDetailLines+1 lines into b.buf,
+	// discarding any further lines.
+	//
+	// Note that the read may begin or end in the middle of a UTF-8 character,
+	// so don't try to do anything fancy with characters that encode to larger
+	// than one byte.
+	if b.bufLines <= maxErrorDetailLines {
+		for _, line := range bytes.SplitAfterN(p[:n], []byte("\n"), maxErrorDetailLines-b.bufLines) {
+			b.buf.Write(line)
+			if len(line) > 0 && line[len(line)-1] == '\n' {
+				b.bufLines++
+				if b.bufLines > maxErrorDetailLines {
+					break
+				}
+			}
+		}
+	}
+
+	return n, err
+}

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

@@ -14,7 +14,7 @@ package web
 import (
 	"crypto/tls"
 	"fmt"
-	"io/ioutil"
+	"mime"
 	"net/http"
 	urlpkg "net/url"
 	"os"
@@ -64,7 +64,7 @@ func get(security SecurityMode, url *urlpkg.URL) (*Response, error) {
 			Status:     "404 testing",
 			StatusCode: 404,
 			Header:     make(map[string][]string),
-			Body:       ioutil.NopCloser(strings.NewReader("")),
+			Body:       http.NoBody,
 		}
 		if cfg.BuildX {
 			fmt.Fprintf(os.Stderr, "# get %s: %v (%.3fs)\n", Redacted(url), res.Status, time.Since(start).Seconds())
@@ -167,6 +167,7 @@ func get(security SecurityMode, url *urlpkg.URL) (*Response, error) {
 	if cfg.BuildX {
 		fmt.Fprintf(os.Stderr, "# get %s: %v (%.3fs)\n", Redacted(fetched), res.Status, time.Since(start).Seconds())
 	}
+
 	r := &Response{
 		URL:        Redacted(fetched),
 		Status:     res.Status,
@@ -174,6 +175,20 @@ func get(security SecurityMode, url *urlpkg.URL) (*Response, error) {
 		Header:     map[string][]string(res.Header),
 		Body:       res.Body,
 	}
+
+	if res.StatusCode != http.StatusOK {
+		contentType := res.Header.Get("Content-Type")
+		if mediaType, params, _ := mime.ParseMediaType(contentType); mediaType == "text/plain" {
+			switch charset := strings.ToLower(params["charset"]); charset {
+			case "us-ascii", "utf-8", "":
+				// Body claims to be plain text in UTF-8 or a subset thereof.
+				// Try to extract a useful error message from it.
+				r.errorDetail.r = res.Body
+				r.Body = &r.errorDetail
+			}
+		}
+	}
+
 	return r, nil
 }
 
@@ -190,6 +205,7 @@ func getFile(u *urlpkg.URL) (*Response, error) {
 			Status:     http.StatusText(http.StatusNotFound),
 			StatusCode: http.StatusNotFound,
 			Body:       http.NoBody,
+			fileErr:    err,
 		}, nil
 	}
 
@@ -199,6 +215,7 @@ func getFile(u *urlpkg.URL) (*Response, error) {
 			Status:     http.StatusText(http.StatusForbidden),
 			StatusCode: http.StatusForbidden,
 			Body:       http.NoBody,
+			fileErr:    err,
 		}, nil
 	}
 

src/cmd/go/testdata/script/mod_auth.txt

@@ -8,6 +8,8 @@ env GOSUMDB=off
 # basic auth should fail.
 env NETRC=$WORK/empty
 ! go list all
+stderr '^\tserver response: ACCESS DENIED, buddy$'
+stderr '^\tserver response: File\? What file\?$'
 
 # With credentials from a netrc file, it should succeed.
 env NETRC=$WORK/netrc

src/cmd/go/testdata/script/mod_proxy_errors.txt

+[!net] skip
+
+env GO111MODULE=on
+env GOSUMDB=off
+env GOPROXY=direct
+
+# Server responses should be truncated to some reasonable number of lines.
+# (For now, exactly eight.)
+! go list -m vcs-test.golang.org/auth/ormanylines@latest
+stderr '\tserver response:\n(.|\n)*\tline 8\n\t\[Truncated: too many lines.\]$'
+
+# Server responses should be truncated to some reasonable number of characters.
+! go list -m vcs-test.golang.org/auth/oronelongline@latest
+! stderr 'blah{40}'
+stderr '\tserver response: \[Truncated: too long\.\]$'
+
+# Responses from servers using the 'mod' protocol should be propagated.
+! go list -m vcs-test.golang.org/go/modauth404@latest
+stderr '\tserver response: File\? What file\?'

src/cmd/go/testdata/script/mod_sumdb_file_path.txt

@@ -7,10 +7,13 @@ env GOPATH=$WORK/gopath1
 # With a file-based proxy with an empty checksum directory,
 # downloading a new module should fail, even if a subsequent
 # proxy contains a more complete mirror of the sum database.
+#
+# TODO(bcmills): The error message here is a bit redundant.
+# It comes from the sumweb package, which isn't yet producing structured errors.
 [windows] env GOPROXY=file:///$WORK/sumproxy,https://proxy.golang.org
 [!windows] env GOPROXY=file://$WORK/sumproxy,https://proxy.golang.org
 ! go get -d golang.org/x/text@v0.3.2
-stderr '^verifying golang.org/x/text.*: Not Found'
+stderr '^verifying golang.org/x/text@v0.3.2: golang.org/x/text@v0.3.2: reading file://.*/sumdb/sum.golang.org/lookup/golang.org/x/text@v0.3.2: (no such file or directory|.*cannot find the file specified.*)'
 
 # If the proxy does not claim to support the database,
 # checksum verification should fall through to the next proxy,