Skip to content

Commit

Permalink
bug #19549 [HttpFoundation] fixed Request::getContent() reusage bug (…
Browse files Browse the repository at this point in the history
…1ma)

This PR was squashed before being merged into the 2.7 branch (closes #19549).

Discussion
----------

[HttpFoundation] fixed Request::getContent() reusage bug

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

After calling ```Request::getContent(true)```, subsequent calls to the
same instance method (withouth the ```$asResource``` flag) always returned
```false``` instead of the request body as a plain string.

A unit test already existed to guard against this behaviour (the 'Resource then fetch' case) but it
yielded a false positive because it was comparing ```''``` to ```false``` using
PHPUnit's ```assertEquals``` method instead of ```assertSame```.

For completeness sake I also added the missing usage permutations in
the data provider, which already worked OK.

Commits
-------

c42ac66 [HttpFoundation] fixed Request::getContent() reusage bug
  • Loading branch information
fabpot committed Aug 15, 2016
2 parents 1a059e5 + c42ac66 commit 2345ec1
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 4 deletions.
2 changes: 1 addition & 1 deletion src/Symfony/Component/HttpFoundation/Request.php
Expand Up @@ -1516,7 +1516,7 @@ public function getContent($asResource = false)
return stream_get_contents($this->content);
}

if (null === $this->content) {
if (null === $this->content || false === $this->content) {
$this->content = file_get_contents('php://input');
}

Expand Down
16 changes: 13 additions & 3 deletions src/Symfony/Component/HttpFoundation/Tests/RequestTest.php
Expand Up @@ -1042,8 +1042,16 @@ public function testGetContentCantBeCalledTwiceWithResources($first, $second)
$req->getContent($second);
}

public function getContentCantBeCalledTwiceWithResourcesProvider()
{
return array(
'Resource then fetch' => array(true, false),
'Resource then resource' => array(true, true),
);
}

/**
* @dataProvider getContentCantBeCalledTwiceWithResourcesProvider
* @dataProvider getContentCanBeCalledTwiceWithResourcesProvider
* @requires PHP 5.6
*/
public function testGetContentCanBeCalledTwiceWithResources($first, $second)
Expand All @@ -1060,12 +1068,14 @@ public function testGetContentCanBeCalledTwiceWithResources($first, $second)
$b = stream_get_contents($b);
}

$this->assertEquals($a, $b);
$this->assertSame($a, $b);
}

public function getContentCantBeCalledTwiceWithResourcesProvider()
public function getContentCanBeCalledTwiceWithResourcesProvider()
{
return array(
'Fetch then fetch' => array(false, false),
'Fetch then resource' => array(false, true),
'Resource then fetch' => array(true, false),
'Resource then resource' => array(true, true),
);
Expand Down

0 comments on commit 2345ec1

Please sign in to comment.