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] Make sessions secure and lazy #24523

Merged
merged 1 commit into from Oct 16, 2017

Conversation

@nicolas-grekas
Member

nicolas-grekas commented Oct 11, 2017

Q A
Branch? 3.4
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? yes
Tests pass? not yet
Fixed tickets #6388, #6036, #12375, #12325
License MIT
Doc PR -

The SessionUpdateTimestampHandlerInterface (new to PHP 7.0) is mostly undocumented, and just not implemented anywhere. Yet, it's required to implement session fixation preventions and lazy write in userland session handlers (there is https://wiki.php.net/rfc/session-read_only-lazy_write which describes the behavior.)

By implementing it, we would make Symfony session handling much better and stronger. Meanwhile, doing some cookie headers management, this also gives the opportunity to fix the "don't start if session is only read issue".

So, here we are for the general idea. Now needs more (and green) tests, and review of course.

@eXtreme

This comment has been minimized.

Show comment
Hide comment
@eXtreme

eXtreme Oct 11, 2017

Contributor

wow, there's literally nothing on google about SessionUpdateTimestampHandlerInterface, yet it exists for so long ..

Contributor

eXtreme commented Oct 11, 2017

wow, there's literally nothing on google about SessionUpdateTimestampHandlerInterface, yet it exists for so long ..

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 12, 2017

Member

PR is green and ready. This is a significant improvement over the current session handlers, thanks to this new PHP 7.0 interface.

This adds two main classes:

  • AbstractSessionHandler, which implements the logic to remove/delete cookies when needed + deal with validateId()+updateTimestamp()
  • SessionHandlerProxy, which turns any handler into an instance of AbstractSessionHandler, especially useful and required for the native session handler, which doesn't implement the special interface.

The rest are related tweaks.

Member

nicolas-grekas commented Oct 12, 2017

PR is green and ready. This is a significant improvement over the current session handlers, thanks to this new PHP 7.0 interface.

This adds two main classes:

  • AbstractSessionHandler, which implements the logic to remove/delete cookies when needed + deal with validateId()+updateTimestamp()
  • SessionHandlerProxy, which turns any handler into an instance of AbstractSessionHandler, especially useful and required for the native session handler, which doesn't implement the special interface.

The rest are related tweaks.

@TerjeBr

This comment has been minimized.

Show comment
Hide comment
@TerjeBr

TerjeBr Oct 12, 2017

Have you tested this with a full symfony stack?

When a typical symfony SessionBag is empty it is not the empty string, it contains some metadata.
A new session that is started with no data will contain an array with these 3 keys: _sf2_attributes, _sf2_flashes and _sf2_meta

Because of this, this PR will not fix #6388

TerjeBr commented Oct 12, 2017

Have you tested this with a full symfony stack?

When a typical symfony SessionBag is empty it is not the empty string, it contains some metadata.
A new session that is started with no data will contain an array with these 3 keys: _sf2_attributes, _sf2_flashes and _sf2_meta

Because of this, this PR will not fix #6388

Show outdated Hide outdated ...y/Component/HttpFoundation/Session/Storage/Handler/PdoSessionHandler.php
1 => array('file', '/dev/null', 'w'),
2 => array('file', '/dev/null', 'w'),
);
if (!self::$server = @proc_open('exec php -S localhost:8053', $spec, $pipes, __DIR__.'/Fixtures')) {

This comment has been minimized.

@stof

stof Oct 12, 2017

Member

exec won't work on windows

@stof

stof Oct 12, 2017

Member

exec won't work on windows

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 12, 2017

Member

Correct, on purpose: signaling the php -S process to kill it is complex, so this is skipped on Windows as I don't want to import the Process component just for this.

@nicolas-grekas

nicolas-grekas Oct 12, 2017

Member

Correct, on purpose: signaling the php -S process to kill it is complex, so this is skipped on Windows as I don't want to import the Process component just for this.

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 12, 2017

Member

@TerjeBr thanks for the hint, that's now fixed (but not tested yet).
That allowed me to spot that the BC layer in place right now is broken. We didn't spot it because of #23003 (PhpUnit-Bridge can't deal with @runTestsInSeparateProcesses).
Fixed also.

Member

nicolas-grekas commented Oct 12, 2017

@TerjeBr thanks for the hint, that's now fixed (but not tested yet).
That allowed me to spot that the BC layer in place right now is broken. We didn't spot it because of #23003 (PhpUnit-Bridge can't deal with @runTestsInSeparateProcesses).
Fixed also.

}
if (array($key = $this->metadataBag->getStorageKey()) === array_keys($_SESSION)) {
unset($_SESSION[$key]);
}

