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

[HttpFoundation] NativeSessionStorage erases Set-Cookie headers #29675

Closed
eiannone opened this issue Dec 23, 2018 · 3 comments
Closed

[HttpFoundation] NativeSessionStorage erases Set-Cookie headers #29675

eiannone opened this issue Dec 23, 2018 · 3 comments

Comments

@eiannone
Copy link
Contributor

eiannone commented Dec 23, 2018

Symfony version(s) affected: 4.2

Description
Starting or regenerating a session using NativeSessionStorage with option 'cookie_samesite' = true, erases previously set cookies.
This issue affects PHP versions < 7.3

How to reproduce

use Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage;

$storage = new NativeSessionStorage(['cookie_samesite' => true]);

// Tries to send a cookie
setcookie("TestCookie", "foo");

// This method (and also NativeSessionStorage::regenerate()) deletes the previous cookie
$storage->start(); 

$headers = headers_list();
print_r($headers); 
// We expect 'Set-Cookie: TestCookie=foo' in the headers, but it's missing.

NOTE: To properly reproduce the problem, you must delete the PHPSESSID cookie in your browser, if exists

Possible Solution
When setting session cookie with header() function, we can prevent replacing existing "Set-Cookie" headers using the second optional parameter for header() function: replace = false.

In NativeSessionStorage class, inside start() and regenerate() methods, change this code:

header(sprintf('%s; SameSite=%s', $originalCookie, $this->emulateSameSite));

into this:

header(sprintf('%s; SameSite=%s', $originalCookie, $this->emulateSameSite), false);
eiannone referenced this issue in symfony/http-foundation Dec 23, 2018
Uses `session.cookie_samesite` for PHP >= 7.3. For PHP < 7.3 it first
does a session_start(), find the emitted header, changes it, and emits
it again with the value for SameSite added.
@eiannone
Copy link
Contributor Author

eiannone added a commit to eiannone/http-foundation that referenced this issue Dec 23, 2018
Prevent replacing existing cookies when starting or regenerating session on PHP < 7.3 with 'cookie_samesite' option.
See symfony/symfony#29675
eiannone added a commit to eiannone/symfony that referenced this issue Dec 23, 2018
Prevent replacing existing cookies when starting or regenerating session on PHP < 7.3 with 'cookie_samesite' option.
See issue symfony#29675
@rpkamp
Copy link
Contributor

rpkamp commented Dec 23, 2018

Nice catch! I'll create a patch somewhere this week.

@eiannone
Copy link
Contributor Author

Thank you @rpkamp , I already submitted a pull request which fixes this: #29676

@javiereguiluz javiereguiluz changed the title [symfony/http-foundation] NativeSessionStorage erases Set-Cookie headers [HttpFoundation] NativeSessionStorage erases Set-Cookie headers Dec 26, 2018
fabpot pushed a commit to eiannone/symfony that referenced this issue Jan 1, 2019
Prevent replacing existing cookies when starting or regenerating session on PHP < 7.3 with 'cookie_samesite' option.
See issue symfony#29675
@fabpot fabpot closed this as completed Jan 1, 2019
fabpot added a commit that referenced this issue Jan 1, 2019
This PR was submitted for the master branch but it was merged into the 4.2 branch instead (closes #29676).

Discussion
----------

[HttpFoundation] Fix erasing cookies issue

| Q             | A
| ------------- | ---
| Branch?       | 4.2 (to be switched when merging)
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #29675
| License       | MIT

Prevent replacing existing cookies when starting or regenerating session on PHP < 7.3 with 'cookie_samesite' option.
See issue #29675

Commits
-------

b40801a Fix erasing cookies issue
symfony-splitter pushed a commit to symfony/http-foundation that referenced this issue Jan 1, 2019
Prevent replacing existing cookies when starting or regenerating session on PHP < 7.3 with 'cookie_samesite' option.
See issue symfony/symfony#29675
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants