From debf0ee2ed1fa9626fd45baa20ecb92595312f8a Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Tue, 24 Feb 2026 02:13:19 +0000 Subject: [PATCH] fix: map Secure field in cookie conversions and add per-cookie error handling The Secure field was dropped in both Playwright<->internal cookie conversion functions, causing cookies with __Secure-/__Host- prefixes to be rejected by Chromium. Additionally, batch AddCookies meant one invalid cookie would fail browser creation entirely. Changes: - Map Secure field in cookieToPlaywrightOptionalCookie and playwrightCookieToCookie - Add cookies one-by-one with slog.Warn on failure instead of failing the entire batch - Add unit tests for both conversion functions Closes #75 Co-Authored-By: Claude Opus 4.6 --- browser_init.go | 11 ++-- playwright.go | 2 + playwright_test.go | 138 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 145 insertions(+), 6 deletions(-) create mode 100644 playwright_test.go diff --git a/browser_init.go b/browser_init.go index 5ceeec7..f009400 100644 --- a/browser_init.go +++ b/browser_init.go @@ -152,12 +152,11 @@ func initBrowser(opt BrowserOptions) (*browserInitResult, error) { if err != nil { return nil, fmt.Errorf("error getting cookies from cookie jar: %w", err) } - pwCookies := make([]playwright.OptionalCookie, len(cookies)) - for i, c := range cookies { - pwCookies[i] = cookieToPlaywrightOptionalCookie(c) - } - if err := bctx.AddCookies(pwCookies); err != nil { - return nil, fmt.Errorf("error adding cookies to browser: %w", err) + for _, c := range cookies { + oc := cookieToPlaywrightOptionalCookie(c) + if err := bctx.AddCookies([]playwright.OptionalCookie{oc}); err != nil { + slog.Warn("skipping invalid cookie", "name", c.Name, "host", c.Host, "error", err) + } } } diff --git a/playwright.go b/playwright.go index bacba5b..063fcbd 100644 --- a/playwright.go +++ b/playwright.go @@ -128,6 +128,7 @@ func cookieToPlaywrightOptionalCookie(cookie Cookie) playwright.OptionalCookie { Domain: playwright.String(cookie.Host), Path: playwright.String(cookie.Path), Expires: playwright.Float(float64(cookie.Expires.Unix())), + Secure: playwright.Bool(cookie.Secure), HttpOnly: playwright.Bool(cookie.HttpOnly), } if cookie.SameSite != "" { @@ -143,6 +144,7 @@ func playwrightCookieToCookie(cookie playwright.Cookie) Cookie { Host: cookie.Domain, Path: cookie.Path, Expires: time.Unix(int64(cookie.Expires), 0), + Secure: cookie.Secure, HttpOnly: cookie.HttpOnly, SameSite: playwrightSameSiteToSameSite(cookie.SameSite), } diff --git a/playwright_test.go b/playwright_test.go new file mode 100644 index 0000000..ad99401 --- /dev/null +++ b/playwright_test.go @@ -0,0 +1,138 @@ +package extractor + +import ( + "testing" + "time" + + "github.com/playwright-community/playwright-go" +) + +func TestPlaywrightCookieToCookie_AllFields(t *testing.T) { + pwCookie := playwright.Cookie{ + Name: "session", + Value: "abc123", + Domain: ".example.com", + Path: "/app", + Expires: 1700000000, + Secure: true, + HttpOnly: true, + SameSite: playwright.SameSiteAttributeStrict, + } + + c := playwrightCookieToCookie(pwCookie) + + if c.Name != "session" { + t.Errorf("Name = %q, want %q", c.Name, "session") + } + if c.Value != "abc123" { + t.Errorf("Value = %q, want %q", c.Value, "abc123") + } + if c.Host != ".example.com" { + t.Errorf("Host = %q, want %q", c.Host, ".example.com") + } + if c.Path != "/app" { + t.Errorf("Path = %q, want %q", c.Path, "/app") + } + if c.Expires != time.Unix(1700000000, 0) { + t.Errorf("Expires = %v, want %v", c.Expires, time.Unix(1700000000, 0)) + } + if !c.Secure { + t.Error("Secure = false, want true") + } + if !c.HttpOnly { + t.Error("HttpOnly = false, want true") + } + if c.SameSite != SameSiteStrict { + t.Errorf("SameSite = %q, want %q", c.SameSite, SameSiteStrict) + } +} + +func TestPlaywrightCookieToCookie_SecureFalse(t *testing.T) { + pwCookie := playwright.Cookie{ + Name: "tracking", + Value: "xyz", + Domain: "example.com", + Path: "/", + Secure: false, + } + + c := playwrightCookieToCookie(pwCookie) + + if c.Secure { + t.Error("Secure = true, want false") + } +} + +func TestCookieToPlaywrightOptionalCookie_AllFields(t *testing.T) { + c := Cookie{ + Name: "__Secure-ID", + Value: "token123", + Host: ".example.com", + Path: "/secure", + Expires: time.Unix(1700000000, 0), + Secure: true, + HttpOnly: true, + SameSite: SameSiteLax, + } + + oc := cookieToPlaywrightOptionalCookie(c) + + if oc.Name != "__Secure-ID" { + t.Errorf("Name = %q, want %q", oc.Name, "__Secure-ID") + } + if oc.Value != "token123" { + t.Errorf("Value = %q, want %q", oc.Value, "token123") + } + if oc.Domain == nil || *oc.Domain != ".example.com" { + t.Errorf("Domain = %v, want %q", oc.Domain, ".example.com") + } + if oc.Path == nil || *oc.Path != "/secure" { + t.Errorf("Path = %v, want %q", oc.Path, "/secure") + } + if oc.Expires == nil || *oc.Expires != 1700000000 { + t.Errorf("Expires = %v, want %v", oc.Expires, 1700000000) + } + if oc.Secure == nil || !*oc.Secure { + t.Error("Secure = nil or false, want *true") + } + if oc.HttpOnly == nil || !*oc.HttpOnly { + t.Error("HttpOnly = nil or false, want *true") + } + if oc.SameSite == nil || *oc.SameSite != *playwright.SameSiteAttributeLax { + t.Errorf("SameSite = %v, want Lax", oc.SameSite) + } +} + +func TestCookieToPlaywrightOptionalCookie_SecureFalse(t *testing.T) { + c := Cookie{ + Name: "tracker", + Value: "v", + Host: "example.com", + Path: "/", + Secure: false, + } + + oc := cookieToPlaywrightOptionalCookie(c) + + if oc.Secure == nil { + t.Fatal("Secure = nil, want *false") + } + if *oc.Secure { + t.Error("Secure = *true, want *false") + } +} + +func TestCookieToPlaywrightOptionalCookie_NoSameSite(t *testing.T) { + c := Cookie{ + Name: "basic", + Value: "val", + Host: "example.com", + Path: "/", + } + + oc := cookieToPlaywrightOptionalCookie(c) + + if oc.SameSite != nil { + t.Errorf("SameSite = %v, want nil", oc.SameSite) + } +} -- 2.49.1