This comment has been minimized.

@TerjeBr

TerjeBr Oct 13, 2017

If the session is empty at this point, it will be an empty array, right? Will that serialize to an empty string? Or will it be something like a:0:{}?

I am a little unfamiliar with how your tests work, could you please be so kind to point out for me the file and line where you test this?

@TerjeBr

TerjeBr Oct 13, 2017

If the session is empty at this point, it will be an empty array, right? Will that serialize to an empty string? Or will it be something like a:0:{}?

I am a little unfamiliar with how your tests work, could you please be so kind to point out for me the file and line where you test this?

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

Correct: sessions serialization is special and doesn't use "serialiaze()" but a variant of it. The empty session array is represented by the empty string (that's why session handlers return the empty string when they have no data for a specific session id.)

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

Correct: sessions serialization is special and doesn't use "serialiaze()" but a variant of it. The empty session array is represented by the empty string (that's why session handlers return the empty string when they have no data for a specific session id.)

This comment has been minimized.

@TerjeBr

TerjeBr Oct 13, 2017

Just out of curiosity, will null also serialize to an empty string?

You did not tell me about the test, I assume the full use of a nativeSessionStorage bag is covered in the tests?

@TerjeBr

TerjeBr Oct 13, 2017

Just out of curiosity, will null also serialize to an empty string?

You did not tell me about the test, I assume the full use of a nativeSessionStorage bag is covered in the tests?

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

"null" is not a valid value for $_SESSION so I don't know how PHP will behave if you such such a thing?

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

"null" is not a valid value for $_SESSION so I don't know how PHP will behave if you such such a thing?

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

Oh, about the tests, there is none right now, I'm on it :)

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

Oh, about the tests, there is none right now, I'm on it :)

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

I opened igbinary/igbinary#146 about igbinary, and meanwhile it is now handled in the code. All other serialization handlers store the empty session as the empty string.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

I opened igbinary/igbinary#146 about igbinary, and meanwhile it is now handled in the code. All other serialization handlers store the empty session as the empty string.

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

This comment has been minimized.

@TerjeBr

TerjeBr Oct 13, 2017

I do not see where you have tested with all the different session handlers.

And in which test do you test that it does not start a session if just just read from a session bag?

@TerjeBr

TerjeBr Oct 13, 2017

I do not see where you have tested with all the different session handlers.

And in which test do you test that it does not start a session if just just read from a session bag?

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

The existing session handlers are already tested, there's no need to test them again. See fixture I linked.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

The existing session handlers are already tested, there's no need to test them again. See fixture I linked.

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

storage.php

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

storage.php

