Merge pull request #60 from ooni/merged-main

chore: upgrade to go1.21.8

.github/workflows/go.yml

@@ -16,7 +16,7 @@ jobs:
     - name: Set up Go
       uses: actions/setup-go@v4
       with:
-        go-version: "1.20.14"
+        go-version: "1.21.8"
 
     - name: Build
       run: go build -v ./...

README.md

@@ -225,20 +225,21 @@ minor changes (e.g., updating docs) directly on the `main` branch.
 - [ ] update [UPSTREAM](UPSTREAM), commit the change, and then
 run the `./tools/merge.bash` script to merge from upstream;
 
-- [ ] make sure you synch [./internal/safefilepath](./internal/safefilepath) with the
-`./src/internal/safefilepath` of the Go release you're merging from;
-
 - [ ] solve the very-likely merge conflicts and ensure [the original spirit of the
 patches](#patches) still hold;
 
+- [ ] make sure you synch [./internal/safefilepath](./internal/safefilepath) with the
+`./src/internal/safefilepath` of the Go release you're merging from;
+
 - [ ] make sure the codebase does not assume `*tls.Conn` *anywhere* (`git grep -n '\*tls\.Conn'`)
 and otherwise replace `*tls.Conn` with `TLSConn`;
 
 - [ ] make sure the codebase does not call `tls.Client` *anywhere* except for `tlsconn.go`
 (`git grep -n 'tls\.Client'`) and otherwise replace `tls.Client` with `TLSClientFactory`;
 
-- [ ] diff with upstream (`diff --color=never -ru .../golang/go/src/net/http .`) and
-make sure what you see makes sense in terms of the original patches;
+- [ ] diff with upstream (`./tools/compare.bash`) and make sure what you see
+makes sense in terms of the original patches, save the diff, and include it into
+the PR to document the actual changes between us and upstream.
 
 - [ ] ensure `go build -v ./...` still works;
 

UPSTREAM

@@ -1 +1 @@
-go1.20.14
+go1.21.8

cgi/child_test.go

@@ -12,8 +12,8 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/ooni/oohttp"
-	"github.com/ooni/oohttp/httptest"
+	http "github.com/ooni/oohttp"
+	httptest "github.com/ooni/oohttp/httptest"
 )
 
 func TestRequest(t *testing.T) {

cgi/host.go

@@ -39,7 +39,7 @@ var osDefaultInheritEnv = func() []string {
 	switch runtime.GOOS {
 	case "darwin", "ios":
 		return []string{"DYLD_LIBRARY_PATH"}
-	case "linux", "freebsd", "netbsd", "openbsd":
+	case "android", "linux", "freebsd", "netbsd", "openbsd":
 		return []string{"LD_LIBRARY_PATH"}
 	case "hpux":
 		return []string{"LD_LIBRARY_PATH", "SHLIB_PATH"}

client.go

@@ -145,14 +145,19 @@ type RoundTripper interface {
 
 // refererForURL returns a referer without any authentication info or
 // an empty string if lastReq scheme is https and newReq scheme is http.
-func refererForURL(lastReq, newReq *url.URL) string {
+// If the referer was explicitly set, then it will continue to be used.
+func refererForURL(lastReq, newReq *url.URL, explicitRef string) string {
 	// https://tools.ietf.org/html/rfc7231#section-5.5.2
 	//   "Clients SHOULD NOT include a Referer header field in a
 	//    (non-secure) HTTP request if the referring page was
 	//    transferred with a secure protocol."
 	if lastReq.Scheme == "https" && newReq.Scheme == "http" {
 		return ""
 	}
+	if explicitRef != "" {
+		return explicitRef
+	}
+
 	referer := lastReq.String()
 	if lastReq.User != nil {
 		// This is not very efficient, but is the best we can
@@ -200,6 +205,9 @@ func (c *Client) transport() RoundTripper {
 	return DefaultTransport
 }
 
+// ErrSchemeMismatch is returned when a server returns an HTTP response to an HTTPS client.
+var ErrSchemeMismatch = errors.New("http: server gave HTTP response to HTTPS client")
+
 // send issues an HTTP request.
 // Caller should close resp.Body when done reading from it.
 func send(ireq *Request, rt RoundTripper, deadline time.Time) (resp *Response, didTimeout func() bool, err error) {
@@ -261,7 +269,7 @@ func send(ireq *Request, rt RoundTripper, deadline time.Time) (resp *Response, d
 			// response looks like HTTP and give a more helpful error.
 			// See golang.org/issue/11111.
 			if string(tlsErr.RecordHeader[:]) == "HTTP/" {
-				err = errors.New("http: server gave HTTP response to HTTPS client")
+				err = ErrSchemeMismatch
 			}
 		}
 		return nil, didTimeout, err
@@ -677,7 +685,7 @@ func (c *Client) do(req *Request) (retres *Response, reterr error) {
 
 			// Add the Referer header from the most recent
 			// request URL to the new one, if it's not https->http:
-			if ref := refererForURL(reqs[len(reqs)-1].URL, req.URL); ref != "" {
+			if ref := refererForURL(reqs[len(reqs)-1].URL, req.URL, req.Header.Get("Referer")); ref != "" {
 				req.Header.Set("Referer", ref)
 			}
 			err = c.checkRedirect(req, reqs)
@@ -991,8 +999,8 @@ func shouldCopyHeaderOnRedirect(headerKey string, initial, dest *url.URL) bool {
 		// directly, we don't know their scope, so we assume
 		// it's for *.domain.com.
 
-		ihost := canonicalAddr(initial)
-		dhost := canonicalAddr(dest)
+		ihost := idnaASCIIFromURL(initial)
+		dhost := idnaASCIIFromURL(dest)
 		return isDomainOrSubdomain(dhost, ihost)
 	}
 	// All other headers are copied:
@@ -1007,6 +1015,12 @@ func isDomainOrSubdomain(sub, parent string) bool {
 	if sub == parent {
 		return true
 	}
+	// If sub contains a :, it's probably an IPv6 address (and is definitely not a hostname).
+	// Don't check the suffix in this case, to avoid matching the contents of a IPv6 zone.
+	// For example, "::1%.www.example.com" is not a subdomain of "www.example.com".
+	if strings.ContainsAny(sub, ":%") {
+		return false
+	}
 	// If sub is "foo.example.com" and parent is "example.com",
 	// that means sub must end in "."+parent.
 	// Do it without allocating.

client_test.go

@@ -1208,7 +1208,7 @@ func testClientTimeout(t *testing.T, mode testMode) {
 	}))
 
 	// Try to trigger a timeout after reading part of the response body.
-	// The initial timeout is emprically usually long enough on a decently fast
+	// The initial timeout is empirically usually long enough on a decently fast
 	// machine, but if we undershoot we'll retry with exponentially longer
 	// timeouts until the test either passes or times out completely.
 	// This keeps the test reasonably fast in the typical case but allows it to
@@ -1412,24 +1412,32 @@ func (f eofReaderFunc) Read(p []byte) (n int, err error) {
 
 func TestReferer(t *testing.T) {
 	tests := []struct {
-		lastReq, newReq string // from -> to URLs
-		want            string
+		lastReq, newReq, explicitRef string // from -> to URLs, explicitly set Referer value
+		want                         string
 	}{
 		// don't send user:
-		{"http://[email protected]", "http://link.com", "http://test.com"},
-		{"https://[email protected]", "https://link.com", "https://test.com"},
+		{lastReq: "http://[email protected]", newReq: "http://link.com", want: "http://test.com"},
+		{lastReq: "https://[email protected]", newReq: "https://link.com", want: "https://test.com"},
 
 		// don't send a user and password:
-		{"http://gopher:[email protected]", "http://link.com", "http://test.com"},
-		{"https://gopher:[email protected]", "https://link.com", "https://test.com"},
+		{lastReq: "http://gopher:[email protected]", newReq: "http://link.com", want: "http://test.com"},
+		{lastReq: "https://gopher:[email protected]", newReq: "https://link.com", want: "https://test.com"},
 
 		// nothing to do:
-		{"http://test.com", "http://link.com", "http://test.com"},
-		{"https://test.com", "https://link.com", "https://test.com"},
+		{lastReq: "http://test.com", newReq: "http://link.com", want: "http://test.com"},
+		{lastReq: "https://test.com", newReq: "https://link.com", want: "https://test.com"},
 
 		// https to http doesn't send a referer:
-		{"https://test.com", "http://link.com", ""},
-		{"https://gopher:[email protected]", "http://link.com", ""},
+		{lastReq: "https://test.com", newReq: "http://link.com", want: ""},
+		{lastReq: "https://gopher:[email protected]", newReq: "http://link.com", want: ""},
+
+		// https to http should remove an existing referer:
+		{lastReq: "https://test.com", newReq: "http://link.com", explicitRef: "https://foo.com", want: ""},
+		{lastReq: "https://gopher:[email protected]", newReq: "http://link.com", explicitRef: "https://foo.com", want: ""},
+
+		// don't override an existing referer:
+		{lastReq: "https://test.com", newReq: "https://link.com", explicitRef: "https://foo.com", want: "https://foo.com"},
+		{lastReq: "https://gopher:[email protected]", newReq: "https://link.com", explicitRef: "https://foo.com", want: "https://foo.com"},
 	}
 	for _, tt := range tests {
 		l, err := url.Parse(tt.lastReq)
@@ -1440,7 +1448,7 @@ func TestReferer(t *testing.T) {
 		if err != nil {
 			t.Fatal(err)
 		}
-		r := ExportRefererForURL(l, n)
+		r := ExportRefererForURL(l, n, tt.explicitRef)
 		if r != tt.want {
 			t.Errorf("refererForURL(%q, %q) = %q; want %q", tt.lastReq, tt.newReq, r, tt.want)
 		}
@@ -1471,6 +1479,9 @@ func TestClientRedirectResponseWithoutRequest(t *testing.T) {
 }
 
 // Issue 4800: copy (some) headers when Client follows a redirect.
+// Issue 35104: Since both URLs have the same host (localhost)
+// but different ports, sensitive headers like Cookie and Authorization
+// are preserved.
 func TestClientCopyHeadersOnRedirect(t *testing.T) { run(t, testClientCopyHeadersOnRedirect) }
 func testClientCopyHeadersOnRedirect(t *testing.T, mode testMode) {
 	const (
@@ -1484,6 +1495,8 @@ func testClientCopyHeadersOnRedirect(t *testing.T, mode testMode) {
 			"X-Foo":           []string{xfoo},
 			"Referer":         []string{ts2URL},
 			"Accept-Encoding": []string{"gzip"},
+			"Cookie":          []string{"foo=bar"},
+			"Authorization":   []string{"secretpassword"},
 		}
 		if !reflect.DeepEqual(r.Header, want) {
 			t.Errorf("Request.Header = %#v; want %#v", r.Header, want)
@@ -1502,9 +1515,11 @@ func testClientCopyHeadersOnRedirect(t *testing.T, mode testMode) {
 	c := ts1.Client()
 	c.CheckRedirect = func(r *Request, via []*Request) error {
 		want := Header{
-			"User-Agent": []string{ua},
-			"X-Foo":      []string{xfoo},
-			"Referer":    []string{ts2URL},
+			"User-Agent":    []string{ua},
+			"X-Foo":         []string{xfoo},
+			"Referer":       []string{ts2URL},
+			"Cookie":        []string{"foo=bar"},
+			"Authorization": []string{"secretpassword"},
 		}
 		if !reflect.DeepEqual(r.Header, want) {
 			t.Errorf("CheckRedirect Request.Header = %#v; want %#v", r.Header, want)
@@ -1708,18 +1723,31 @@ func TestShouldCopyHeaderOnRedirect(t *testing.T) {
 		{"cookie", "http://foo.com/", "http://bar.com/", false},
 		{"cookie2", "http://foo.com/", "http://bar.com/", false},
 		{"authorization", "http://foo.com/", "http://bar.com/", false},
+		{"authorization", "http://foo.com/", "https://foo.com/", true},
+		{"authorization", "http://foo.com:1234/", "http://foo.com:4321/", true},
 		{"www-authenticate", "http://foo.com/", "http://bar.com/", false},
+		{"authorization", "http://foo.com/", "http://[::1%25.foo.com]/", 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/", "https://foo.com/", true},
 		{"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:1234/", "http://foo.com/", true},
+
+		{"authorization", "http://foo.com/", "http://foo.com/", true},
+		{"authorization", "http://foo.com/", "http://sub.foo.com/", true},
+		{"authorization", "http://foo.com/", "http://notfoo.com/", false},
+		{"authorization", "http://foo.com/", "https://foo.com/", true},
+		{"authorization", "http://foo.com:80/", "http://foo.com/", true},
+		{"authorization", "http://foo.com:80/", "http://sub.foo.com/", true},
+		{"authorization", "http://foo.com:443/", "https://foo.com/", true},
+		{"authorization", "http://foo.com:443/", "https://sub.foo.com/", true},
+		{"authorization", "http://foo.com:1234/", "http://foo.com/", true},
 	}
 	for i, tt := range tests {
 		u0, err := url.Parse(tt.initialURL)

clientserver_test.go

@@ -1240,9 +1240,9 @@ func testTransportRejectsInvalidHeaders(t *testing.T, mode testMode) {
 func TestInterruptWithPanic(t *testing.T) {
 	run(t, func(t *testing.T, mode testMode) {
 		t.Run("boom", func(t *testing.T) { testInterruptWithPanic(t, mode, "boom") })
-		t.Run("nil", func(t *testing.T) { testInterruptWithPanic(t, mode, nil) })
+		t.Run("nil", func(t *testing.T) { t.Setenv("GODEBUG", "panicnil=1"); testInterruptWithPanic(t, mode, nil) })
 		t.Run("ErrAbortHandler", func(t *testing.T) { testInterruptWithPanic(t, mode, ErrAbortHandler) })
-	})
+	}, testNotParallel)
 }
 func testInterruptWithPanic(t *testing.T, mode testMode, panicValue any) {
 	const msg = "hello"
@@ -1284,24 +1284,28 @@ func testInterruptWithPanic(t *testing.T, mode testMode, panicValue any) {
 	}
 	wantStackLogged := panicValue != nil && panicValue != ErrAbortHandler
 
-	if err := waitErrCondition(5*time.Second, 10*time.Millisecond, func() error {
+	waitCondition(t, 10*time.Millisecond, func(d time.Duration) bool {
 		gotLog := logOutput()
 		if !wantStackLogged {
 			if gotLog == "" {
-				return nil
+				return true
 			}
-			return fmt.Errorf("want no log output; got: %s", gotLog)
+			t.Fatalf("want no log output; got: %s", gotLog)
 		}
 		if gotLog == "" {
-			return fmt.Errorf("wanted a stack trace logged; got nothing")
+			if d > 0 {
+				t.Logf("wanted a stack trace logged; got nothing after %v", d)
+			}
+			return false
 		}
 		if !strings.Contains(gotLog, "created by ") && strings.Count(gotLog, "\n") < 6 {
-			return fmt.Errorf("output doesn't look like a panic stack trace. Got: %s", gotLog)
+			if d > 0 {
+				t.Logf("output doesn't look like a panic stack trace after %v. Got: %s", d, gotLog)
+			}
+			return false
 		}
-		return nil
-	}); err != nil {
-		t.Fatal(err)
-	}
+		return true
+	})
 }
 
 type lockedBytesBuffer struct {

cookiejar/jar.go

@@ -363,10 +363,17 @@ func jarKey(host string, psl PublicSuffixList) string {
 
 // isIP reports whether host is an IP address.
 func isIP(host string) bool {
+	if strings.ContainsAny(host, ":%") {
+		// Probable IPv6 address.
+		// Hostnames can't contain : or %, so this is definitely not a valid host.
+		// Treating it as an IP is the more conservative option, and avoids the risk
+		// of interpeting ::1%.www.example.com as a subtomain of www.example.com.
+		return true
+	}
 	return net.ParseIP(host) != nil
 }
 
-// defaultPath returns the directory part of an URL's path according to
+// defaultPath returns the directory part of a URL's path according to
 // RFC 6265 section 5.1.4.
 func defaultPath(path string) string {
 	if len(path) == 0 || path[0] != '/' {
@@ -380,7 +387,7 @@ func defaultPath(path string) string {
 	return path[:i] // Path is either of form "/abc/xyz" or "/abc/xyz/".
 }
 
-// newEntry creates an entry from a http.Cookie c. now is the current time and
+// newEntry creates an entry from an http.Cookie c. now is the current time and
 // is compared to c.Expires to determine deletion of c. defPath and host are the
 // default-path and the canonical host name of the URL c was received from.
 //
@@ -466,14 +473,14 @@ func (j *Jar) domainAndType(host, domain string) (string, bool, error) {
 		// dot in the domain-attribute before processing the cookie.
 		//
 		// Most browsers don't do that for IP addresses, only curl
-		// version 7.54) and IE (version 11) do not reject a
+		// (version 7.54) and IE (version 11) do not reject a
 		//     Set-Cookie: a=1; domain=.127.0.0.1
 		// This leading dot is optional and serves only as hint for
 		// humans to indicate that a cookie with "domain=.bbc.co.uk"
 		// would be sent to every subdomain of bbc.co.uk.
 		// It just doesn't make sense on IP addresses.
 		// The other processing and validation steps in RFC 6265 just
-		// collaps to:
+		// collapse to:
 		if host != domain {
 			return "", false, errIllegalDomain
 		}

cookiejar/jar_test.go

@@ -253,6 +253,7 @@ var isIPTests = map[string]bool{
 	"127.0.0.1":            true,
 	"1.2.3.4":              true,
 	"2001:4860:0:2001::68": true,
+	"::1%zone":             true,
 	"example.com":          false,
 	"1.1.1.300":            false,
 	"www.foo.bar.net":      false,
@@ -350,7 +351,7 @@ func expiresIn(delta int) string {
 	return "expires=" + t.Format(time.RFC1123)
 }
 
-// mustParseURL parses s to an URL and panics on error.
+// mustParseURL parses s to a URL and panics on error.
 func mustParseURL(s string) *url.URL {
 	u, err := url.Parse(s)
 	if err != nil || u.Scheme == "" || u.Host == "" {
@@ -630,6 +631,15 @@ var basicsTests = [...]jarTest{
 			{"http://www.host.test:1234/", "a=1"},
 		},
 	},
+	{
+		"IPv6 zone is not treated as a host.",
+		"https://example.com/",
+		[]string{"a=1"},
+		"a=1",
+		[]query{
+			{"https://[::1%25.example.com]:80/", ""},
+		},
+	},
 }
 
 func TestBasics(t *testing.T) {
@@ -671,7 +681,7 @@ var updateAndDeleteTests = [...]jarTest{
 		},
 	},
 	{
-		"Clear Secure flag from a http.",
+		"Clear Secure flag from an http.",
 		"http://www.host.test/",
 		[]string{
 			"b=xx",