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
set secure and samesite for cookie on delete #17255
Conversation
I reproduced the bug and what I found is Firefox is happy to delete the cookie without the extra settings, but Chrome doesn't. I checked the setIgnoreCookie() function, and replicated the same if-else during the delete. The question is: what happens if the cookie is already there, but it was made without Other solution could be to send two setcookie headers, one without and one with the extra settings. This is what the original issue's description mentions as a solution. |
@flamisz I reckon it be good to have the additional core there too if it doesn't cause any issue. This way it might be more likely that any older cookie will be deleted too (or if the original cookie was set maybe on http and then now it deletes it on https etc). If there's no harm, it would be great to have. |
@tsteur something like this? if (ProxyHttp::isHttps()) {
$this->setCookie($this->name, 'deleted', time() - 31536001, $this->path, $this->domain, TRUE, FALSE, 'None');
$this->setCookie($this->name, 'deleted', time() - 31536001, $this->path, $this->domain);
} else {
$this->setCookie($this->name, 'deleted', time() - 31536001, $this->path, $this->domain);
} or just simple (without if) $this->setCookie($this->name, 'deleted', time() - 31536001, $this->path, $this->domain, TRUE, FALSE, 'None');
$this->setCookie($this->name, 'deleted', time() - 31536001, $this->path, $this->domain); |
would probably simply do the second one without if but I'm not too much into the details. Just seems like it could maybe cover more cases where it would otherwise maybe not delete the cookie. |
I tried to replicate the case, when we have a non secure cookie on a secure site. It looks like Chrome doesn't care and will delete anyway with the secure and samesite settings, and Firefox actually deleted by itself the non-secure cookie on secure domain immediately. |
I like the simple solution, let's go with that one. Multiple set-cookie in the header is possible and it doesn't hurt anything. |
tested locally, seems to work. (also tested in safari & ie11) |
Description:
fix: #16637
Review