Commits
-------
9a74b85 [HttpFoundation] CS and phpdoc fixes
Discussion
----------
[HttpFoundation] CS and phpdoc fixes
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
Hey
---------------------------------------------------------------------------
by travisbot at 2012-06-02T00:30:49Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1504379) (merged 2f0117f2 into 1541fe26).
---------------------------------------------------------------------------
by fabpot at 2012-06-25T14:53:18Z
@adrienbrault Can you have a look at my comments?
---------------------------------------------------------------------------
by adrienbrault at 2012-06-25T16:24:49Z
Done! Sorry for the delay
---------------------------------------------------------------------------
by travisbot at 2012-06-25T17:50:24Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1702850) (merged 9a74b851 into 58436de1).
Commits
-------
b865b09 [Session] Fix the PDO handler for mysql concurrent write
Discussion
----------
[RFC][Session] Make the PDO handler looks less hacky
Related discussion: ebc2f01e5b (commitcomment-1304221)
The current code works but looks hacky (`$dbTimeCol = CASE WHEN $dbTimeCol = :time THEN (VALUES($dbTimeCol) + 1) ELSE VALUES($dbTimeCol) END`).
Todo: wrap the mysql specific code in a `try...catch` if we choose this PR way (to be consistent with all other PDO invocations).
---------------------------------------------------------------------------
by travisbot at 2012-05-10T07:50:39Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1293131) (merged b865b096 into 48099a85).
This is a very important option which allows the cookie lifetime to be changed on migrate.
For example when a user converts from an anonymous session to a logged in session one might
wish to change from a persistent cookie to browser session (e.g. a banking application).
Commits
-------
c4ee947 Native Redis Session Storage update
665f593 NativeRedisSessionStorage added
Discussion
----------
[HttpFoundation] Native Redis Session Storage
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
---------------------------------------------------------------------------
by lstrojny at 2012-03-04T23:15:43Z
Does Symfony (or any of its dependencies) has Redis support in any form whatsoever? If not this might be a good point to decide which clients to support
---------------------------------------------------------------------------
by lsmith77 at 2012-03-04T23:36:11Z
well ideally we just get this cache interface stuff done .. for this use case it would be perfect.
---------------------------------------------------------------------------
by pulzarraider at 2012-03-05T00:35:59Z
There is RedisProfilerStorage available (based on phpredis). I prefer and write code for [phpredis](https://github.com/nicolasff/phpredis).
It's recommended by [official Redis homepage](http://redis.io/clients#PHP). [In this benchmark](http://dev.af83.com/2011/01/01/which-php-library-to-use-with-redis-the-benchmark.html
) is fastest and less memory consumpting.
But if somebody prefer predis (with phpiredis), rediska or something other widely used, there are no limitations to add support of it to Symfony.
My opinion is, that the C extension should be supported at first, because of good performance and native session storage support. Redis is quite young and the process of creating PHP clients is comparable to Memcache.
There were created pure PHP Memcache clients in the past (Google found for example [this](http://www.phpclasses.org/browse/file/20284.html) and [this](http://code.blitzaffe.com/pages/phpclasses/files/memcached_client_52-12)), but they are not being used now. Everyone, who is seriously thinking about performance, is using only the C Redis/Memcache(d)/... extensions.
---------------------------------------------------------------------------
by drak at 2012-03-05T07:40:06Z
+1 on this PR. Needs a test written though.
I don't think there is any need to wait for #3493 imo. I'll deal with it if this is merged before #3493.
Are there any PHP ini settings for this for this driver or is everything via the `session.save_path` directive? (A quick look at the C code seems to indicate there are no explicit ini directives).
---------------------------------------------------------------------------
by lstrojny at 2012-03-05T12:14:34Z
@pulzarraider I don’t necessarily disagree with the usage of phpredis, I just wanted to bring up the issues of various clients and people having different preferences about them.
---------------------------------------------------------------------------
by fabpot at 2012-03-05T14:46:22Z
@pulzarraider Can you add some unit tests before I merge?
---------------------------------------------------------------------------
by pulzarraider at 2012-03-11T20:19:57Z
@drak No there are no php.ini settings. Only RedisArray has some, but it's another feature.
@fabpot I've added simple test based on other session storage tests.
I planned to create a RedisSessionStorage, too, but I have no time for it now. This can be added later in another PR as it's independent from NativeRedisSessionStorage.
---------------------------------------------------------------------------
by drak at 2012-03-12T02:21:25Z
The code looks OK to me.
---------------------------------------------------------------------------
by fabpot at 2012-03-15T06:05:27Z
#3493 has been merged now.
---------------------------------------------------------------------------
by pulzarraider at 2012-03-16T23:21:27Z
Code updated.
It does not make sense to try and store session ini directives since they can be changes outside
of the class as they are part of the global state.
Coding stan
Commits
-------
471b564 auto_start should be false
6e2a7da Support session cookie options with cookie_ prefix
e0fba80 Properly merge session cookie_* parameters
Discussion
----------
Set session.cookie_* parameters properly
Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: /
Cookie parameters in $options are not prefixed with cookie_ the same is true for data returned from session_get_cookie_params.
I've marked this as BC because the options that get dumped into the container have different name. But I don't think anybody was actually changing them or accessing them in their bundles.
P.S. @drak also desires some credits for this PR as I incorporated some lines written by him in one of the iterations.
---------------------------------------------------------------------------
by drak at 2012-02-23T14:24:42Z
@mvrhov - what does this fix exactly? It looks like a different way of doing the same thing but now there is no default value on `cookie_httponly`.
---------------------------------------------------------------------------
by mvrhov at 2012-02-23T15:09:17Z
Like I said in description. $option contains some cookie options and none of them has cookie_ prefix.
And this prefix is needed in two cases:
- to properly merge defaults and override them with what user set
- in a foreach for for proper ini_set
Sorry non native speaker an a bit hard to explain, could you ping me in a couple of hours on IRC if this still doesn't make any sense.
---------------------------------------------------------------------------
by drak at 2012-02-23T15:29:41Z
@mvrhov - I wrote some tests for this particular code and I still don't see what this PR fixes. I'll try to catch you on IRC later on but can't guarantee it.
---------------------------------------------------------------------------
by mvrhov at 2012-02-23T16:02:41Z
added test
---------------------------------------------------------------------------
by drak at 2012-02-24T08:30:51Z
Just for reference for those reading this ticket, `session_set_cookie_params()` alters the runtime ini settings it corresponds to see http://docs.php.net/manual/en/function.session-set-cookie-params.php so we agreed to remove the special handling that was present since it is redundant.
---------------------------------------------------------------------------
by dlsniper at 2012-02-28T22:19:32Z
Hi, Is this patch relevant or not after all?
ping @drak @mvrhov
Thanks :)
---------------------------------------------------------------------------
by drak at 2012-02-29T03:34:22Z
It is relevant. Maybe I'll do the cleanup this PR by forking it if @mvrhov doesn't have time.
---------------------------------------------------------------------------
by mvrhov at 2012-02-29T05:40:47Z
Fixed the typo and changed the false to ture as reported in comments. I've also rebased. I'll see what I can do about config file change later today. Sorry for the delay, been too busy for the past week.
---------------------------------------------------------------------------
by mvrhov at 2012-02-29T08:49:23Z
I've also done the config part.
---------------------------------------------------------------------------
by mvrhov at 2012-02-29T11:01:14Z
Ok, this should be it.
---------------------------------------------------------------------------
by drak at 2012-03-01T00:59:16Z
@fabpot - looks good from my side.
Added blocks, updated links and references and fixed typos.
Note it is not possible to throw exceptions in the write or close methods of a session save handler.
Commits
-------
fb2bb65 [HttpFoundation] Fix session.cache_limiter is not set correctly
Discussion
----------
[HttpFoundation] Fix session.cache_limiter is not set correctly
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Fixes a regression after the session refactoring where extra cache control http headers are sent.
This was previously handled by [calling session_cache_limiter(false) in NativeSessionStorage](https://github.com/symfony/symfony/blob/2.0/src/Symfony/Component/HttpFoundation/SessionStorage/NativeSessionStorage.php#L81)
---------------------------------------------------------------------------
by drak at 2012-02-21T12:23:48Z
@fabpot - this code can be merged imo.
