diff --git a/runtime/httpcache.go b/runtime/httpcache.go index 3e1fbf2a10..662741eaae 100644 --- a/runtime/httpcache.go +++ b/runtime/httpcache.go @@ -57,20 +57,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 diff --git a/runtime/httpcache_test.go b/runtime/httpcache_test.go index 200d4c9b8f..4ef5f7ee3a 100644 --- a/runtime/httpcache_test.go +++ b/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) } diff --git a/runtime/modules/starlarkhttp/starlarkhttp.go b/runtime/modules/starlarkhttp/starlarkhttp.go index 5fb8764e21..03c3132d03 100644 --- a/runtime/modules/starlarkhttp/starlarkhttp.go +++ b/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 }