Skip to content

Commit f9b8417

Browse files
MatTheCatMatTheCat
committed
[HttpFoundation] Make Request::createFromGlobals() use request_parse_body when possible
1 parent 3d9e8f1 commit f9b8417

File tree

5 files changed

+182
-40
lines changed

5 files changed

+182
-40
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ CHANGELOG
99
* Add support for structured MIME suffix
1010
* Deprecate using `Request::sendHeaders()` after headers have already been sent; use a `StreamedResponse` instead
1111
* Deprecate method `Request::get()`, use properties `->attributes`, `query` or `request` directly instead
12+
* Make `Request::createFromGlobals()` parse the body of PUT, DELETE, PATCH and QUERY requests
1213

1314
7.3
1415
---

‎Request.php‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -280,9 +280,19 @@ public static function createFromGlobals(): static
280280
{
281281
$request = self::createRequestFromFactory($_GET, $_POST, [], $_COOKIE, $_FILES, $_SERVER);
282282

283-
if (str_starts_with($request->headers->get('CONTENT_TYPE', ''), 'application/x-www-form-urlencoded')
284-
&& \in_array(strtoupper($request->server->get('REQUEST_METHOD', 'GET')), ['PUT', 'DELETE', 'PATCH', 'QUERY'], true)
285-
) {
283+
if (!\in_array($request->server->get('REQUEST_METHOD', 'GET'), ['PUT', 'DELETE', 'PATCH', 'QUERY'], true)) {
284+
return $request;
285+
}
286+
287+
if (\PHP_VERSION_ID >= 80400) {
288+
try {
289+
[$post, $files] = request_parse_body();
290+
291+
$request->request->add($post);
292+
$request->files->add($files);
293+
} catch (\RequestParseBodyException) {
294+
}
295+
} elseif (str_starts_with($request->headers->get('CONTENT_TYPE', ''), 'application/x-www-form-urlencoded')) {
286296
parse_str($request->getContent(), $data);
287297
$request->request = new InputBag($data);
288298
}

‎Tests/Fixtures/request-functional/index.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
<?php
2+
3+
use Symfony\Component\HttpFoundation\File\UploadedFile;
4+
use Symfony\Component\HttpFoundation\JsonResponse;
5+
use Symfony\Component\HttpFoundation\Request;
6+
7+
$parent = __DIR__;
8+
while (!@file_exists($parent.'/vendor/autoload.php')) {
9+
if (!@file_exists($parent)) {
10+
// open_basedir restriction in effect
11+
break;
12+
}
13+
if ($parent === dirname($parent)) {
14+
echo "vendor/autoload.php not found\n";
15+
exit(1);
16+
}
17+
18+
$parent = dirname($parent);
19+
}
20+
21+
require $parent.'/vendor/autoload.php';
22+
23+
error_reporting(-1);
24+
ini_set('html_errors', 0);
25+
ini_set('display_errors', 1);
26+
27+
if (filter_var(ini_get('xdebug.default_enable'), \FILTER_VALIDATE_BOOL)) {
28+
xdebug_disable();
29+
}
30+
31+
$request = Request::createFromGlobals();
32+
33+
$r = new JsonResponse([
34+
'request' => $request->request->all(),
35+
'files' => array_map(
36+
static fn (UploadedFile $file) => [
37+
'clientOriginalName' => $file->getClientOriginalName(),
38+
'clientMimeType' => $file->getClientMimeType(),
39+
'content' => $file->getContent(),
40+
],
41+
$request->files->all()
42+
),
43+
]);
44+
45+
$r->send();

‎Tests/RequestFunctionalTest.php‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
<?php
2+
3+
/*
4+
* This file is part of the Symfony package.
5+
*
6+
* (c) Fabien Potencier <[email protected]>
7+
*
8+
* For the full copyright and license information, please view the LICENSE
9+
* file that was distributed with this source code.
10+
*/
11+
12+
namespace Symfony\Component\HttpFoundation\Tests;
13+
14+
use PHPUnit\Framework\Attributes\DataProvider;
15+
use PHPUnit\Framework\Attributes\RequiresPhp;
16+
use PHPUnit\Framework\TestCase;
17+
18+
#[RequiresPhp('>=8.4')]
19+
class RequestFunctionalTest extends TestCase
20+
{
21+
/** @var resource|false */
22+
private static $server;
23+
24+
public static function setUpBeforeClass(): void
25+
{
26+
$spec = [
27+
1 => ['file', '/dev/null', 'w'],
28+
2 => ['file', '/dev/null', 'w'],
29+
];
30+
if (!self::$server = @proc_open('exec '.\PHP_BINARY.' -S localhost:8054', $spec, $pipes, __DIR__.'/Fixtures/request-functional')) {
31+
self::markTestSkipped('PHP server unable to start.');
32+
}
33+
sleep(1);
34+
}
35+
36+
public static function tearDownAfterClass(): void
37+
{
38+
if (self::$server) {
39+
proc_terminate(self::$server);
40+
proc_close(self::$server);
41+
}
42+
}
43+
44+
public static function provideMethodsRequiringExplicitBodyParsing()
45+
{
46+
return [
47+
['PUT'],
48+
['DELETE'],
49+
['PATCH'],
50+
// PHP’s built-in server doesn’t support QUERY
51+
];
52+
}
53+
54+
#[DataProvider('provideMethodsRequiringExplicitBodyParsing')]
55+
public function testFormUrlEncodedBodyParsing(string $method)
56+
{
57+
$response = file_get_contents('http://localhost:8054/', false, stream_context_create([
58+
'http' => [
59+
'header' => 'Content-type: application/x-www-form-urlencoded',
60+
'method' => $method,
61+
'content' => http_build_query(['foo' => 'bar']),
62+
],
63+
]));
64+
65+
$this->assertSame(['foo' => 'bar'], json_decode($response, true)['request']);
66+
}
67+
68+
#[DataProvider('provideMethodsRequiringExplicitBodyParsing')]
69+
public function testMultipartFormDataBodyParsing(string $method)
70+
{
71+
$response = file_get_contents('http://localhost:8054/', false, stream_context_create([
72+
'http' => [
73+
'header' => 'Content-Type: multipart/form-data; boundary=boundary',
74+
'method' => $method,
75+
'content' => "--boundary\r\n".
76+
"Content-Disposition: form-data; name=foo\r\n".
77+
"\r\n".
78+
"bar\r\n".
79+
"--boundary\r\n".
80+
"Content-Disposition: form-data; name=baz; filename=baz.txt\r\n".
81+
"Content-Type: text/plain\r\n".
82+
"\r\n".
83+
"qux\r\n".
84+
'--boundary--',
85+
],
86+
]));
87+
88+
$data = json_decode($response, true);
89+
90+
$this->assertSame(['foo' => 'bar'], $data['request']);
91+
$this->assertSame(['baz' => [
92+
'clientOriginalName' => 'baz.txt',
93+
'clientMimeType' => 'text/plain',
94+
'content' => 'qux',
95+
]], $data['files']);
96+
}
97+
}

‎Tests/RequestTest.php‎

Lines changed: 26 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,15 @@
1414
use PHPUnit\Framework\Attributes\DataProvider;
1515
use PHPUnit\Framework\Attributes\Group;
1616
use PHPUnit\Framework\Attributes\IgnoreDeprecations;
17+
use PHPUnit\Framework\Attributes\RequiresPhp;
1718
use PHPUnit\Framework\Attributes\RequiresPhpExtension;
1819
use PHPUnit\Framework\Attributes\TestWith;
1920
use PHPUnit\Framework\TestCase;
2021
use Symfony\Component\HttpFoundation\Exception\BadRequestException;
2122
use Symfony\Component\HttpFoundation\Exception\ConflictingHeadersException;
2223
use Symfony\Component\HttpFoundation\Exception\JsonException;
2324
use Symfony\Component\HttpFoundation\Exception\SuspiciousOperationException;
24-
use Symfony\Component\HttpFoundation\InputBag;
2525
use Symfony\Component\HttpFoundation\IpUtils;
26-
use Symfony\Component\HttpFoundation\ParameterBag;
2726
use Symfony\Component\HttpFoundation\Request;
2827
use Symfony\Component\HttpFoundation\Session\Session;
2928
use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage;
@@ -1281,15 +1280,13 @@ public static function getContentCanBeCalledTwiceWithResourcesProvider()
12811280
];
12821281
}
12831282

