Commits
-------
c40a4e5 [HttpFoundation] fix query string normalization
f9ec2ea refactored test method
0880174 [HttpFoundation] added failing tests for query string normalization
Discussion
----------
[HttpFoundation] fix query string normalization
This fixes the query string normalization. There were several problems in it (see test cases that I added).
The main issue, that first catched my eye, was that the query string was urldecoded before it was exploded by `=`. See old code: `explode('=', rawurldecode($segment), 2);`. This means an encoded `=` (`%3D`) would falsely be considered a separator and thus lead to complete different parameters. The fixed test case is at `pa%3Dram=foo%26bar%3Dbaz&test=test`.
---------------------------------------------------------------------------
by Tobion at 2012-07-04T02:21:25Z
cc @simensen considering your PR 4711
Commits
-------
d37003e [HttpFoundation] small fixes in Request
Discussion
----------
[HttpFoundation] small fixes in Request
phpdoc fixes,
making http_build_query explicit
fixing query string of '0', that was ignored.
Unfortunately this '0' problematic is omnipresent because PHP makes it so easy to get wrong (as it is converted to boolean false). I don't know how often I fixed such issue already.
Commits
-------
f72ba0a Fixed detection of an active session
Discussion
----------
[WIP][HttpFoundation][Session] Fixed detection of an active session
Bug fix: yes
Feature addition: no
Backwards compatibility break: not sure
Symfony2 tests pass: no
Fixes the following tickets: #4529
Todo: Fix failing tests
License of the code: MIT
Documentation PR: ~
This fixes the problem when the session variable inside $request now has always data in it as it's now more powerful but this introduces the problem that the old way of detecting if a session is started or not doesn't work anymore.
---------------------------------------------------------------------------
by travisbot at 2012-06-09T21:53:17Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1578839) (merged 9ae13e12 into 6266b72d).
---------------------------------------------------------------------------
by drak at 2012-06-10T01:57:59Z
Sessions should be started implicitly. The SF auto_start config parameter controls the session listener to start the session.
---------------------------------------------------------------------------
by dlsniper at 2012-06-11T06:46:02Z
So this patch is correct then and I should continue the work on it?
---------------------------------------------------------------------------
by drak at 2012-06-11T07:51:39Z
@dlsniper - no it's not correct. The session should not be auto-started like this, @fabpot and I recently discussed it.
---------------------------------------------------------------------------
by dlsniper at 2012-06-11T07:52:55Z
@Drak, ok I'll remove the patch for auto_start then but the fix for start would still stand, right?
---------------------------------------------------------------------------
by drak at 2012-06-12T18:40:35Z
@dlsniper - I have no objection to the rest of the PR except for the autostart stuff. I've annotated for clarity :)
---------------------------------------------------------------------------
by travisbot at 2012-06-12T19:51:12Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1604158) (merged 3499980e into 37550d23).
---------------------------------------------------------------------------
by travisbot at 2012-06-12T19:52:00Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1604166) (merged dcc73071 into 37550d23).
---------------------------------------------------------------------------
by dlsniper at 2012-06-12T19:56:51Z
Seems Travis doesn't like the squashing of commits that I've did but the PR does pass the normal tests.
@drak is this good for merging now?
Thanks :)
---------------------------------------------------------------------------
by dlsniper at 2012-06-13T09:05:09Z
@fabpot this can be merged safely, I've just applied the patch on my production application and the patch is ok, it's just travis failing.
Thanks
---------------------------------------------------------------------------
by travisbot at 2012-06-13T09:23:46Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1608735) (merged 1a6eabd2 into 37550d23).
---------------------------------------------------------------------------
by travisbot at 2012-06-13T09:28:26Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1608758) (merged 4e3a93c8 into 37550d23).
---------------------------------------------------------------------------
by dlsniper at 2012-06-13T09:29:28Z
I've noticed that this is failing, I'll fix it later on today.
---------------------------------------------------------------------------
by travisbot at 2012-06-13T15:14:01Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1611541) (merged 5504c4b7 into 37550d23).
---------------------------------------------------------------------------
by drak at 2012-06-13T15:23:47Z
It's possible that other tests are failing not related to this PR. Run the tests on the current master, and try rebasing your branch to the current master also.
---------------------------------------------------------------------------
by dlsniper at 2012-06-13T15:44:22Z
I've just reminded why this is failing on builds, I can't do them locally because of this:
```
Installing dev dependencies
Your requirements could not be solved to an installable set of packages.
Problems:
- Problem caused by:
- Installation request for doctrine/orm [>= 2.2.0.0, < 2.4.0.0-dev]: Satisfiable by [doctrine/orm-2.2.2, doctrine/orm-2.2.1, doctrine/orm-2.2.0, doctrine/orm-2.2.x-dev, doctrine/orm-2.3.x-dev].
```
I'll try and install this somehow and see what's wrong with it.
---------------------------------------------------------------------------
by mvrhov at 2012-06-13T18:08:58Z
@dlsniper: as @stof said to me this should be resolved in latest versions of composer, but it seems that is not. The problem is that composer cannot figure out that you are on dev-master if you try to instal dev. dependencies on feature branch. Take a look at the .travis.yml file on how to do a proper dev vendors install.
cc @Seldaek
---------------------------------------------------------------------------
by dlsniper at 2012-06-13T23:08:53Z
@mvrhov Thanks for pointing this out.
@drak I still got two tests not passing but I'm not sure how to fix them as adding $session->start() will either fail with the message that the session has already been started, the headers_sent() call which returns true. Any help with them will be greatly appreciated. Thanks!
Here is what the HttpKernel tests are returning:
```
There were 2 failures:
1) Symfony\Component\HttpKernel\Tests\EventListener\LocaleListenerTest::testDefaultLocaleWithSession
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'es'
+'fr'
/var/www/symfony-orig/src/Symfony/Component/HttpKernel/Tests/EventListener/LocaleListenerTest.php:51
2) Symfony\Component\HttpKernel\Tests\EventListener\LocaleListenerTest::testLocaleFromRequestAttribute
Expectation failed for method name is equal to <string:set> when invoked 1 time(s).
Method was expected to be called 1 times, actually called 0 times.
FAILURES!
Tests: 263, Assertions: 1025, Failures: 2, Skipped: 10.
```
---------------------------------------------------------------------------
by travisbot at 2012-06-13T23:42:59Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1614883) (merged 1004b7c0 into c07e9163).
---------------------------------------------------------------------------
by travisbot at 2012-06-13T23:53:06Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1614897) (merged f72ba0a2 into c07e9163).
---------------------------------------------------------------------------
by dlsniper at 2012-06-16T20:14:41Z
@stof / @vicb Hi, do either of you think that you can either point me out to the right direction for fixing this either ping someone else for home help as @drak doesn't seem available for this and at the moment I'm pretty much clueless in what direction I should take this fix.
Thanks!
---------------------------------------------------------------------------
by dlsniper at 2012-06-19T14:16:29Z
ping @fabpot Can you please provide some input on this one as I'm a bit stuck and seems noone else is available.
---------------------------------------------------------------------------
by drak at 2012-06-20T10:24:43Z
fyi - I'll be able to look again in a few days
---------------------------------------------------------------------------
by fabpot at 2012-07-01T07:53:28Z
I'm +1 to add the `isStarted()` method, but -1 for the change of `Request::hasSession`.
---------------------------------------------------------------------------
by drak at 2012-07-01T09:06:15Z
@fabpot, I agree. `hasSession()` should not be changed, it's semantically incorrect to make it return effectively "hasActiveSession".
Commits
-------
df8d94e added Request::getSchemeAndHttpHost() and Request::getUserInfo() (closes#4312, refs #3416, refs #3056)
Discussion
----------
added Request::getSchemeAndHttpHost() and Request::getUserInfo()
see #4312
---------------------------------------------------------------------------
by travisbot at 2012-06-28T15:22:03Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1730172) (merged 598bd56f into 0d275701).
---------------------------------------------------------------------------
by Seldaek at 2012-06-28T15:22:35Z
Why not just `getSchemeAndHost`? That sounds long enough, and is fairly explicit given the context.
---------------------------------------------------------------------------
by fabpot at 2012-06-28T15:25:34Z
@Seldaek because (and that's probably unfortunate) we have both `getHost()` and `getHttpHost()`. The former does not include the port whereas the latter does.
---------------------------------------------------------------------------
by Seldaek at 2012-06-28T15:26:42Z
Ok makes sense.
---------------------------------------------------------------------------
by travisbot at 2012-06-28T16:11:16Z
This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1730630) (merged df8d94e3 into 884a8264).
Commits
-------
b217897 [HttpFoundation] Complete Request::overrideGlobals
Discussion
----------
[2.2][HttpFoundation] complete Request::overrideGlobals
Bug fix: yes
Feature addition: yes
Backwards compatibility break: yes
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/stealth35/symfony.png?branch=populate_files)](http://travis-ci.org/stealth35/symfony)Fixes the following tickets: -
Todo: -
---------------------------------------------------------------------------
by stealth35 at 2011-12-15T14:20:36Z
Thank guys, should be better now
---------------------------------------------------------------------------
by stealth35 at 2011-12-15T16:14:40Z
@stloyd ✌️
---------------------------------------------------------------------------
by stloyd at 2011-12-15T16:22:48Z
@stealth35 You should update also [`RequestTest`](https://github.com/symfony/symfony/blob/master/tests/Symfony/Tests/Component/HttpFoundation/RequestTest.php#L623) which would show you typos you have few mins ago ;-)
---------------------------------------------------------------------------
by stealth35 at 2011-12-15T16:57:16Z
@stloyd done, thanks for your review
---------------------------------------------------------------------------
by canni at 2011-12-15T20:27:28Z
As this is bugfix, this shouldn't be re-based against 2.0?
---------------------------------------------------------------------------
by stealth35 at 2011-12-15T20:50:05Z
@canni It's more a forget feature, I tagged it to bug fix because of the `FIXME`, and it's add a method, IMO there is no rush
---------------------------------------------------------------------------
by canni at 2011-12-15T20:55:28Z
@stealth35 no rush at all, I was just curious :)
---------------------------------------------------------------------------
by vicb at 2012-01-06T16:24:31Z
I would say "Backwards compatibility break: yes" i.e.tests have been modified.
---------------------------------------------------------------------------
by stealth35 at 2012-01-06T16:36:15Z
@vicb the tests are not modified, just some addition
---------------------------------------------------------------------------
by vicb at 2012-01-06T16:40:30Z
@stealth35 https://github.com/symfony/symfony/pull/2892/files#L2R46
---------------------------------------------------------------------------
by stealth35 at 2012-01-06T17:13:07Z
@vicb it's not a compatibility break ...
---------------------------------------------------------------------------
by vicb at 2012-01-06T17:19:35Z
Well, same inputs, different outputs, this is a compatibility break to me.
But however it is named we should not change the behavior of this class; Client values are values as passed by the client you should no try to guess them.
---------------------------------------------------------------------------
by stealth35 at 2012-01-06T17:32:41Z
@vicb the behavior ? when you change the GET or POST values with `HttpFoundation\*Bag` (replace/set) it's the same thing
---------------------------------------------------------------------------
by vicb at 2012-01-06T17:35:39Z
I am referring to the difference in behavior between the current implementation and the version in this PR.
They do not behave the same and that's why the tests have been modified.
---------------------------------------------------------------------------
by fabpot at 2012-02-14T23:33:42Z
any progress on this PR?
---------------------------------------------------------------------------
by vicb at 2012-02-15T07:48:34Z
To make it clear I strongly disagree with the modifs in this PR. Available to help if needed.
---------------------------------------------------------------------------
by stealth35 at 2012-02-15T09:24:50Z
@fabpot Well, `move_uploaded_file` will not work so I have some doubt about this, @vicb just don't like the fact to add the mime type type and the size, it's not an important thing, I can remove it we can discuss later about that,
@vicb the last thing to do, it's to recreate the weird php $_FILES array
---------------------------------------------------------------------------
by vicb at 2012-02-23T17:11:29Z
@stealth35 I don't think we can bypass the `move_uploaded_file` security check - which is good. Is there any interest in this PR w/o this ?
If no we should just update phpDoc comment and remove the FIXME (meaning we can not override the `$_FILES`).
---------------------------------------------------------------------------
by stealth35 at 2012-03-10T16:13:14Z
@vicb updated
---------------------------------------------------------------------------
by vicb at 2012-03-11T09:38:20Z
@stealth35 what about adding some unit tests ?
---------------------------------------------------------------------------
by stealth35 at 2012-03-11T11:06:44Z
> what about adding some unit tests ?
@vicb `request_order` is PHP_INI_PERDIR, so I don't really how to handle this
---------------------------------------------------------------------------
by vicb at 2012-03-11T11:15:55Z
by creating a `protected getRequestOrder()` method or something like this ?
---------------------------------------------------------------------------
by stealth35 at 2012-03-11T11:36:11Z
it's too bad to create a method just for this, I can make cond in the test
``` php
<?php
$request->initialize(array('get' => 'foo'), array('post' => 'bar'));
$request->overrideGlobals();
$request_order = ini_get('request_order');
if ('gp' === $request_order) {
$this->assertEquals(array('get' => 'foo', 'post' => 'bar'), $_REQUEST);
} else if ('pg' === $request_order) {
$this->assertEquals(array('post' => 'bar', 'get' => 'foo'), $_REQUEST);
}
// ...
```
---------------------------------------------------------------------------
by vicb at 2012-03-11T12:02:17Z
This would only test one case.
Some thoughts about your snippet:
* The init should probably be `$request->initialize(array('foo' => 'get'), array('foo' => 'post'));`,
* `$request_order` does not take into account `variables_order.ini`,
* missing `strtolower`
---------------------------------------------------------------------------
by fabpot at 2012-03-23T21:21:59Z
What's the status of this PR? What needs to be done before merging?
---------------------------------------------------------------------------
by stealth35 at 2012-03-24T18:33:42Z
@fabpot missing some tests, it's not essay to tests an `ini`directive, @vicb recommand a `getRequestOrder` method, it's not a bad idea
---------------------------------------------------------------------------
by vicb at 2012-03-24T20:06:53Z
and change `$request_order` to `$requestOrder` as suggested by @henrikbjorn I can't find where
---------------------------------------------------------------------------
by stealth35 at 2012-06-14T12:42:25Z
I need help for testing
``` php
<?php
$request = $this->getMock('Request', array('overrideGlobals', 'initialize'));
$request->expects($this->any())
->method('getRequestOrder')
->will($this->returnValue('gp'));
$request->initialize(array('foo' => 'fooget'), array('foo' => 'foopost'));
$request->overrideGlobals();
$this->assertEquals(array_merge($_GET, $_POST), $_REQUEST);
```
Commits
-------
9a74b85 [HttpFoundation] CS and phpdoc fixes
Discussion
----------
[HttpFoundation] CS and phpdoc fixes
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
Hey
---------------------------------------------------------------------------
by travisbot at 2012-06-02T00:30:49Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1504379) (merged 2f0117f2 into 1541fe26).
---------------------------------------------------------------------------
by fabpot at 2012-06-25T14:53:18Z
@adrienbrault Can you have a look at my comments?
---------------------------------------------------------------------------
by adrienbrault at 2012-06-25T16:24:49Z
Done! Sorry for the delay
---------------------------------------------------------------------------
by travisbot at 2012-06-25T17:50:24Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1702850) (merged 9a74b851 into 58436de1).
Commits
-------
280fc05 failing test for HEAD StreamedResponse requests
Discussion
----------
[WIP] failing test for HEAD StreamedResponse requests
An exception is thrown if you prepare a StreamedResponse with a HEAD request. I'm not sure what the right fix is…
---------------------------------------------------------------------------
by kriswallsmith at 2012-06-06T15:51:04Z
The Travis build is here: http://travis-ci.org/#!/symfony/symfony/builds/1543352
---------------------------------------------------------------------------
by sstok at 2012-06-08T11:07:31Z
Well a HEAD can't/shouldn't be streamed as it doesn't contain a body so what is the real problem here?
---------------------------------------------------------------------------
by kriswallsmith at 2012-06-08T16:14:18Z
@sstok the response is prepared by the ResponseListener regardless of request method
---------------------------------------------------------------------------
by adrienbrault at 2012-06-08T19:41:27Z
Shouldn't the test at least assert something ?
Commits
-------
5d55726 [HttpFoundation] Added 308 as a valid redirect code
Discussion
----------
[HttpFoundation] Added 308 as a valid redirect code
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/jalliot/symfony.png?branch=patch-4)](http://travis-ci.org/jalliot/symfony)
Fixes the following tickets: -
Todo: -
I think this should go on 2.0 but error code 308 has only been added in master...
---------------------------------------------------------------------------
by lyrixx at 2012-06-09T22:56:32Z
👍
---------------------------------------------------------------------------
by travisbot at 2012-06-10T06:27:18Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1579937) (merged 5d557261 into 6266b72d).
Commits
-------
3c8cc0a [HttpFoundation][Sessions] Refactored tests
13a2c82 [FrameworkBundle] Refactor session file handler service name and update changelogs
b2cc580 [HttpFoundation] Removed Native*Handler session save handler classes
f33b77c [HttpFoundation] Added a custom file save handler
Discussion
----------
[HttpFoundation][Sessions] Removed native save handlers
Bug fix: no
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
Added a specific filesessionhandler
Removed native handlers to slim down code.
---------------------------------------------------------------------------
by travisbot at 2012-05-30T02:54:40Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1473181) (merged 3c8cc0a1 into adf07f1e).
Commits
-------
d046fed [HttpFoundation] Remove temporary files after tests run
Discussion
----------
[HttpFoundation] Remove temporary files after tests run
Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [yes|no]
Fixes the following tickets: -
Todo: -
License of the code: MIT
Documentation PR: -
---------------------------------------------------------------------------
by travisbot at 2012-05-28T00:26:30Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1451809) (merged 30082e97 into adf07f1e).
---------------------------------------------------------------------------
by travisbot at 2012-05-28T06:59:39Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1453569) (merged d046fede into adf07f1e).
Commits
-------
4fa8e68 Add support for javascript object notation in allowed JSONP callback
Discussion
----------
Add support for javascript object notation in allowed JSONP callback
---------------------------------------------------------------------------
by travisbot at 2012-05-18T23:09:45Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1371497) (merged 4fa8e68c into 18132c18).
Commits
-------
a450d00 [HttpFoundation] HTTP Basic authentication is broken with PHP as cgi/fastCGI under Apache
Discussion
----------
[HttpFoundation] HTTP Basic authentication is broken with php-cgi under Apache
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]
---------------------------------------------------------------------------
by stof at 2012-03-10T17:34:26Z
you should also add a unit test for this
---------------------------------------------------------------------------
by kepten at 2012-03-11T15:34:04Z
Thanks for the feedback, I committed the changes.
---------------------------------------------------------------------------
by stof at 2012-04-04T01:59:53Z
@fabpot could you review it ?
---------------------------------------------------------------------------
by fabpot at 2012-04-04T07:15:34Z
My comments:
* `ServerBag` represents what we have in the `$_SERVER` global variables. As such, the code should be moved to the `getHeaders()` method instead like the other tweaks we do for the HTTP headers.
* A comment must be added explaining why this is needed and the configuration the user must have to make it work (then remove the Github URLs).
* The code should only be executed when `PHP_AUTH_USER` is not available (to not have any overhead when not needed).
---------------------------------------------------------------------------
by danielholmes at 2012-04-14T13:27:09Z
A quick note on that .htaccess/apache configuration required, if adding to the Symfony SE htaccess file, then it will need to look like this:
```
<IfModule mod_rewrite.c>
RewriteEngine On
RewriteRule .* - [E=HTTP_AUTHORIZATION:%{HTTP:Authorization}]
RewriteCond %{REQUEST_FILENAME} !-f
RewriteRule ^(.*)$ app.php [QSA,L]
</IfModule>
```
NOTE: No **,L** in the Authorization Rewrite as in the original example - it prevents the front controller rewrite from happening
---------------------------------------------------------------------------
by towards at 2012-04-20T16:12:49Z
@kepten you were faster than me applying @fabpot's comments :) nevertheless part of the bug hunt day I also modified the ServerBag class and tested them on a productive LAMP hosting server using Apache and FastCGI
---------------------------------------------------------------------------
by kepten at 2012-04-20T16:15:57Z
ok, so is my PR is useless or should I still fix problems?
---------------------------------------------------------------------------
by towards at 2012-04-20T16:20:26Z
your PR is fine for sure and I don't want to interfere, just wanted to mention that part of the bug hunt day of Symfony I had a go at this PR as an "exercise" but just saw later on that you already fixed the problem, so you can ignore my pushes
---------------------------------------------------------------------------
by vicb at 2012-04-20T16:20:36Z
I have been working with @towards: your PR is useful, please implement his comments and squash your PR.
---------------------------------------------------------------------------
by kepten at 2012-04-20T16:59:07Z
never squashed before, is it okay now? :)
---------------------------------------------------------------------------
by stof at 2012-04-20T17:21:07Z
it is
---------------------------------------------------------------------------
by vicb at 2012-05-20T19:57:51Z
@fabpot this should be ready to be merged
Commits
-------
d1c831d Change must-proxy-revalidate by proxy-revalidate
Discussion
----------
Change must-proxy-revalidate by proxy-revalidate
---------------------------------------------------------------------------
by travisbot at 2012-05-16T09:20:54Z
This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1344060) (merged d1c831d7 into 8cd6cbcf).
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.