Skip to content

[HttpFoundation] Do not send Set-Cookie header twice for deleted session cookie #47273

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

Conversation

X-Coder264
Copy link
Contributor

@X-Coder264 X-Coder264 commented Aug 14, 2022

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets Fix #47228
License MIT
Doc PR -

In a user logout flow \Symfony\Component\Security\Http\EventListener\SessionLogoutListener gets triggered which invalidates the user session by calling session_regenerate_id(true) which calls \Symfony\Component\HttpFoundation\Session\Storage\Handler\AbstractSessionHandler::destroy which calls setcookie($this->sessionName, '', $params);. Calling setcookie with the second argument (the value) being an empty string signals to PHP that we want to send a Set-Cookie HTTP header to the client so that the client deletes the cookie (aka the expiry time of the cookie gets set to some time in the past) and PHP also sets the value of that cookie to deleted -> https://github.com/php/php-src/blob/php-8.1.9/ext/standard/head.c#L124

The \Symfony\Component\HttpKernel\EventListener\AbstractSessionListener class did not handle this case as \Symfony\Component\HttpKernel\EventListener\AbstractSessionListener::onKernelResponse method would call $response->headers->clearCookie() without calling the popSessionCookie method which would then add this same cookie again which would then result in two Set-Cookie headers being sent to the client for the same cookie.

@carsonbot carsonbot added this to the 5.4 milestone Aug 14, 2022
@X-Coder264 X-Coder264 force-pushed the do-not-send-deleted-session-cookie-twice-in-the-response branch 6 times, most recently from edb3d26 to b0aef93 Compare August 14, 2022 01:52
@X-Coder264 X-Coder264 force-pushed the do-not-send-deleted-session-cookie-twice-in-the-response branch 2 times, most recently from c30457f to f29f02b Compare August 14, 2022 14:43
@X-Coder264 X-Coder264 force-pushed the do-not-send-deleted-session-cookie-twice-in-the-response branch from f29f02b to b08025d Compare August 14, 2022 14:56
@X-Coder264 X-Coder264 changed the title [HttpFoundation] Do not send Set-Cookie header twice for deleted session cookie [HttpKernel] Do not send Set-Cookie header twice for deleted session cookie Aug 14, 2022
@wouterj
Copy link
Member

wouterj commented Aug 15, 2022

For completeness: this fix has been tested by the original reporter and they confirmed this fixes their problem. ref #47228 (comment)

@carsonbot carsonbot changed the title [HttpKernel] Do not send Set-Cookie header twice for deleted session cookie [HttpFoundation] Do not send Set-Cookie header twice for deleted session cookie Aug 15, 2022
@fabpot
Copy link
Member

fabpot commented Aug 15, 2022

Thank you @X-Coder264.

@fabpot fabpot merged commit c77a645 into symfony:5.4 Aug 15, 2022
@X-Coder264 X-Coder264 deleted the do-not-send-deleted-session-cookie-twice-in-the-response branch August 15, 2022 11:49
This was referenced Aug 26, 2022
@fabpot fabpot mentioned this pull request Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants