fix(network): Include subdomains of localhost when including cookies #35771
Conversation
…ookies to include
This comment has been minimized.
This comment has been minimized.
|
@yury-s I added a failing test but I couldn't figure out why it's failing, any ideas? |
This comment has been minimized.
This comment has been minimized.
| res.setHeader('Set-Cookie', ['a=v; secure', 'b=v']); | ||
| res.end(); | ||
| }); | ||
| await request.get(`${`http://a.b.localhost:${server.PORT}`}/setcookie.html`); |
There was a problem hiding this comment.
| await request.get(`${`http://a.b.localhost:${server.PORT}`}/setcookie.html`); | |
| await request.get(`http://a.b.localhost:${server.PORT}/setcookie.html`); |
|
@microsoft-github-policy-service agree |
|
@yury-s I'm not sure if I've added |
Global fetch uses its own cookie store, so you'll need something like this: diff --git i/packages/playwright-core/src/server/cookieStore.ts w/packages/playwright-core/src/server/cookieStore.ts
index 34ebc7287..0ba6bd7d6 100644
--- i/packages/playwright-core/src/server/cookieStore.ts
+++ w/packages/playwright-core/src/server/cookieStore.ts
@@ -30,7 +30,7 @@ class Cookie {
// https://datatracker.ietf.org/doc/html/rfc6265#section-5.4
matches(url: URL): boolean {
- if (this._raw.secure && (url.protocol !== 'https:' && url.hostname !== 'localhost'))
+ if (this._raw.secure && (url.protocol !== 'https:' && url.hostname !== 'localhost' && !url.hostname.endsWith('.localhost')))
return false;
if (!domainMatches(url.hostname, this._raw.domain))
return false; |
Do all browsers actually treat '*.localhost' subdomains as secure? |
I created a test repo today that shows Safari, Webkit and Chromium treat subdomains of localhost as secure, at least when it comes to cookies: https://github.com/simenbrekken/playwright-localhost-secure-cookie-test |
Test results for "tests 1"1 failed 4 flaky39161 passed, 803 skipped Merge workflow run. |
|
This is the behavior that I see with the test page from your repo:
|
| @@ -43,14 +43,18 @@ export function filterCookies(cookies: channels.NetworkCookie[], urls: string[]) | |||
| continue; | |||
| if (!parsedURL.pathname.startsWith(c.path)) | |||
| continue; | |||
| if (parsedURL.protocol !== 'https:' && parsedURL.hostname !== 'localhost' && c.secure) | |||
| if (parsedURL.protocol !== 'https:' && !isLocalHostname(parsedURL.hostname) && c.secure) | |||
There was a problem hiding this comment.
Since you are changing logic for browsers too, please add a test for the cookies received by the browsers from the server. You can use proxyServer fixture, see this test for an example, you'll just need to use page.goto() for navigation. My understanding is that before this change BrowserContext.cookies('http://subdomain.localhost:3000/') would not return secure cookies and now it does.
Replaced the direct check for
parsedURL.hostname === 'localhost'with a new helper functionisLocalHostname, which also considers hostnames ending with.localhostas local. This improves flexibility and correctness when handling local hostnames.See also RFC 6761 section 6.3 on handling of
localhost.