diff --git a/runtime/httpcache.go b/runtime/httpcache.go index 1eb1d7886d..3e1fbf2a10 100644 --- a/runtime/httpcache.go +++ b/runtime/httpcache.go @@ -9,12 +9,14 @@ import ( "fmt" "math/rand" "net/http" + "net/http/cookiejar" "net/http/httputil" "strconv" "strings" "time" "github.com/pkg/errors" + "golang.org/x/net/publicsuffix" "tidbyt.dev/pixlet/runtime/modules/starlarkhttp" ) @@ -55,7 +57,16 @@ 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, } diff --git a/runtime/httpcache_test.go b/runtime/httpcache_test.go index 1f1bb76abb..200d4c9b8f 100644 --- a/runtime/httpcache_test.go +++ b/runtime/httpcache_test.go @@ -4,11 +4,14 @@ import ( "fmt" "math/rand" "net/http" + "net/http/httptest" "os" + "strings" "testing" "time" "github.com/stretchr/testify/assert" + "go.starlark.net/starlark" ) func TestInitHTTP(t *testing.T) { @@ -182,3 +185,44 @@ func TestDetermineTTLNoHeaders(t *testing.T) { ttl := DetermineTTL(req, res) assert.Equal(t, MinRequestTTL, ttl) } + +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") { + w.Header().Set("Set-Cookie", "doodad=foobar; path=/; HttpOnly") + w.Header().Set("Location", "/destination") + w.WriteHeader(302) + return + } + // Requests to /destination must have cookie set + 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 + } + if c.Value != "foobar" { + t.Errorf("Cookie `doodad` value mismatch. Expected foobar, got %s", c.Value) + } + if _, err := w.Write([]byte(`{"hello":"world"}`)); err != nil { + t.Fatal(err) + } + return + } + t.Errorf("Unexpected path requested: %s", r.URL.Path) + })) + + starlark.Universe["test_server_url"] = starlark.String(ts.URL) + c := NewInMemoryCache() + InitHTTP(c) + + b, err := os.ReadFile("testdata/httpredirect.star") + assert.NoError(t, err) + + app := &Applet{} + err = app.Load("httpredirect", "httpredirect.star", b, nil) + assert.NoError(t, err) + + _, err = app.Run(map[string]string{}) + assert.NoError(t, err) +} diff --git a/runtime/modules/starlarkhttp/starlarkhttp.go b/runtime/modules/starlarkhttp/starlarkhttp.go index 1548579225..5fb8764e21 100644 --- a/runtime/modules/starlarkhttp/starlarkhttp.go +++ b/runtime/modules/starlarkhttp/starlarkhttp.go @@ -32,7 +32,6 @@ import ( "io" "mime/multipart" "net/http" - "net/http/cookiejar" "net/url" "strconv" "strings" @@ -40,7 +39,6 @@ import ( util "github.com/qri-io/starlib/util" "go.starlark.net/starlark" "go.starlark.net/starlarkstruct" - "golang.org/x/net/publicsuffix" ) // AsString unquotes a starlark string value @@ -53,18 +51,9 @@ func AsString(x starlark.Value) (string, error) { const ModuleName = "http.star" var ( - // 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}) // StarlarkHTTPClient is the http client used to create the http module. override with // a custom client before calling LoadModule - StarlarkHTTPClient = &http.Client{ - Jar: jar, - } + StarlarkHTTPClient = http.DefaultClient // StarlarkHTTPGuard is a global RequestGuard used in LoadModule. override with a custom // implementation before calling LoadModule StarlarkHTTPGuard RequestGuard diff --git a/runtime/modules/starlarkhttp/starlarkhttp_test.go b/runtime/modules/starlarkhttp/starlarkhttp_test.go index a867e7aad6..83e38e82cf 100644 --- a/runtime/modules/starlarkhttp/starlarkhttp_test.go +++ b/runtime/modules/starlarkhttp/starlarkhttp_test.go @@ -140,41 +140,3 @@ func TestSetBody(t *testing.T) { } } } - -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") { - w.Header().Set("Set-Cookie", "doodad=foobar; path=/; HttpOnly") - w.Header().Set("Location", "/destination") - w.WriteHeader(302) - return - } - // Requests to /destination must have cookie set - if strings.HasSuffix(r.URL.Path, "/destination") { - c, err := r.Cookie("doodad") - if err != nil { - t.Errorf("Expected cookie `doodad` not present") - } - if c.Value != "foobar" { - t.Errorf("Cookie `doodad` value mismatch. Expected foobar, got %s", c.Value) - } - if _, err := w.Write([]byte(`{"hello":"world"}`)); err != nil { - t.Fatal(err) - } - return - } - t.Errorf("Unexpected path requested: %s", r.URL.Path) - })) - starlark.Universe["test_server_url"] = starlark.String(ts.URL) - - thread := &starlark.Thread{Name: "unittests/setcookieonredirect", Load: testdata.NewLoader(starlarkhttp.LoadModule, starlarkhttp.ModuleName)} - starlarktest.SetReporter(thread, t) - - // Execute test file - _, err := starlark.ExecFile(thread, "testdata/test_redirect.star", nil, nil) - if err != nil { - t.Error(err) - } - -} diff --git a/runtime/modules/starlarkhttp/testdata/test_redirect.star b/runtime/modules/starlarkhttp/testdata/test_redirect.star deleted file mode 100644 index c99cd78c55..0000000000 --- a/runtime/modules/starlarkhttp/testdata/test_redirect.star +++ /dev/null @@ -1,7 +0,0 @@ -load("http.star", "http") -load("assert.star", "assert") - -res_1 = http.post(test_server_url + "/login") -assert.eq(res_1.status_code, 200) -assert.eq(res_1.body(), '{"hello":"world"}') -assert.eq(res_1.json(), {"hello": "world"}) diff --git a/runtime/testdata/httpredirect.star b/runtime/testdata/httpredirect.star new file mode 100644 index 0000000000..521cfd9a33 --- /dev/null +++ b/runtime/testdata/httpredirect.star @@ -0,0 +1,10 @@ +load("http.star", "http") +load("assert.star", "assert") +load("render.star", "render") + +def main(): + res_1 = http.post(test_server_url + "/login") + assert.eq(res_1.status_code, 200) + assert.eq(res_1.body(), '{"hello":"world"}') + assert.eq(res_1.json(), {"hello": "world"}) + return render.Root(child = render.Text("pass"))