[HttpFoundation] Complete Request::overrideGlobals
[HttpFoundation] Fix the UploadedFilename name sanitization (fix #2577)
getClientIp() will now only return valid IP addresses, rather than assuming the X_FORWARDED_FOR is the first comma seperated value.
Since getClientIp() no longer takes a parameter, removed that old test

Added a new test for HTTP_X_FORWARDED_FOR that starts with 'unknown, '
failing test for HEAD StreamedResponse requests
[HttpFoundation] Make JsonResponse HTML safe.
[HttpFoundation] added some unit tests (refs #4510)
[HttpFoundation] Added 308 as a valid redirect code
Authorization header should only be rebuild when Basic Auth scheme is used
[HttpFoundation] removed unused variable
fixed an issue with session mocking in functional tests that do not start with a fresh session instance for each request (Silex for instance)
merged branch jalliot/patch-4 (PR #4544)

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

:+1:

---------------------------------------------------------------------------

by travisbot at 2012-06-10T06:27:18Z

This pull request [passes](http://travis-ci.org/symfony/symfony/builds/1579937) (merged 5d557261 into 6266b72d).
merged branch kriswallsmith/head-streamed-response-err (PR #4502)

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 ?
[HttpFoundation] fixed StreamedResponse with HEAD as a method (closes #4502)
merged 2.0
[HttpFoundation] CS and phpdoc fixes
merged branch adrienbrault/http-foundation-fixes (PR #4483)

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).
merged branch stealth35/populate_files (PR #2892)

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 :v:

---------------------------------------------------------------------------

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);
```
fixed CS
added Request::getSchemeAndHttpHost() and Request::getUserInfo() (closes #4312, refs #3416, refs #3056)