1284-
public static function provideOverloadedMethods()
1283+
public static function provideMethodsRequiringExplicitBodyParsing()
12851284
{
12861285
return [
12871286
['PUT'],
12881287
['DELETE'],
12891288
['PATCH'],
1290-
['put'],
1291-
['delete'],
1292-
['patch'],
1289+
['QUERY'],
12931290
];
12941291
}
12951292

@@ -1331,50 +1328,42 @@ public function testGetPayload()
13311328
$this->assertSame([], $req->getPayload()->all());
13321329
}
13331330

1334-
#[DataProvider('provideOverloadedMethods')]
1335-
public function testCreateFromGlobals($method)
1331+
public function testCreateFromGlobals()
13361332
{
1337-
$normalizedMethod = strtoupper($method);
1338-
13391333
$_GET['foo1'] = 'bar1';
13401334
$_POST['foo2'] = 'bar2';
13411335
$_COOKIE['foo3'] = 'bar3';
13421336
$_FILES['foo4'] = ['bar4'];
13431337
$_SERVER['foo5'] = 'bar5';
13441338

13451339
$request = Request::createFromGlobals();
1346-
$this->assertEquals('bar1', $request->query->get('foo1'), '::fromGlobals() uses values from $_GET');
1347-
$this->assertEquals('bar2', $request->request->get('foo2'), '::fromGlobals() uses values from $_POST');
1348-
$this->assertEquals('bar3', $request->cookies->get('foo3'), '::fromGlobals() uses values from $_COOKIE');
1349-
$this->assertEquals(['bar4'], $request->files->get('foo4'), '::fromGlobals() uses values from $_FILES');
1350-
$this->assertEquals('bar5', $request->server->get('foo5'), '::fromGlobals() uses values from $_SERVER');
1351-
$this->assertInstanceOf(InputBag::class, $request->request);
1352-
$this->assertInstanceOf(ParameterBag::class, $request->request);
1340+
$this->assertEquals('bar1', $request->query->get('foo1'), '::createFromGlobals() uses values from $_GET');
1341+
$this->assertEquals('bar2', $request->request->get('foo2'), '::createFromGlobals() uses values from $_POST');
1342+
$this->assertEquals('bar3', $request->cookies->get('foo3'), '::createFromGlobals() uses values from $_COOKIE');
1343+
$this->assertEquals(['bar4'], $request->files->get('foo4'), '::createFromGlobals() uses values from $_FILES');
1344+
$this->assertEquals('bar5', $request->server->get('foo5'), '::createFromGlobals() uses values from $_SERVER');
1345+
}
1346+
1347+
public function testGetRealMethod()
1348+
{
1349+
Request::enableHttpMethodParameterOverride();
1350+
$request = new Request(request: ['_method' => 'PUT'], server: ['REQUEST_METHOD' => 'PoSt']);
13531351

1354-
unset($_GET['foo1'], $_POST['foo2'], $_COOKIE['foo3'], $_FILES['foo4'], $_SERVER['foo5']);
1352+
$this->assertEquals('POST', $request->getRealMethod(), '->getRealMethod() returns the uppercased request method, even if it has been overridden');
13551353

1354+
$this->disableHttpMethodParameterOverride();
1355+
}
1356+
1357+
#[RequiresPhp('< 8.4')]
1358+
#[DataProvider('provideMethodsRequiringExplicitBodyParsing')]
1359+
public function testFormUrlEncodedBodyParsing(string $method)
1360+
{
13561361
$_SERVER['REQUEST_METHOD'] = $method;
13571362
$_SERVER['CONTENT_TYPE'] = 'application/x-www-form-urlencoded';
1358-
$request = RequestContentProxy::createFromGlobals();
1359-
$this->assertEquals($normalizedMethod, $request->getMethod());
1360-
$this->assertEquals('mycontent', $request->request->get('content'));
1361-
$this->assertInstanceOf(InputBag::class, $request->request);
1362-
$this->assertInstanceOf(ParameterBag::class, $request->request);
13631363

1364-
unset($_SERVER['REQUEST_METHOD'], $_SERVER['CONTENT_TYPE']);
1365-
1366-
Request::createFromGlobals();
1367-
Request::enableHttpMethodParameterOverride();
1368-
$_POST['_method'] = $method;
1369-
$_POST['foo6'] = 'bar6';
1370-
$_SERVER['REQUEST_METHOD'] = 'PoSt';
1371-
$request = Request::createFromGlobals();
1372-
$this->assertEquals($normalizedMethod, $request->getMethod());
1373-
$this->assertEquals('POST', $request->getRealMethod());
1374-
$this->assertEquals('bar6', $request->request->get('foo6'));
1364+
$request = RequestContentProxy::createFromGlobals();
13751365

1376-
unset($_POST['_method'], $_POST['foo6'], $_SERVER['REQUEST_METHOD']);
1377-
$this->disableHttpMethodParameterOverride();
1366+
$this->assertEquals('mycontent', $request->request->get('content'));
13781367
}
13791368

13801369
public function testOverrideGlobals()
@@ -2675,7 +2664,7 @@ class RequestContentProxy extends Request
26752664
{
26762665
public function getContent($asResource = false)
26772666
{
2678-
return http_build_query(['_method' => 'PUT', 'content' => 'mycontent'], '', '&');
2667+
return http_build_query(['content' => 'mycontent'], '', '&');
26792668
}
26802669
}
26812670

0 commit comments

Comments
 (0)