if (self::LOCK_TRANSACTIONAL === $this->lockMode && 'sqlite' !== $this->driver) {
if (!ini_get('session.use_strict_mode') && self::LOCK_TRANSACTIONAL === $this->lockMode && 'sqlite' !== $this->driver) {
// In strict mode, session fixation is not possible: new sessions always start with a unique
// random id, so that concurrency is not possible and this code path can be skipped.

This comment has been minimized.

@Tobion

Tobion Oct 13, 2017

Member

I don't agree. If the session just expired or was deleted manually, there can still be race conditions.

@Tobion

Tobion Oct 13, 2017

Member

I don't agree. If the session just expired or was deleted manually, there can still be race conditions.

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

Can you describe the scenario?
To me, when the session expired, or when the session was deleted in another place,
in strict mode only, php will generate a new id, so that there is no race possible.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

Can you describe the scenario?
To me, when the session expired, or when the session was deleted in another place,
in strict mode only, php will generate a new id, so that there is no race possible.

This comment has been minimized.

@Tobion

Tobion Oct 13, 2017

Member

I wonder how php knows if the session is new? so when you do session_start() with an old id which expired, the save handler is executed which returns an empty string. Does php finish the save handler by calling close() etc? Then generates a new session id i guess. Does it call the save handler open, read etc again with the new id? Or does it just call save?

@Tobion

Tobion Oct 13, 2017

Member

I wonder how php knows if the session is new? so when you do session_start() with an old id which expired, the save handler is executed which returns an empty string. Does php finish the save handler by calling close() etc? Then generates a new session id i guess. Does it call the save handler open, read etc again with the new id? Or does it just call save?

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

Thanks to the new interface, PHP calls "validateId", which returns false if the handler's "read" returns the empty string.
Then, PHP calls "read()" directly, but now with a new random id.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

Thanks to the new interface, PHP calls "validateId", which returns false if the handler's "read" returns the empty string.
Then, PHP calls "read()" directly, but now with a new random id.

This comment has been minimized.

@Tobion

Tobion Oct 13, 2017

Member

This is important also for the advisory locks which might otherwise not be released at all if it just calls save() with a new id.

@Tobion

Tobion Oct 13, 2017

Member

This is important also for the advisory locks which might otherwise not be released at all if it just calls save() with a new id.

This comment has been minimized.

@Tobion

Tobion Oct 13, 2017

Member

I think the validation is only about checking if the session id has invalid characters in it according to the patch for strict sessions https://gist.github.com/yohgaki/1379668#file-php-trunk-strict-session-patch-L55

Thus I don't think validation should actually read the data. It is only about checking if the provided session id (from a hacker) is fake and might break the save handler. If yes, php generates a new session id, which php calls read with.

@Tobion

Tobion Oct 13, 2017

Member

I think the validation is only about checking if the session id has invalid characters in it according to the patch for strict sessions https://gist.github.com/yohgaki/1379668#file-php-trunk-strict-session-patch-L55

Thus I don't think validation should actually read the data. It is only about checking if the provided session id (from a hacker) is fake and might break the save handler. If yes, php generates a new session id, which php calls read with.

This comment has been minimized.

@Tobion

Tobion Oct 13, 2017

Member

What I wonder is, if php implements a default validation of the id, e.g. by checking the length of the session id against session.sid_length php ini. Something like that seems more the correct approach to me.

@Tobion

Tobion Oct 13, 2017

Member

What I wonder is, if php implements a default validation of the id, e.g. by checking the length of the session id against session.sid_length php ini. Something like that seems more the correct approach to me.

This comment has been minimized.

@Tobion

Tobion Oct 13, 2017

Member

Reading https://wiki.php.net/rfc/strict_sessions

This patch adds validate_id() function to ps_modules (session save handler) that can check if the session ID is already initialized one or not. Bundled ps_modules are now have validate function and will never accept uninitialized session ID

it suggests validate id is really about checking whether the session is initialized. Then the current solution is fine. I'd suggest to prevent new session from being read twice, to do

if ($prefetchId === $sessionId || '' === $prefetchData) {
    return $prefetchData;
}
@Tobion

Tobion Oct 13, 2017

Member

Reading https://wiki.php.net/rfc/strict_sessions

This patch adds validate_id() function to ps_modules (session save handler) that can check if the session ID is already initialized one or not. Bundled ps_modules are now have validate function and will never accept uninitialized session ID

it suggests validate id is really about checking whether the session is initialized. Then the current solution is fine. I'd suggest to prevent new session from being read twice, to do

if ($prefetchId === $sessionId || '' === $prefetchData) {
    return $prefetchData;
}

This comment has been minimized.

@Tobion

Tobion Oct 13, 2017

Member

Back to the original question. With the validateId and strict session in place, race conditions on new session ids cannot happen indeed. So it's fine to skip inserting fake rows.

@Tobion

Tobion Oct 13, 2017

Member

Back to the original question. With the validateId and strict session in place, race conditions on new session ids cannot happen indeed. So it's fine to skip inserting fake rows.

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

if ($prefetchId === $sessionId || '' === $prefetchData) {

good idea, done

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

if ($prefetchId === $sessionId || '' === $prefetchData) {

good idea, done

Array
(
[0] => Content-Type: text/plain; charset=utf-8
[1] => Cache-Control: private, max-age=10800

This comment has been minimized.

@Tobion

Tobion Oct 13, 2017

Member

http://php.net/manual/en/function.session-cache-limiter.php says something about pre-check. So php documentation is wrong about that? pre-check doesn't seem to be a standard anyway.

@Tobion

Tobion Oct 13, 2017

Member

http://php.net/manual/en/function.session-cache-limiter.php says something about pre-check. So php documentation is wrong about that? pre-check doesn't seem to be a standard anyway.

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

Looks like so yes...

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

Looks like so yes...

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

PR green and ready :)

Member

nicolas-grekas commented Oct 13, 2017

PR green and ready :)

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

(fixes current failures on 3.4)

Member

nicolas-grekas commented Oct 13, 2017

(fixes current failures on 3.4)

@TerjeBr

This comment has been minimized.

Show comment
Hide comment
@TerjeBr

TerjeBr Oct 13, 2017

@nicolas-grekas wrote:

I opened igbinary/igbinary#146 about igbinary, and meanwhile it is now handled in the code. All other serialization handlers store the empty session as the empty string.

What about the php_serialize handler? From http://php.net/manual/en/session.configuration.php#ini.session.serialize-handler:

php_serialize uses plain serialize/unserialize function internally

serialize(array()) returns "a:0:{}" and not an empty string.

TerjeBr commented Oct 13, 2017

@nicolas-grekas wrote:

I opened igbinary/igbinary#146 about igbinary, and meanwhile it is now handled in the code. All other serialization handlers store the empty session as the empty string.

What about the php_serialize handler? From http://php.net/manual/en/session.configuration.php#ini.session.serialize-handler:

php_serialize uses plain serialize/unserialize function internally

serialize(array()) returns "a:0:{}" and not an empty string.

[0] => bar
)
$_SESSION is not empty
write

This comment has been minimized.

@Tobion

Tobion Oct 13, 2017

Member

I thought the lazy_write ini does not call write if data didn't change. The data was empty at the start of the session and it is empty at the end (because the flash got added and removed again by reading it). So why does it call write anyway?

@Tobion

Tobion Oct 13, 2017

Member

I thought the lazy_write ini does not call write if data didn't change. The data was empty at the start of the session and it is empty at the end (because the flash got added and removed again by reading it). So why does it call write anyway?

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

lazy write calls updateTimestamp instead of write, but only when the session is not empty.
When it's empty, it calls write.

@nicolas-grekas

nicolas-grekas Oct 13, 2017

Member

lazy write calls updateTimestamp instead of write, but only when the session is not empty.
When it's empty, it calls write.

@Tobion

Tobion approved these changes Oct 16, 2017

Nice work @nicolas-grekas. Improving the session system was long overdue and I like how you solved it.

@@ -401,11 +408,13 @@ public function setSaveHandler($saveHandler = null)
if (!$saveHandler instanceof AbstractProxy && $saveHandler instanceof \SessionHandlerInterface) {
$saveHandler = new SessionHandlerProxy($saveHandler);
} elseif (!$saveHandler instanceof AbstractProxy) {
$saveHandler = new SessionHandlerProxy(new \SessionHandler());
$saveHandler = new SessionHandlerProxy(new StrictSessionHandler(new \SessionHandler()));

This comment has been minimized.

@Tobion

Tobion Oct 16, 2017

Member

It feels strange that we need to make the native session handlers strict. The files session handler is already strict I assume. So this is only needed because if you use \SessionHandler, you lose that strictness? \SessionHandler does not implement \SessionUpdateTimestampHandlerInterface! So if we not not set a save handler at all it would even be better as it would then use the validate id implementation of the native session handler? But currently that is not possible do.

@Tobion

Tobion Oct 16, 2017

Member

It feels strange that we need to make the native session handlers strict. The files session handler is already strict I assume. So this is only needed because if you use \SessionHandler, you lose that strictness? \SessionHandler does not implement \SessionUpdateTimestampHandlerInterface! So if we not not set a save handler at all it would even be better as it would then use the validate id implementation of the native session handler? But currently that is not possible do.

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 16, 2017

Member

Correct, I don't think we can do better here, id validation works with the wrapper so not a big issue. There is only updateTimestamp which ships no optimization but only writes. Not sure we can do better.

@nicolas-grekas

nicolas-grekas Oct 16, 2017

Member

Correct, I don't think we can do better here, id validation works with the wrapper so not a big issue. There is only updateTimestamp which ships no optimization but only writes. Not sure we can do better.

This comment has been minimized.

@Tobion

Tobion Oct 16, 2017

Member

If php doesn't change this behavior, we can think about deprecating NativeFilesSessionHandler in SF 4.1. So by default it just sets the session.save_handler ini without actually overwriting \SessionHandler.

@Tobion

Tobion Oct 16, 2017

Member

If php doesn't change this behavior, we can think about deprecating NativeFilesSessionHandler in SF 4.1. So by default it just sets the session.save_handler ini without actually overwriting \SessionHandler.

public function read($sessionId)
public function updateTimestamp($sessionId, $data)
{
$expiry = $this->createDateTime(time() + (int) ini_get('session.gc_maxlifetime'));

This comment has been minimized.

@TerjeBr

TerjeBr Oct 16, 2017

This is duplicated code from function doWrite (line 144). Would it not be better to put this common code in a new protected method? May be something like createExpiryTime.

@TerjeBr

TerjeBr Oct 16, 2017

This is duplicated code from function doWrite (line 144). Would it not be better to put this common code in a new protected method? May be something like createExpiryTime.

This comment has been minimized.

@nicolas-grekas

nicolas-grekas Oct 16, 2017

Member

not worth it for one line IMHO

@nicolas-grekas

nicolas-grekas Oct 16, 2017

Member

not worth it for one line IMHO

@chalasr chalasr added this to RFR in Session Oct 16, 2017

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 16, 2017

Member

comments addressed

Member

nicolas-grekas commented Oct 16, 2017

comments addressed

@fabpot

fabpot approved these changes Oct 16, 2017

@fabpot

This comment has been minimized.

Show comment
Hide comment
@fabpot

fabpot Oct 16, 2017

Member

Thank you @nicolas-grekas.

Member

fabpot commented Oct 16, 2017

Thank you @nicolas-grekas.

@fabpot fabpot merged commit 347939c into symfony:3.4 Oct 16, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details

fabpot added a commit that referenced this pull request Oct 16, 2017

feature #24523 [HttpFoundation] Make sessions secure and lazy (nicola…
…s-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[HttpFoundation] Make sessions secure and lazy

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | not yet
| Fixed tickets | #6388, #6036, #12375, #12325
| License       | MIT
| Doc PR        | -

The `SessionUpdateTimestampHandlerInterface` (new to PHP 7.0) is mostly undocumented, and just not implemented anywhere. Yet, it's required to implement session fixation preventions and lazy write in userland session handlers (there is https://wiki.php.net/rfc/session-read_only-lazy_write which describes the behavior.)

By implementing it, we would make Symfony session handling much better and stronger. Meanwhile, doing some cookie headers management, this also gives the opportunity to fix the "don't start if session is only read issue".

So, here we are for the general idea. Now needs more (and green) tests, and review of course.

Commits
-------

347939c [HttpFoundation] Make sessions secure and lazy

@nicolas-grekas nicolas-grekas deleted the nicolas-grekas:session-remove-on-empty branch Oct 16, 2017

@Tobion

This comment has been minimized.

Show comment
Hide comment
@Tobion

Tobion Oct 16, 2017

Member

@nicolas-grekas what I proposed in #12325 is to not start the session at all until data is set when there is no session cookie in the request. I don't think that it implemented yet. This would avoid generating a session id, starting the session handler etc when you just read session data to then realize nothing is there.

Member

Tobion commented Oct 16, 2017

@nicolas-grekas what I proposed in #12325 is to not start the session at all until data is set when there is no session cookie in the request. I don't think that it implemented yet. This would avoid generating a session id, starting the session handler etc when you just read session data to then realize nothing is there.

@nicolas-grekas

This comment has been minimized.

Show comment
Hide comment
@nicolas-grekas

nicolas-grekas Oct 16, 2017

Member

@Tobion from the HTTP pov, the observed behavior is exactly the same. In fact, starting the session has the benefit of sending the appropriate Cache-Control header. If HTTP cacheability is improved by the current change, it means we may not have to care anymore about whether the session is really started or not.

Member

nicolas-grekas commented Oct 16, 2017

@Tobion from the HTTP pov, the observed behavior is exactly the same. In fact, starting the session has the benefit of sending the appropriate Cache-Control header. If HTTP cacheability is improved by the current change, it means we may not have to care anymore about whether the session is really started or not.

@Tobion

This comment has been minimized.

Show comment
Hide comment
@Tobion

Tobion Oct 16, 2017

Member

If I just want to check if the user is logged in to then redirect or whatever, I don't need to start the session if there is no session cookie. And a cache control header might not be what I want.

Member

Tobion commented Oct 16, 2017

If I just want to check if the user is logged in to then redirect or whatever, I don't need to start the session if there is no session cookie. And a cache control header might not be what I want.

@chalasr chalasr moved this from RFR to DONE in Session Oct 18, 2017

This was referenced Oct 18, 2017

fabpot added a commit that referenced this pull request Oct 19, 2017

minor #24623 [Session] remove lazy_write polyfill for php < 7.0 (Tobion)
This PR was merged into the 4.0-dev branch.

Discussion
----------

[Session] remove lazy_write polyfill for php < 7.0

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    |no
| Deprecations? | no <!-- don't forget to update UPGRADE-*.md files -->
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Remove the session.lazy_write fallback implementation for php < 7 introduced in #24523 as we don't need it in sf 4

Commits
-------

1f84b1f [Session] remove lazy_write polyfill for php < 7.0

ddeboer added a commit to driebit/SessionStorageHandlerChainBundle that referenced this pull request Dec 3, 2017

Add Symfony 3.4 compatibility
* Make compatible with  symfony/symfony#24523.
* Open session in all readers/writers to fix error
  during session_regenerate_id.
* Return boolean from close(), which Symfony expects.

@Seldaek Seldaek referenced this pull request Mar 29, 2018

Merged

New session handler #404

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment