Use new http client and cookie jar across executions

This ensures that cookies from one app aren't leaked to other apps. Instead of InitHTTP setting a single global instance of http.Client, it now sets a factory method. Each time the http module is loaded, that factory is used to create a new http client. Those clients all share the same cache, but not cookie jars

Author: dinosaursrarr

runtime/httpcache.go

@@ -15,7 +15,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/pkg/errors"
 	"golang.org/x/net/publicsuffix"
 	"tidbyt.dev/pixlet/runtime/modules/starlarkhttp"
 )
@@ -57,20 +56,22 @@ func InitHTTP(cache Cache) {
 		transport: http.DefaultTransport,
 	}
 
-	// Providing a cookie jar allows sessions and redirects to work properly. With a
-	// jar present, any cookies set in a response will automatically be added to
-	// subsequent requests. This means that we can follow redirects after logging into
-	// a session. Without a jar, any cookies will be dropped from redirects unless explicitly
-	// set in the original outgoing request.
-	// https://cs.opensource.google/go/go/+/master:src/net/http/client.go;drc=4c394b5638cc2694b1eff6418bc3e7db8132de0e;l=88
-	jar, _ := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List}) // never returns non-nil err
-
-	httpClient := &http.Client{
-		Jar:       jar,
-		Transport: cc,
-		Timeout:   HTTPTimeout * 2,
+	starlarkhttp.StarlarkHTTPClient = func() *http.Client {
+		// Providing a cookie jar allows sessions and redirects to work properly. With a
+		// jar present, any cookies set in a response will automatically be added to
+		// subsequent requests. This means that we can follow redirects after logging into
+		// a session. Without a jar, any cookies will be dropped from redirects unless explicitly
+		// set in the original outgoing request.
+		// https://cs.opensource.google/go/go/+/master:src/net/http/client.go;drc=4c394b5638cc2694b1eff6418bc3e7db8132de0e;l=88
+		jar, _ := cookiejar.New(&cookiejar.Options{PublicSuffixList: publicsuffix.List}) // never returns non-nil err
+
+		httpClient := &http.Client{
+			Jar:       jar,
+			Transport: cc,
+			Timeout:   HTTPTimeout * 2,
+		}
+		return httpClient
 	}
-	starlarkhttp.StarlarkHTTPClient = httpClient
 }
 
 // RoundTrip is an approximation of what our internal HTTP proxy does. It should

runtime/httpcache_test.go

@@ -190,6 +190,9 @@ func TestSetCookieOnRedirect(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// Requests to "/login" set a cookie and redirect to /destination
 		if strings.HasSuffix(r.URL.Path, "/login") {
+			if len(r.Cookies()) != 0 {
+				t.Errorf("Cookie was already set on initial call")
+			}
 			w.Header().Set("Set-Cookie", "doodad=foobar; path=/; HttpOnly")
 			w.Header().Set("Location", "/destination")
 			w.WriteHeader(302)
@@ -199,7 +202,7 @@ func TestSetCookieOnRedirect(t *testing.T) {
 		if strings.HasSuffix(r.URL.Path, "/destination") {
 			c, err := r.Cookie("doodad")
 			if err != nil {
-				t.Errorf("Expected cookie `doodad` not present")  // Occurs if client has no cookie jar
+				t.Errorf("Expected cookie `doodad` not present") // Occurs if client has no cookie jar
 			}
 			if c.Value != "foobar" {
 				t.Errorf("Cookie `doodad` value mismatch. Expected foobar, got %s", c.Value)
@@ -225,4 +228,13 @@ func TestSetCookieOnRedirect(t *testing.T) {
 
 	_, err = app.Run(map[string]string{})
 	assert.NoError(t, err)
+
+	// Run it again and check that we're not using the same cookie jar
+	// across executions. If we were, the first request would error out.
+	app2 := &Applet{}
+	err = app2.Load("httpredirect", "httpredirect.star", b, nil)
+	assert.NoError(t, err)
+
+	_, err = app2.Run(map[string]string{})
+	assert.NoError(t, err)
 }

runtime/modules/starlarkhttp/starlarkhttp.go

@@ -51,9 +51,11 @@ func AsString(x starlark.Value) (string, error) {
 const ModuleName = "http.star"
 
 var (
-	// StarlarkHTTPClient is the http client used to create the http module. override with
-	// a custom client before calling LoadModule
-	StarlarkHTTPClient = http.DefaultClient
+	// StarlarkHTTPClient is a factory method for creating the http client used to create the http module.
+	// override with a custom function before calling LoadModule
+	StarlarkHTTPClient = func() *http.Client {
+		return http.DefaultClient
+	}
 	// StarlarkHTTPGuard is a global RequestGuard used in LoadModule. override with a custom
 	// implementation before calling LoadModule
 	StarlarkHTTPGuard RequestGuard
@@ -69,7 +71,7 @@ const (
 
 // LoadModule creates an http Module
 func LoadModule() (starlark.StringDict, error) {
-	var m = &Module{cli: StarlarkHTTPClient}
+	var m = &Module{cli: StarlarkHTTPClient()}
 	if StarlarkHTTPGuard != nil {
 		m.rg = StarlarkHTTPGuard
 	}