Skip to content

Commit

Permalink
fix: return non-secure cookies with HTTPS URLs (#5507)
Browse files Browse the repository at this point in the history
Cookies have a "Secure" attribute which tells the browsers
that a given cookie should only be sent via HTTPS. In it's
absense "Secure" is falsy and these cookies should be sent
with both HTTP and HTTPS requests. Playwright now returns
only the "Non-Secure" cookies for HTTP URLs, and both
"Secure" and "Non-Secure" cookies for HTTPS URLs.

Fixes #5504
  • Loading branch information
zevisert committed Feb 19, 2021
1 parent a9c91b0 commit cbcc609
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 9 deletions.
2 changes: 1 addition & 1 deletion src/server/network.ts
Expand Up @@ -36,7 +36,7 @@ export function filterCookies(cookies: types.NetworkCookie[], urls: string[]): t
continue;
if (!parsedURL.pathname.startsWith(c.path))
continue;
if ((parsedURL.protocol === 'https:') !== c.secure)
if (parsedURL.protocol !== 'https:' && c.secure)
continue;
return true;
}
Expand Down
57 changes: 49 additions & 8 deletions test/browsercontext-cookies.spec.ts
Expand Up @@ -106,10 +106,9 @@ it('should get multiple cookies', async ({context, page, server}) => {
document.cookie = 'password=1234';
return document.cookie.split('; ').sort().join('; ');
});
const cookies = await context.cookies();
cookies.sort((a, b) => a.name.localeCompare(b.name));
const cookies = new Set(await context.cookies());
expect(documentCookie).toBe('password=1234; username=John Doe');
expect(cookies).toEqual([
expect(cookies).toEqual(new Set([
{
name: 'password',
value: '1234',
Expand All @@ -130,7 +129,7 @@ it('should get multiple cookies', async ({context, page, server}) => {
secure: false,
sameSite: 'None',
},
]);
]));
});

it('should get cookies from multiple urls', async ({context}) => {
Expand All @@ -147,9 +146,8 @@ it('should get cookies from multiple urls', async ({context}) => {
name: 'birdo',
value: 'tweets',
}]);
const cookies = await context.cookies(['https://foo.com', 'https://baz.com']);
cookies.sort((a, b) => a.name.localeCompare(b.name));
expect(cookies).toEqual([{
const cookies = new Set(await context.cookies(['https://foo.com', 'https://baz.com']));
expect(cookies).toEqual(new Set([{
name: 'birdo',
value: 'tweets',
domain: 'baz.com',
Expand All @@ -167,7 +165,7 @@ it('should get cookies from multiple urls', async ({context}) => {
httpOnly: false,
secure: true,
sameSite: 'None',
}]);
}]));
});

it('should work with subdomain cookie', async ({context, page, server}) => {
Expand Down Expand Up @@ -210,3 +208,46 @@ it('should not return cookies with empty value', async ({context, page, server})
expect(cookies.length).toBe(0);
});

it('should return secure cookies based on HTTP(S) protocol', async ({context}) => {
await context.addCookies([{
url: 'https://foo.com',
name: 'doggo',
value: 'woofs',
secure: true
}, {
url: 'http://foo.com',
name: 'catto',
value: 'purrs',
secure: false
}]);
const cookies = new Set(await context.cookies('https://foo.com'));
expect(cookies).toEqual(new Set([{
name: 'catto',
value: 'purrs',
domain: 'foo.com',
path: '/',
expires: -1,
httpOnly: false,
secure: false,
sameSite: 'None',
}, {
name: 'doggo',
value: 'woofs',
domain: 'foo.com',
path: '/',
expires: -1,
httpOnly: false,
secure: true,
sameSite: 'None',
}]));
expect(await context.cookies('http://foo.com/')).toEqual([{
name: 'catto',
value: 'purrs',
domain: 'foo.com',
path: '/',
expires: -1,
httpOnly: false,
secure: false,
sameSite: 'None',
}]);
});

0 comments on commit cbcc609

Please sign in to comment.