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: -
Rename ArraySessionStorage to make it clear the session is a mock for testing purposes only.
Has BC class for ArraySessionStorage
Added sanity check when starting the session.
Fixed typos and incorrect php extension test method
session_module_name() also sets session.save_handler, so must use extension_loaded() to check if module exist
or not.
Respect autostart settings.
Session object now implements SessionInterface to make it more portable.
AbstractSessionStorage and SessionSaveHandlerInterface now makes implementation
of session storage drivers simple and easy to write for both custom save handlers
and native php save handlers and respect the PHP session workflow.
This commit outsources the flash message processing to it's own interface.
Overall flash messages now can have multiple flash types and each type can
store multiple messages. For convenience there are now four flash types
by default, INFO, NOTICE, WARNING and ERROR.
There are two concrete implementations: one preserving the old behaviour of
flash messages expiring exactly after one page load, regardless of being
displayed or not; and the other where flash messages persist until explicitly
popped.
This commit outsources session attribute storage to it's own class.
There are two concrete implementations, one with structured namespace storage and the other
without.
Apache expects the response to already be in chunked format in that case,
which causes it to not deliver the streamed body.
If no Content-Length is set on the response, web servers will automatically
switch to chunked Transfer-Encoding, and handle the chunking for you.
Nginx does not share the issue that apache has, but will add the Content-
Length header too.
Commits
-------
887c0e9 moved EngineInterface::stream() to a new StreamingEngineInterface to keep BC with 2.0
473741b added the possibility to change a StreamedResponse callback after its creation
8717d44 moved a test in the constructor
e44b8ba made some cosmetic changes
0038d1b [HttpFoundation] added support for streamed responses
Discussion
----------
[HttpFoundation] added support for streamed responses
To stream a Response, use the StreamedResponse class instead of the
standard Response class:
$response = new StreamedResponse(function () {
echo 'FOO';
});
$response = new StreamedResponse(function () {
echo 'FOO';
}, 200, array('Content-Type' => 'text/plain'));
As you can see, a StreamedResponse instance takes a PHP callback instead of
a string for the Response content. It's up to the developer to stream the
response content from the callback with standard PHP functions like echo.
You can also use flush() if needed.
From a controller, do something like this:
$twig = $this->get('templating');
return new StreamedResponse(function () use ($templating) {
$templating->stream('BlogBundle:Annot:streamed.html.twig');
}, 200, array('Content-Type' => 'text/html'));
If you are using the base controller, you can use the stream() method instead:
return $this->stream('BlogBundle:Annot:streamed.html.twig');
You can stream an existing file by using the PHP built-in readfile() function:
new StreamedResponse(function () use ($file) {
readfile($file);
}, 200, array('Content-Type' => 'image/png');
Read http://php.net/flush for more information about output buffering in PHP.
Note that you should do your best to move all expensive operations to
be "activated/evaluated/called" during template evaluation.
Templates
---------
If you are using Twig as a template engine, everything should work as
usual, even if are using template inheritance!
However, note that streaming is not supported for PHP templates. Support
is impossible by design (as the layout is rendered after the main content).
Exceptions
----------
Exceptions thrown during rendering will be rendered as usual except that
some content might have been rendered already.
Limitations
-----------
As the getContent() method always returns false for streamed Responses, some
event listeners won't work at all:
* Web debug toolbar is not available for such Responses (but the profiler works fine);
* ESI is not supported.
Also note that streamed responses cannot benefit from HTTP caching for obvious
reasons.
---------------------------------------------------------------------------
by Seldaek at 2011/12/21 06:34:13 -0800
Just an idea: what about exposing flush() to twig? Possibly in a way that it will not call it if the template is not streaming. That way you could always add a flush() after your </head> tag to make sure that goes out as fast as possible, but it wouldn't mess with non-streamed responses. Although it appears flush() doesn't affect output buffers, so I guess it doesn't need anything special.
When you say "ESI is not supported.", that means only the AppCache right? I don't see why this would affect Varnish, but then again as far as I know Varnish will buffer if ESI is used so the benefit of streaming there is non-existent.
---------------------------------------------------------------------------
by cordoval at 2011/12/21 08:04:21 -0800
wonder what the use case is for streaming a response, very interesting.
---------------------------------------------------------------------------
by johnkary at 2011/12/21 08:19:48 -0800
@cordoval Common use cases are present fairly well by this RailsCast video: http://railscasts.com/episodes/266-http-streaming
Essentially it allows faster fetching of web assets (JS, CSS, etc) located in the <head></head>, allowing those assets to be fetched as soon as possible before the remainder of the content body is computed and sent to the browser. The end goal is to improve page load speed.
There are other uses cases too like making large body content available quickly to the service consuming it. Think if you were monitoring a live feed of JSON data of newest Twitter comments.
---------------------------------------------------------------------------
by lsmith77 at 2011/12/21 08:54:35 -0800
How does this relate the limitations mentioned in:
http://yehudakatz.com/2010/09/07/automatic-flushing-the-rails-3-1-plan/
Am I right to understand that due to how twig works we are not really streaming the content pieces when we call render(), but instead the entire template with its layout is rendered and only then will we flush? or does it mean that the render call will work its way to the top level layout template and form then on it can send the content until it hits another block, which it then first renders before it continues to send the data?
---------------------------------------------------------------------------
by stof at 2011/12/21 09:02:53 -0800
@lsmith77 this is why the ``stream`` method calls ``display`` in Twig instead of ``render``. ``display`` uses echo to print the output of the template line by line (and blocks are simply method calls in the middle). Look at your compiled templates to see it (the ``doDisplay`` method)
Rendering a template with Twig simply use an output buffer around the rendering.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:24:33 -0800
@lsmith77: We don't have the Rails problem thanks to Twig as the order of execution is the right one by default (the layout is executed first); it means that we can have the flush feature without any change to how the core works. As @stof mentioned, we are using `display`, not `render`, so we are streaming your templates for byte one.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:36:41 -0800
@Seldaek: yes, I meant ESI with the PHP reverse proxy.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:37:34 -0800
@Seldaek: I have `flush()` support for Twig on my todo-list. As you mentioned, It should be trivial to implement.
---------------------------------------------------------------------------
by fzaninotto at 2011/12/21 09:48:18 -0800
How do streaming responses deal with assets that must be called in the head, but are declared in the body?
---------------------------------------------------------------------------
by fabpot at 2011/12/21 09:52:12 -0800
@fzaninotto: What do you mean?
With Twig, your layout is defined with blocks ("holes"). These blocks are overridden by child templates, but evaluated as they are encountered in the layout. So, everything works as expected.
As noted in the commit message, this does not work with PHP templates for the problems mentioned in the Rails post (as the order of execution is not the right one -- the child template is first evaluated and then the layout).
---------------------------------------------------------------------------
by fzaninotto at 2011/12/21 10:07:35 -0800
I was referring to using Assetic. Not sure if this compiles to Twig the same way as javascript and stylesheet blocks placed in the head - and therefore executed in the right way.
---------------------------------------------------------------------------
by fabpot at 2011/12/21 10:34:59 -0800
@Seldaek: I've just added a `flush` tag in Twig 1.5: 1d6dfad4f5
---------------------------------------------------------------------------
by catchamonkey at 2011/12/21 13:29:22 -0800
I'm really happy you've got this into the core, it's a great feature to have! Good work.
To stream a Response, use the StreamedResponse class instead of the
standard Response class:
$response = new StreamedResponse(function () {
echo 'FOO';
});
$response = new StreamedResponse(function () {
echo 'FOO';
}, 200, array('Content-Type' => 'text/plain'));
As you can see, a StreamedResponse instance takes a PHP callback instead of
a string for the Response content. It's up to the developer to stream the
response content from the callback with standard PHP functions like echo.
You can also use flush() if needed.
From a controller, do something like this:
$twig = $this->get('templating');
return new StreamedResponse(function () use ($templating) {
$templating->stream('BlogBundle:Annot:streamed.html.twig');
}, 200, array('Content-Type' => 'text/html'));
If you are using the base controller, you can use the stream() method instead:
return $this->stream('BlogBundle:Annot:streamed.html.twig');
You can stream an existing file by using the PHP built-in readfile() function:
new StreamedResponse(function () use ($file) {
readfile($file);
}, 200, array('Content-Type' => 'image/png');
Read http://php.net/flush for more information about output buffering in PHP.
Note that you should do your best to move all expensive operations to
be "activated/evaluated/called" during template evaluation.
Templates
---------
If you are using Twig as a template engine, everything should work as
usual, even if are using template inheritance!
However, note that streaming is not supported for PHP templates. Support
is impossible by design (as the layout is rendered after the main content).
Exceptions
----------
Exceptions thrown during rendering will be rendered as usual except that
some content might have been rendered already.
Limitations
-----------
As the getContent() method always returns false for streamed Responses, some
event listeners won't work at all:
* Web debug toolbar is not available for such Responses (but the profiler works fine);
* ESI is not supported.
Also note that streamed responses cannot benefit from HTTP caching for obvious
reasons.
Commits
-------
1e370d7 typo fix
93d8d44 added some more infos about Config
27efd59 added READMEs for the bridges
34fc866 cosmetic tweaks
d6af3f1 fixed README for Console
6a72b8c added basic README files for all components
Discussion
----------
added basic README files for all components and bridges
heavily based on http://fabien.potencier.org/article/49/what-is-symfony2 and the official Symfony2 documentation
---------------------------------------------------------------------------
by jmikola at 2011/11/03 13:36:07 -0700
Great work. For syntax highlighting on the PHP snippets, you could add "php" after the three backticks.
---------------------------------------------------------------------------
by lsmith77 at 2011/11/03 13:41:29 -0700
done
---------------------------------------------------------------------------
by stealth35 at 2011/11/03 13:49:31 -0700
Nice job, but you also need to add `<?php`
ex :
``` php
<?php
use Symfony\Component\DomCrawler\Crawler;
$crawler = new Crawler();
$crawler->addContent('<html><body><p>Hello World!</p></body></html>');
print $crawler->filter('body > p')->text();
```
---------------------------------------------------------------------------
by lsmith77 at 2011/11/03 13:56:57 -0700
done
---------------------------------------------------------------------------
by ericclemmons at 2011/11/03 19:57:57 -0700
@lsmith77 Well done! This makes consumption of individual components that much easier, *especially* now that `composer.json` files have been added.
---------------------------------------------------------------------------
by lsmith77 at 2011/11/04 01:18:23 -0700
ok .. fixed the issues you mentioned @fabpot
---------------------------------------------------------------------------
by lsmith77 at 2011/11/11 15:00:27 -0800
@fabpot anything else left? seems like an easy merge .. and imho there is considerable benefit for our efforts to spread the word about the components with this PR merged.
---------------------------------------------------------------------------
by drak at 2011/11/11 18:54:13 -0800
You know, it might be a nice idea to put a link to the documentation for each component if there is some at symfony.com
---------------------------------------------------------------------------
by lsmith77 at 2011/11/12 00:59:14 -0800
i did that in some. but i might have missed a few places.
On 12.11.2011, at 03:54, Drak <reply@reply.github.com> wrote:
> You know, it might be a nice idea to put a link to the documentation for each component if there is some at symfony.com
>
> ---
> Reply to this email directly or view it on GitHub:
> https://github.com/symfony/symfony/pull/2561#issuecomment-2715762
---------------------------------------------------------------------------
by breerly at 2011/11/21 10:28:36 -0800
Pretty excited with this.
---------------------------------------------------------------------------
by dbu at 2011/11/24 00:02:50 -0800
is there anything we can help with to make this ready to be merged?
---------------------------------------------------------------------------
by lsmith77 at 2011/12/18 02:39:23 -0800
@fabpot: seriously .. if you are not going to deliver something "better" and don't provide a reason what is wrong with this .. then its beyond frustrating. i obviously do not claim that these README's are perfect (and certainly still no replacement for proper documentation), but I do claim that in their current form they are a radical step forward to potential users of the Symfony2 components.
Commits
-------
cd24fb8 change explode's limit parameter based on known variable content
b3cc270 minor optimalisations for explode
Discussion
----------
[FrameworkBundle][CssSelector][HttpFoundation][HttpKernel] [Security][Validator] Minor optimizations for "explode" function
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
I added limit parameter in some places, where it may be usefull. I did not check the context of what values may have been exploded. So to not break anything, I added +1 to limit parameter.
If you find out that in some places limit (or limit+1) is not important or meaningless, write a comment please and I will fix it.
---------------------------------------------------------------------------
by fabpot at 2011/12/07 06:56:49 -0800
Adding +1 just to be sure to not break anything is clearly something we won't do. What is the benefit of doing that anyway?
---------------------------------------------------------------------------
by pulzarraider at 2011/12/07 13:50:24 -0800
The main idea of making this PR was to notify about some places that may run faster with just adding one parameter to explode function.
If in code is someting like: ```list($a, $b) = explode(':', $s);```
Function ```explode``` will create n-items (depends on ```$s```), but we need in code only the first two items. There is no reason to let ```explode``` create more items in memory that are NEVER used in our code. The limit parameter is there for these situations, so let's use it.
I know that it is microoptimization and may look unimportant, but we are writing a framework - so people expect that code will be as fast as possible without this kind of mistakes.
As I've noticed above, I know that +1 is not ideal solution, but the fastest without debugging the code. I expect that someone (with good knowledge of that code) will look at it and write in comments if variable may contain 1 comma (dot or someting on what is doing the explode) or maybe 2 in some situations or more.
Anyway, +1 will not break anything, because same items are created as it is now, but no unnecessary item is created.
---------------------------------------------------------------------------
by fabpot at 2011/12/07 23:14:59 -0800
I'm +1 for adding the number to avoid problems but I'm -1 on the optimization side of things as it won't optimize anything.
---------------------------------------------------------------------------
by helmer at 2011/12/08 12:46:49 -0800
*.. The main idea of making this PR was to notify about some places that **may** run faster ..*
I am also unsure the optimization is really an optimization, care to benchmark (with meaningful inputs)? As for the limit+1 thing, why would you want to +1 it? The number of ``list`` arguments should always reflect the ``limit`` parameter, no?
---------------------------------------------------------------------------
by pulzarraider at 2011/12/08 23:11:34 -0800
@helmer please try this simple benchmark:
```
<?php
header('Content-Type: text/plain; charset=UTF-8');
define('COUNT', 10000);
$source_string = 'aaaaaaaaaaaaaaaaaaaa:bbbbbbbbbbbbbbbbbbbbb:cccccccccccccccccccccccc:dddddddddddddddddddddd:eeeeeeeeeeeeeeeeeeeeeeeee:fffffffffffffffffffffffffff';
$start = microtime(true);
for ($i = 0; $i < COUNT; $i++) {
list($a, $b) = explode(':', $source_string);
}
$end = microtime(true)-$start;
echo 'without limit: '.$end."\n";
$start = microtime(true);
for ($i = 0; $i < COUNT; $i++) {
list($a, $b) = explode(':', $source_string, 2);
}
$end = microtime(true)-$start;
echo 'with limit: '.$end."\n";
```
My results are:
```
without limit: 0.057228803634644
with limit: 0.028676986694336
```
That is 50% difference (with APC enabled). Of course the result depends on the length of source string and if it's too short, the difference may be none or very very small. That's why I said, that it **may** run faster and is just a micro optimization.
---------------------------------------------------------------------------
by pulzarraider at 2011/12/08 23:18:12 -0800
@helmer And why +1? It depends on a code:
```
$source_string = 'aaaaaaaaaaaaaaaaaaaa:bbbbbbbbbbbbbbbbbbbbb:cccccccccccccccccccccccc';
list($a, $b) = explode(':', $source_string, 2);
var_dump($a, $b);
```
and
```
$source_string = 'aaaaaaaaaaaaaaaaaaaa:bbbbbbbbbbbbbbbbbbbbb:cccccccccccccccccccccccc';
list($a, $b) = explode(':', $source_string, 3);
var_dump($a, $b);
```
gives different results. That's why the content of the variable must be known.
---------------------------------------------------------------------------
by helmer at 2011/12/09 00:08:28 -0800
@pulzarraider Thanks for the benchmark, seems like a gain enough. Although, we are more likely having a scenario of:
``explode(':', 'a🅱️c')`` vs ``explode(':', 'a🅱️c', 3)`` with a ``COUNT`` of 10, where the difference is not even in microseconds anymore :)
The limit addition alters the behaviour though, ie suddenly you can define a controller [logical name](http://symfony.com/doc/current/book/routing.html#controller-string-syntax) as ´´AcmeBlogBundle:Blog:show:something``, and things go downhill from there on.
All that aside, I'm +1 for setting the limit to the exact number of ``list`` parameters, but certainly not number+1, this is just too wtfy (as you said, this was a safety thing, but I reckon for this PR to be merged it needs to be +0).
---------------------------------------------------------------------------
by drak at 2011/12/09 08:28:58 -0800
Overall `list()` is ugly as it's not very explicit. Even though it would mean extra lines, it's better to `explode()` then explicitly assign variables:
```
$parts = explode(':', $foo);
$name = $parts[0];
$tel = $parts[1];
```
`list()` is one of those bad relics from the PHP past...
---------------------------------------------------------------------------
by fabpot at 2011/12/11 10:07:47 -0800
@drak: why is `list` not explicit? It is in fact as explicit as the more verbose syntax you propose.
---------------------------------------------------------------------------
by pulzarraider at 2011/12/11 13:08:50 -0800
@drak: I agree with @fabpot. In speech of benchmarks ```list``` is faster then using a helper variable.
@fabpot, @helmer I've changed explode's limit to be correct (without +1) and removed some changes from this PR, where I can't find out what the content of variable may be. Unit tests pass, so I think it's ready for merge.
Commits
-------
11b6156 updated unittest
a931e21 get correct client IP from X-forwarded-for header
Discussion
----------
[HttpFoundation] Get correct client IP when using trusted proxy (Varnish)
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
Note: This is reopened PR #2686 for 2.0 branch.
If using trusted proxy (Varnish, ...) the client IP must be identified from X-Forwarded-For header. The header has de-facto standard format:
X-Forwarded-For : client1, proxy1, proxy2,
where the value is a comma+space separated list of IP addresses, the left-most being the farthest downstream client, and each successive proxy that passed the request adding the IP address where it received the request from. See: http://en.wikipedia.org/wiki/X-Forwarded-For
Function getClientIp should return only one client IP, not a list of all nonimportant IPs as it's now. Similar example can be seen in Cake framework: http://api.cakephp.org/view_source/request-handler-component/#line-477
There are many ways how to chose the first IP from X-Forwarded-For header. Any other faster and more reliable way is welcome.
Commits
-------
b6bf018 tweaked error handling for the forward compatibility
dd606b5 added note about the purpose of this class
c1426ba added locale handling forward compatibility
10eed30 added MessageDataCollector forward compatibility
Discussion
----------
Forward compat
Bug fix: no
Feature addition: yes
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: #2522
* 2.0:
[HttpKernel] fixed Content-Length header when using ESI tags (closes#2623)
[HttpFoundation] added an exception to MimeTypeGuesser::guess() when no guesser are available (closes#2636)
[Security] fixed HttpUtils::checkRequestPath() to not catch all exceptions (closes#2637)
[DoctrineBundle] added missing default parameters, needed to setup and use DBAL without ORM
[Transation] Fix grammar.
[TwigBundle] Fix trace to not show 'in at line' when file/line are empty.
Commits
-------
269a5e6 Added the ablity to get a requests ContentType
Discussion
----------
Added getContentType
I've added the ability for Symfony\Component\HttpFoundation\Request to return the ContentType from serverBag this uses the $formats array to determine if the requested ContentType is valid.
---------------------------------------------------------------------------
by ericclemmons at 2011/11/03 20:00:51 -0700
Have you considered squashing a couple of your commits? They seem doubled up.
Trivial, I know, but it will make each commit stand on its own (instead of appearing as a typo correction)
---------------------------------------------------------------------------
by thomasbibb at 2011/11/04 02:02:36 -0700
done.
---------------------------------------------------------------------------
by ericclemmons at 2011/11/04 07:25:20 -0700
You may need to do a `git push -f origin master`. Check the commits tab to see the duplicate history:
> https://github.com/symfony/symfony/pull/2559/commits
Wheeeee, rebasing is fun!
---------------------------------------------------------------------------
by thomasbibb at 2011/11/04 12:26:06 -0700
There we got thats better :)
---------------------------------------------------------------------------
by ericclemmons at 2011/11/04 12:55:07 -0700
👍 Now let's see if it gets approved by @fabpot :)
---------------------------------------------------------------------------
by thomasbibb at 2011/11/06 03:39:12 -0800
I've removed the space between the method name and the parenthesis.
---------------------------------------------------------------------------
by thomasbibb at 2011/11/06 04:05:15 -0800
done.
---------------------------------------------------------------------------
by fabpot at 2011/11/06 23:44:22 -0800
Can you added some unit tests?
Commits
-------
09e1e60 Grammar changes.
b4d0f4b Grammar changes
a0e62f0 Fix typos.
e6627fb Docblocks.
Discussion
----------
[HttpFoundation] Docblocks.
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: n/a
Fixes the following tickets: -
Added missing docblocks and type hints. Improves use with IDE and auto-generated API docs.
Commits
-------
d3f137b cosmetic tweak
2877883 anything in front of ;q= is part of the mime type, anything after may be ignored
Discussion
----------
[HttpFoundation] fix splitHttpAcceptHeader() parsing of parameters
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
anything in front of ;q= is part of the mime type, anything after may be ignored
see http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.1
---------------------------------------------------------------------------
by lsmith77 at 2011/10/09 04:00:12 -0700
i must admit .. i am not 100% that my implemention is correct either .. but i am sure the current one isn't.
---------------------------------------------------------------------------
by lsmith77 at 2011/10/09 07:57:33 -0700
@fabpot: I am also not sure if getFormat() should optionally not support matching parameters, aka anything before ``;q=..``
Commits
-------
edfa29b session data needs to be encoded because it can contain non binary safe characters e.g null. Fixes#2067
Discussion
----------
session data needs to be encoded because it can contain non binary safe characters e.g null.
Bug fix: yes
Feature addition: no
Backwards compatibility break: yes
Symfony2 tests pass: yes
Fixes the following tickets: #2067
I'm marking this as a compatibility break because session table should be cleared and even if not cleared all currently logged in users will be logged out.
---------------------------------------------------------------------------
by mvrhov at 2011/10/11 12:52:25 -0700
P.S. I know there was a talk about doctrine based session storage but I cannot find this in core. It probably has the same problem.
---------------------------------------------------------------------------
by eventhorizonpl at 2011/10/11 14:34:08 -0700
Thanks for tracking down and fixing this issue!
Best regards,
Michal
---------------------------------------------------------------------------
by stof at 2011/10/11 16:24:18 -0700
@mvrhov The Doctrine based storage is only available in master, not in 2.0
That allows projects that only use HttpFoundation and not HttpKernel to be able to
enforce the HTTP specification "rules".
$request = Request::createFromGlobals();
$response = new Response();
// do whatever you want with the Respons
// enforce HTTP spec
$response->prepare($request);
$response->send();
Within Symfony2, the prepare method is automatically called by the ResponseListener.