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).
Commits
-------
c195957 [Components] Tests/Autoloading fixes
Discussion
----------
Fix components
See #4141
----
This PR:
* configures each component to use composer to manage "dev" dependencies instead of env variables;
* adds phpunit configuration file on Filesystem component;
* fixes READMEs.
It's mergeable without any problems, but I would recommend to wait a fix in Composer in order to use `self.version` in `require`/`require-dev` sections.
Note: I kept `suggest` sections because it makes sense but this PR doesn't aim to provide useful explanations for each entry. It could be another PR, not that one.
---------------------------------------------------------------------------
by willdurand at 2012-04-30T20:43:13Z
@fabpot I reviewed each component, one by one. Now `phpunit` always works, even if tests are skipped. A simple `composer install --dev` allows to run the complete test suite. Each commit is well separated from the others. I guess, everything is ok now.
---------------------------------------------------------------------------
by Tobion at 2012-04-30T20:47:00Z
Please squash, as it makes no sense to have the same commit for each component.
---------------------------------------------------------------------------
by fabpot at 2012-05-01T14:26:11Z
Can you squash your commits before I merge? Thanks.
---------------------------------------------------------------------------
by willdurand at 2012-05-01T14:29:38Z
done
---------------------------------------------------------------------------
by fabpot at 2012-05-01T15:48:25Z
It does not seem that the commits are squashed.
---------------------------------------------------------------------------
by willdurand at 2012-05-01T15:54:08Z
done
* Switched to Composer to manage "dev" dependencies
* Fixed READMEs
* Excluded vendor in phpunit.xml.dist files
* Fixed message in bootstrap.php files
* Added autoloader for the component itself
Commits
-------
6756f28 [Session] Fixed Backward Compatibility issue with getFlashes()
Discussion
----------
[Session] Fixed Backward Compatibility issue with getFlashes()
---------------------------------------------------------------------------
by fabpot at 2012-04-25T22:35:42Z
ping @drak
---------------------------------------------------------------------------
by willdurand at 2012-04-25T22:37:01Z
By the way, I had this issue on a real application I upgraded from Symfony2 2.0.x to 2.1 (and written by @Seldaek)
The code looks like:
``` php
<?php
// in a controller
$this->session->setFlash('foo', array(
'code' => 'success',
'message' => 'lalala',
'params' => array())
);
```
---------------------------------------------------------------------------
by Seldaek at 2012-04-26T07:25:03Z
Yup, to be fair in retrospective maybe that should have been translated in the controller directly (that's why it had message + params as an array), but this is code that predates 2.0 by at least six months, so it was obviously not clear what best practices were. Anyway it seems it can be fixed without much harm, so for the sake of safety and because I may not be the only crazy person having done this, it'd be good to fix IMO.
Commits
-------
40df3bf Add mongodb session storage
Discussion
----------
[HttpFoundation][Session] Add mongodb session storage
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
---------------------------------------------------------------------------
by Baachi at 2012-04-19T19:05:19Z
Review please :)
---------------------------------------------------------------------------
by Baachi at 2012-04-19T19:49:42Z
@stof Can be merged?
---------------------------------------------------------------------------
by stof at 2012-04-19T19:51:28Z
I'm not a Mongo expert but it seems fine. You simply need to wait @fabpot's final review now
---------------------------------------------------------------------------
by Baachi at 2012-04-19T19:52:53Z
Okay, thanks :)
---------------------------------------------------------------------------
by Baachi at 2012-04-20T06:21:52Z
@vicb Sorry, for the email flood :)
I implemented all your suggestions.
---------------------------------------------------------------------------
by fabpot at 2012-04-22T08:27:19Z
@drak, @vicb: Is it ok now?
---------------------------------------------------------------------------
by vicb at 2012-04-22T08:33:31Z
I am ok with this PR
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #1813
Todo: -
In order to work, add this to the .htaccess:
RewriteEngine on
RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^(.*)$ app.php [QSA,L]
Commits
-------
8a0e6d2 [HttpFoundation] Update changelog.
4fc04fa [HttpFoundation] Renamed MetaBag to MetadataBag
2f03b31 [HttpFoundation] Added the ability to change the session cookie lifetime on migrate().
39141e8 [HttpFoundation] Add ability to force the lifetime (allows update of session cookie expiry-time)
ec3f88f [HttpFoundation] Add methods to interface
402254c [HttpFoundation] Changed meta-data responsibility to SessionStorageInterface
d9fd14f [HttpFoundation] Refactored for moved tests location.
29bd787 [HttpFoundation] Added some basic meta-data to Session
Discussion
----------
[2.1][HttpFoundation] Added some basic meta-data to Session
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
References the following tickets: #2171
Todo: -
Session data is stored as an encoded string against a single id. If we want to store meta-data about the session, that data has to be stored as part of the session data to ensure the meta-data can persist using any session save handler.
This patch makes it much easier to determine the logic of session expiration. In general a session expiry can be dealt with by the gc handlers, however, in some applications more specific expiry rules might be required.
Session expiry may also be more complex than a simple, session was idle for x seconds. For example, in Zikula there are three security settings, Low, Medium and High. The rules for session expiry are more complex as under the Medium setting, a session will expire after x minutes idle time, unless the rememberme option was ticked on login. If so, the session will not idle. This gives the user some control over their experience. Under the high security setting, then there is no option, sessions will expire after the idle time is reached and login the UI has the rememberme checkbox removed.
The other advantage is that under this methodology, there can be a UI experience on expiry, like "Sorry, your session expired due to being idle for 10 minutes".
Keeping in the spirit of Symfony2 Components, I am seeking to make session handling flexible enough to accommodate these general requirements without specifically covering expiration rules. It would mean that it would be up to the implementing application to specifcally check and expire session after starting it.
Expiration might look something like this:
$session->start();
if (time() - $session->getMetadataBag()->getLastUpdate() > $maxIdleTime) {
$session->invalidate();
throw new SessionExpired();
}
This commit also brings the ability to change the `cookie_lifetime` when migrating a session. This means one could move from a default of browser only session cookie to long-lived cookie when changing from a anonymous to a logged in user for example.
$session->migrate($destroy, $lifetime);
---------------------------------------------------------------------------
by drak at 2012-03-30T18:18:43Z
@fabpot I have removed [WIP] status.
---------------------------------------------------------------------------
by drak at 2012-03-31T13:34:57Z
NB: This PR has been rebased and the tests relocated as per recent master changes.
---------------------------------------------------------------------------
by drak at 2012-04-03T02:16:43Z
@fabpot - ping
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).
This commit allows applications to know certain meta-data about the session
Session storage is designed to only store some data against a session ID
so this method is necessary to be compatible with any session handler, including
native handlers.
Commits
-------
8dd2c27 [HttpFoundation] Further micro-optimization.
54c5d5e [HttpFoundation] Micro-optimisation.
Discussion
----------
[HttpFoundation] Micro-optimisation.
Ref #3729
---------------------------------------------------------------------------
by robocoder at 2012-03-30T11:45:02Z
If you pre-flip your $validOptions arrays, you can use isset() instead of in_array() in the loop.
This changes the performance from O(m * n) to O(m).
---------------------------------------------------------------------------
by drak at 2012-03-30T11:53:24Z
@robocoder What is the expense of the array_flip though?
---------------------------------------------------------------------------
by robocoder at 2012-03-30T11:56:21Z
Why would you use array_flip if the array doesn't change? Change $validOptions = array('x', 'y', ...) to $validOptions = array('x' => 0, 'y' => 0, ...), then change the in_array() to use isset().
---------------------------------------------------------------------------
by stof at 2012-03-30T11:57:08Z
@drak a loop. But it will be done only once before the other loop so it will be O(n + m) instead of O(m * n)
---------------------------------------------------------------------------
by drak at 2012-03-30T12:00:47Z
Ok :)
Commits
-------
5ae76f1 [HttpFoundation] Update documentation.
910b5c7 [HttpFoudation] CS, more tests and some optimization.
b0466e8 [HttpFoundation] Refactored BC Session class methods.
84c2e3c [HttpFoundation] Allow flash messages to have multiple messages per type.
Discussion
----------
[2.1][HttpFoundation] Multiple session flash messages
Bug fix: no
Feature addition: yes
Backwards compatibility break: yes, but this already happened in #2583. BC `Session` methods remain unbroken.
Symfony2 tests pass: yes
Fixes the following tickets: #1863
References the following tickets: #2714, #2753, #2510, #2543, #2853
Todo: -
This PR alters flash messages so that it is possible to store more than one message per flash type using the `add()` method or by passing an array of messages to `set()`.
__NOTES ABOUT BC__
This PR maintains BC behaviour with the `Session` class in that the old Symfony 2.0 methods will continue to work as before.
---------------------------------------------------------------------------
by drak at 2012-02-13T06:28:33Z
I think this is ready for review @fabpot @lsmith77
---------------------------------------------------------------------------
by lsmith77 at 2012-02-14T19:30:39Z
the FlashBag vs. AutoExpireFlashBag behavior and setup difference should probably also be explained in the upgrading log
---------------------------------------------------------------------------
by drak at 2012-02-15T04:43:14Z
@lsmith77 Those differences are explained already in the changelog
* Added `FlashBag`. Flashes expire when retrieved by `get()` or `all()`.
This makes the implementation ESI compatible.
* Added `AutoExpireFlashBag` (default) to replicate Symfony 2.0.x auto expire behaviour of messages auto expiring
after one page page load. Messages must be retrived by `get()` or `all()`.
---------------------------------------------------------------------------
by Crell at 2012-02-19T17:35:34Z
Drak asked me to weigh in here with use cases. Drupal currently has a similar session-stored-messaging system in place that I'd like to be able to replace with Flash messages. We frequently have multiple messages within a single request, however, so this change is critical to our being able to do so.
For instance, when saving an article in Drupal there is, by default, a "yay, you saved an article!" type message that gets displayed. If you also have the site configured to send email when a post is updated, you may see a "email notifications sent" message (depending on your access level). If you have a Solr server setup for search, and you're in debug mode, there will also be a "record ID X added to Solr, it should update in 2 minutes" message. And if there's a bug somewhere, you'll also get, as an error message rather than notice message, a "Oops, E_NOTICE on line 54" message.
Form validation is another case. If you have multiple errors in a single form, we prefer to list all of them. So if you screw up 4 times on a form, you may get 4 different error messages showing what you screwed up so you can fix it in one go instead of several.
Now sure, one could emulate that by building a multi-message layer on top of single-layer messages, but, really, why? "One is a special case of many", and there are many many cases where you'll want to post multiple messages. Like, most of Drupal. :-)
---------------------------------------------------------------------------
by lsmith77 at 2012-03-06T20:55:51Z
@fabpot is there any information you still need before merging this? do you want more discussion in which case you might want to take this to the mailing list ..
---------------------------------------------------------------------------
by drak at 2012-03-08T18:54:13Z
Another plus for this PR is that it requires no extra lines of code in templates etc to display the flashes, see https://github.com/symfony/symfony/pull/3267/files#diff-1
---------------------------------------------------------------------------
by drak at 2012-03-15T06:38:21Z
Rebased against current `master`, should be mergeable again..
---------------------------------------------------------------------------
by evillemez at 2012-03-17T03:08:41Z
+1 to this, I have an extended version of HttpFoundation just for this... would love to get rid of it.
Commits
-------
bd02554 [HttpFoundation] SPL IteratorAggregate+Countable on *Bags
665fdeb [HttpFoundation] SPL on ParameterBag
Discussion
----------
[HttpFoundation] SPL on ParameterBag
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Added a couple SPL interfaces to ParameterBag, added shortcuts to working with the parameters. For example:
```php
<?php
$post = Request::createFromGlobal()->request;
echo "There are {count($post)} POST variables\n";
foreach ($post as $key => $val) {
echo "{$key}: {$val}\n";
}
```
Thoughts?
---------------------------------------------------------------------------
by stealth35 at 2012-03-07T13:09:11Z
You already have the `all` method
``` php
<?php
$post = Request::createFromGlobals()->request->all();
echo "There are ", count($post), " POST variables\n";
foreach ($post as $key => $val) {
echo "{$key}: {$val}\n";
}
```
---------------------------------------------------------------------------
by cboden at 2012-03-07T13:50:22Z
Yes, but when in the context of working with the Request object (or POST ParamegerBag), it's 1 more call and loose variable to set.
ParameterBag is a container, these common SPL interfaces give standard PHP container methods to it.
---------------------------------------------------------------------------
by lsmith77 at 2012-03-07T18:42:41Z
makes sense to me ..
---------------------------------------------------------------------------
by vicb at 2012-03-09T15:45:40Z
Probably makes sense. Could you check if any other `*Bag.php` needs to be updated so that it could ba an atomic merge.
---------------------------------------------------------------------------
by cboden at 2012-03-09T15:48:40Z
Whoops, good catch @vicb. I made a poor assumption all the *Bags extended ParameterBag, while only some do. I will post an update shortly.
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.
Commits
-------
5fa1c70 [json-response] Add a JsonResponse class for convenient JSON encoding
Discussion
----------
[json-response] Add a JsonResponse class for convenient JSON encoding
Usage example:
$data = array(user => $user->toArray());
return new JsonResponse($data);
---------------------------------------------------------------------------
by drak at 2012-02-16T11:51:11Z
@fabpot - maybe we could benefit with a bit more sub-namespacing in this component. One for Response for example and probably one for Request.
---------------------------------------------------------------------------
by Seldaek at 2012-02-16T15:07:31Z
@drak Please no. Moving the session was already a pain IMO since it was type-hinted in a few places (lack of interface, and interface doesn't include flash stuff still). Creating BC breaks just for fun like that is annoying for interop of bundles. It doesn't matter whether we have 10 or 15 classes in one directory.
---------------------------------------------------------------------------
by drak at 2012-02-17T08:33:46Z
@francodacosta The most optimal place is `__toString()`.
@Saldaek It just looks like the whole namespace is getting more cluttered. I suggest it because things like Request/Response objects are surely only going to grow over time. There is always the possibility to make BC for moved and renamed classes so there doesn't have to be any extra complications for making things look cleaner. Anyway, just a thought :-)
---------------------------------------------------------------------------
by stof at 2012-02-17T14:47:40Z
@drak Changing the namespace of a class is a BC break. The request and the response are used in many more places than the Session so it would be a real pain to update this. And the component is tagged with ``@api`` so BC breaks are forbidden without a good reason. The session refactoring was one as it was really an issue in the implementation, but simply renaming the class is not.
---------------------------------------------------------------------------
by fabpot at 2012-03-05T15:03:53Z
I'm -1 for adding this to the core. It does not add much value and why add a special response for JSON and not other formats?
---------------------------------------------------------------------------
by Seldaek at 2012-03-05T18:38:05Z
I think it's useful because it's a class we need in almost every project, and I don't think we're alone. It's super simple but makes me wonder every time why I have to recreate it. I don't want an additional bundle just for 3lines of code. Similarly I would say a JsonpResponse would be great, or maybe just an optional $callback arg to the json response to enable jsonp mode.
I just had someone ask me on irc how to do JSONP so while I think it's obvious and I'm sure you'd think that too, it obviously isn't to newcomers. The Response stuff is hidden behind those render methods & such and people don't realize they can simply subclass. If a few examples were in core it would be both helpful for learning and useful on a day to day basis.
As for other formats, well JSON is typically used nowadays, except when you want more fancy XML APIs, but for that the JMSSerializerBundle + FOSRestBundle are superior and we can't achieve such things in a few lines of code. I could also see a BinaryResponse or DownloadResponse or such that has proper "force-download" headers and accepts any binary stream, but that's another debate.
---------------------------------------------------------------------------
by dragoonis at 2012-03-05T19:43:05Z
I'm +1 for the concept but not commenting on how it should be implemented I'll leave that to other people.
Typically when you want to force a download you have to do ``content-disposition: attachment; filename="filehere.pdf"``
Modifying some response headers and the likes automatically for the user by returning a DownloadResponse object would be very handy..
I'm +1 for @Seldaek's point about examples of sub-classing for specific use cases. It will help with demonstrating how to do custom stuff the right way rather than people coming up with their own contraptions.
---------------------------------------------------------------------------
by stof at 2012-03-05T20:14:39Z
btw, regarding the BinaryResponse, there is a pending PR about it: #2606
---------------------------------------------------------------------------
by simensen at 2012-03-05T21:07:33Z
I'm +1 for providing reference implementations fo custom Response cases. I wanted to find best practices for handling JSONP requests/responses and couldn't find anything at all on the topic. I thought maybe extending Response might be useful but wasn't sure if that could be done safely or should be done at all.
---------------------------------------------------------------------------
by lsmith77 at 2012-03-05T22:28:01Z
@stof i think @drak was suggesting moving the class, but leaving an empty class extending from the new class in the old location to maintain BC
---------------------------------------------------------------------------
by stof at 2012-03-05T23:55:36Z
@lsmith77 This would force Symfony to use the BC class so that it does not break all typehints in existing code
---------------------------------------------------------------------------
by lsmith77 at 2012-03-06T00:22:15Z
BC hacks are never nice .. the goal would just be to eventually have all those classes and more importantly all new ones in a subnamespace. actually it might be easier to just leave all the classes in the old location and create new ones extending from the old ones. anyway .. personally i am also not such a big fan of these specialized responses .. but i guess i see FOSRestBundle as the alternative answer which makes me biased.
---------------------------------------------------------------------------
by Seldaek at 2012-03-06T07:57:36Z
I'm using FOSRestBundle when it's needed, but when you just have a small scale app that needs one or two json responses for specialized stuff it is slightly overkill. And again, newcomers probably won't know about it, and encouraging using it for simple use cases isn't exactly the best learning curve we can provide.
---------------------------------------------------------------------------
by COil at 2012-03-06T23:12:15Z
+1 for this. I have implemented such a function in all my sf1 projects, it will be the same for sf2.
---------------------------------------------------------------------------
by fabpot at 2012-03-15T13:22:27Z
Closing this PR in favor of a cookbook that explains how a developer can override the default Response class (this JSON class being a good example). see symfony/symfony-docs#1159
---------------------------------------------------------------------------
by Seldaek at 2012-03-15T13:25:08Z
Meh. Forcing people to copy paste code from the cookbook in every second project isn't exactly a step forward with regard to ease of use and user-friendliness.
---------------------------------------------------------------------------
by Seldaek at 2012-03-15T13:26:48Z
I mean following this logic, things like the X509 authentication should just be put in cookbooks too because almost nobody needs that. We have tons of code in the framework, I don't get the resistance with adding such a simple class which makes code more expressive.
---------------------------------------------------------------------------
by fabpot at 2012-03-15T13:53:07Z
because X509 authentication is not easy to get it right. Sending a JSON response is as simple as it can get:
new Response(json_encode($data), 200, array('Content-Type' => 'application/json'));
---------------------------------------------------------------------------
by marijn at 2012-03-15T13:54:25Z
Perhaps we need a `Symfony\Extensions\{Component}` namespace for things that don't necessarily belong in the core but are truly useful...
---------------------------------------------------------------------------
by Seldaek at 2012-03-15T14:03:40Z
I still fail to see why it doesn't belong in core.. There are tons of little helpers here and there, a base controller class made only of proxies, and then this gets turned down because it is simple to do it yourself? Sure it is simple, but it's repetitive and boring too. And while it's simple when you know your way around, some people aren't really sure how to do it.
The whole point of a framework is to avoid repetitive bullshit and be more productive. @fabpot do you have any real arguments against? I can see that you don't see a big use to it, fair enough, but do you see any downside at all?
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
The HTTP status code translation table was updated to include all HTTP status codes as defined by the IANA Hypertext Transfer Protocol (HTTP) Status Code Registry (http://www.iana.org/assignments/http-status-codes/).
Commits
-------
4f8e8ef Improving performance on digit filtering
Discussion
----------
Improving performance on digit filtering
I haven't tested it on a productive system but I think it should be way faster to use filter_var() instead of preg_replace() for several reasons.
This is my first pull request for symfony and I don't know how you do those kind of performance tests but please verify my assumption if you can :-)
Maybe we can also use filter_var() to replace other regular expressions :-)
HTH =)
---------------------------------------------------------------------------
by drak at 2012-02-22T00:35:44Z
@Toflar - nice move +1
---------------------------------------------------------------------------
by drak at 2012-02-22T18:53:40Z
@Toflar - Maybe you can bench the changes using this as a template: https://gist.github.com/1356129
---------------------------------------------------------------------------
by Toflar at 2012-02-23T13:18:18Z
I have already. And it's way faster, otherwise I wouldn't have opened a pull request ;) But obviously it strongly depends on the length of the string and the environment. That's why I was wondering whether you have a general performance tests environment ;) Because the results strongly depend on other factors, there's - in my opinion - no point in exact results. If a general info is sufficient: my tests for the regex resulted in about 7 - 8 microseconds whereas the filter version only took 1.5 - 2 microseconds for the same string.
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
-------
bafcaaf Removed version field
f9d9dc7 Add branch-alias for composer
Discussion
----------
Add branch-alias for composer
This should restore the 2.1-dev version (as an alias of dev-master) so that `2.*` or `2.1.*` constraints work again. I'll adjust packagist soon to also display those aliases.
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.
Commits
-------
d077ede [HttpFoundation] Increase test coverage.
cbb3e69 [HttpFoundation] Increase test coverage.
Discussion
----------
[HttpFoundation] Increase session test coverage.
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
