From 006e4a7c9218e5cf2863ca68b4ef819935ba7a3e Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Mon, 5 Jul 2021 16:25:58 -0600 Subject: [PATCH 1/5] Manually populate POST request body with JSON data only when required This fixes a 6 year old bug introduced in #7026 where GET requests would have GET data populated in the POST body property leading to issues around Request::post() and $request->getParsedBody() returning GET values when called on GET requests. This is a resubmit of #17087 & #36708, and fixes #22805. Credit to @dan-har for the initial solution and @mixlion for updating it for >=6.x. The original PR was meant to support POST requests where their Content-type was set to application/json (instead of the typical application/x-www-form-urlencoded), but it introduced a subtle and dangerous bug because while $request->getInputSource() does return the JSON data for JSON requests, it also returns the GET data for GET requests. This commit solves the underlying issue without breaking compatibility with the original functionality. --- src/Illuminate/Http/Request.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Http/Request.php b/src/Illuminate/Http/Request.php index 06f143c6020f..b0f59fe7d175 100644 --- a/src/Illuminate/Http/Request.php +++ b/src/Illuminate/Http/Request.php @@ -432,7 +432,9 @@ public static function createFromBase(SymfonyRequest $request) $newRequest->content = $request->content; - $newRequest->request = $newRequest->getInputSource(); + if ($newRequest->isJson()) { + $newRequest->request = $newRequest->json(); + } return $newRequest; } From 5da220acf36dab1e1cac293de45cb2d7e4358c43 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 6 Jul 2021 12:04:14 -0600 Subject: [PATCH 2/5] Add test for non-JSON GET requests --- tests/Http/HttpRequestTest.php | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tests/Http/HttpRequestTest.php b/tests/Http/HttpRequestTest.php index c019499419f6..6685b7fb00b8 100644 --- a/tests/Http/HttpRequestTest.php +++ b/tests/Http/HttpRequestTest.php @@ -993,7 +993,12 @@ public function testFingerprintWithoutRoute() $request->fingerprint(); } - public function testCreateFromBase() + /** + * Ensure JSON GET requests populate $request->request with the JSON content + * + * @link https://github.com/laravel/framework/pull/7052 Correctly fill the $request->request parameter bag on creation + */ + public function testJsonRequestFillsRequestBodyParams() { $body = [ 'foo' => 'bar', @@ -1010,6 +1015,24 @@ public function testCreateFromBase() $this->assertEquals($request->request->all(), $body); } + + /** + * Ensure non-JSON GET requests don't pollute $request->request with the GET parameters + * + * @link https://github.com/laravel/framework/pull/37921 Manually populate POST request body with JSON data only when required + */ + public function testNonJsonRequestDoesntFillRequestBodyParams() + { + $params = ['foo' => 'bar']; + + $getRequest = Request::create('/', 'GET', $params, [], [], []); + $this->assertEquals($getRequest->request->all(), []); + $this->assertEquals($getRequest->query->all(), $params); + + $postRequest = Request::create('/', 'POST', $params, [], [], []); + $this->assertEquals($postRequest->request->all(), $params); + $this->assertEquals($postRequest->query->all(), []); + } /** * Tests for Http\Request magic methods `__get()` and `__isset()`. From 564b523f48b5b884ef5299b799a2252bdbb35862 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 6 Jul 2021 12:05:48 -0600 Subject: [PATCH 3/5] Style fixes --- tests/Http/HttpRequestTest.php | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/Http/HttpRequestTest.php b/tests/Http/HttpRequestTest.php index 6685b7fb00b8..0f474df18fcf 100644 --- a/tests/Http/HttpRequestTest.php +++ b/tests/Http/HttpRequestTest.php @@ -994,9 +994,9 @@ public function testFingerprintWithoutRoute() } /** - * Ensure JSON GET requests populate $request->request with the JSON content + * Ensure JSON GET requests populate $request->request with the JSON content. * - * @link https://github.com/laravel/framework/pull/7052 Correctly fill the $request->request parameter bag on creation + * @link https://github.com/laravel/framework/pull/7052 Correctly fill the $request->request parameter bag on creation. */ public function testJsonRequestFillsRequestBodyParams() { @@ -1017,18 +1017,18 @@ public function testJsonRequestFillsRequestBodyParams() } /** - * Ensure non-JSON GET requests don't pollute $request->request with the GET parameters + * Ensure non-JSON GET requests don't pollute $request->request with the GET parameters. * - * @link https://github.com/laravel/framework/pull/37921 Manually populate POST request body with JSON data only when required + * @link https://github.com/laravel/framework/pull/37921 Manually populate POST request body with JSON data only when required. */ public function testNonJsonRequestDoesntFillRequestBodyParams() { $params = ['foo' => 'bar']; - + $getRequest = Request::create('/', 'GET', $params, [], [], []); $this->assertEquals($getRequest->request->all(), []); $this->assertEquals($getRequest->query->all(), $params); - + $postRequest = Request::create('/', 'POST', $params, [], [], []); $this->assertEquals($postRequest->request->all(), $params); $this->assertEquals($postRequest->query->all(), []); From a03c968e97bd6e102047f84612e67df4e93ac22c Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 6 Jul 2021 12:06:29 -0600 Subject: [PATCH 4/5] Extra space removal --- tests/Http/HttpRequestTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Http/HttpRequestTest.php b/tests/Http/HttpRequestTest.php index 0f474df18fcf..a59cb8598f37 100644 --- a/tests/Http/HttpRequestTest.php +++ b/tests/Http/HttpRequestTest.php @@ -1016,7 +1016,7 @@ public function testJsonRequestFillsRequestBodyParams() $this->assertEquals($request->request->all(), $body); } - /** + /** * Ensure non-JSON GET requests don't pollute $request->request with the GET parameters. * * @link https://github.com/laravel/framework/pull/37921 Manually populate POST request body with JSON data only when required. From 320b8d998065c248dbc8701ef3a7f092ba9b6c26 Mon Sep 17 00:00:00 2001 From: Luke Towers Date: Tue, 6 Jul 2021 12:07:25 -0600 Subject: [PATCH 5/5] GitHub's editor needs some work --- tests/Http/HttpRequestTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Http/HttpRequestTest.php b/tests/Http/HttpRequestTest.php index a59cb8598f37..36435677b73c 100644 --- a/tests/Http/HttpRequestTest.php +++ b/tests/Http/HttpRequestTest.php @@ -1015,7 +1015,7 @@ public function testJsonRequestFillsRequestBodyParams() $this->assertEquals($request->request->all(), $body); } - + /** * Ensure non-JSON GET requests don't pollute $request->request with the GET parameters. *