Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fetches are sending cookies they shouldn't #24911

Closed
pshaughn opened this issue Nov 28, 2019 · 3 comments
Closed

Fetches are sending cookies they shouldn't #24911

pshaughn opened this issue Nov 28, 2019 · 3 comments

Comments

@pshaughn
Copy link
Member

@pshaughn pshaughn commented Nov 28, 2019

WPT fetch/api/cors/cors-cookies is correctly preventing two remote origins from seeing each others' cookies, but the local origin is seeing a remote origin's cookies and vice versa.

@pshaughn pshaughn changed the title Fetches are mingling local and remote cookies Fetches are sending cookies they shouldn't Jan 24, 2020
@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Jan 24, 2020

The test names may be scarier than the actual outcome. If I remove earlier tests from the file, these don't fail; it looks like the problem is a failure to remove cookies that should be removed (which is still bad), not an actual cross-origin leak.

@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Jan 24, 2020

The cookie deletion operation here is being handled by asking the server to resend the same Set-Cookie but with max-age=0; that should then mean that it's expired when we get to the next test, but possibly we're (1) not checking expiry; (2) checking expiry within the margin of timing-granularity so it looks like 0 time has passed and we're not expired yet.

@pshaughn
Copy link
Member Author

@pshaughn pshaughn commented Jan 24, 2020

I think we're supposed to be expiring cookies during CookieStorage::cookies_for_url and not just during CookieStorage::push; if we only do it during push, then a cookie could hang around for an arbitrarily long time if we don't see another SetCookie header.

@pshaughn pshaughn mentioned this issue Jan 24, 2020
4 of 4 tasks complete
web-platform-test failures automation moved this from To do to Done Jan 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Linked pull requests

Successfully merging a pull request may close this issue.

2 participants
You can’t perform that action at this time.