From 1489a928c20046c83377b38ce57d21a9772b8371 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 5 Jun 2019 11:13:39 +0200 Subject: [PATCH 1/8] Add regression test --- tests/Integration/RequestIntegrationTest.php | 40 ++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/Integration/RequestIntegrationTest.php b/tests/Integration/RequestIntegrationTest.php index 7d97c4fc2..f5dc1e74f 100644 --- a/tests/Integration/RequestIntegrationTest.php +++ b/tests/Integration/RequestIntegrationTest.php @@ -365,6 +365,46 @@ public function applyToEventDataProvider(): \Generator ], ]; + yield [ + [ + 'max_request_body_size' => 'always', + ], + (new ServerRequest()) + ->withUploadedFiles([ + 'foo' => [ + 'bar' => [ + new UploadedFile('foo content', 123, UPLOAD_ERR_OK, 'foo.ext', 'application/text'), + new UploadedFile('bar content', 321, UPLOAD_ERR_OK, 'bar.ext', 'application/octet-stream'), + ], + ], + ]) + ->withUri(new Uri('http://www.example.com/foo')) + ->withMethod('POST'), + [ + 'url' => 'http://www.example.com/foo', + 'method' => 'POST', + 'headers' => [ + 'Host' => ['www.example.com'], + ], + 'data' => [ + 'foo' => [ + 'bar' => [ + [ + 'client_filename' => 'foo.ext', + 'client_media_type' => 'application/text', + 'size' => 123, + ], + [ + 'client_filename' => 'bar.ext', + 'client_media_type' => 'application/octet-stream', + 'size' => 321, + ], + ], + ], + ], + ], + ]; + yield [ [ 'max_request_body_size' => 'always', From dede9ece59c5e05851cdb6b506ea8584db7f8e1c Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Wed, 5 Jun 2019 12:15:55 +0200 Subject: [PATCH 2/8] Fix the UploadedFiles handling --- src/Integration/RequestIntegration.php | 43 ++++++++++++-------- tests/Integration/RequestIntegrationTest.php | 8 ++-- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/src/Integration/RequestIntegration.php b/src/Integration/RequestIntegration.php index 3747d7381..498ddc332 100644 --- a/src/Integration/RequestIntegration.php +++ b/src/Integration/RequestIntegration.php @@ -164,23 +164,11 @@ private function captureRequestBody(ServerRequestInterface $serverRequest) return null; } - $requestData = $serverRequest->getParsedBody(); - $requestData = \is_array($requestData) ? $requestData : []; - - foreach ($serverRequest->getUploadedFiles() as $fieldName => $uploadedFiles) { - if (!\is_array($uploadedFiles)) { - $uploadedFiles = [$uploadedFiles]; - } - - /** @var UploadedFileInterface $uploadedFile */ - foreach ($uploadedFiles as $uploadedFile) { - $requestData[$fieldName][] = [ - 'client_filename' => $uploadedFile->getClientFilename(), - 'client_media_type' => $uploadedFile->getClientMediaType(), - 'size' => $uploadedFile->getSize(), - ]; - } - } + $parsedBody = $serverRequest->getParsedBody(); + $requestData = array_merge( + $this->parseUploadedFiles($serverRequest->getUploadedFiles()), + \is_array($parsedBody) ? $parsedBody : [] + ); if (!empty($requestData)) { return $requestData; @@ -196,4 +184,25 @@ private function captureRequestBody(ServerRequestInterface $serverRequest) return $requestBody->getContents(); } + + private function parseUploadedFiles(array $uploadedFiles): array + { + $data = []; + + foreach ($uploadedFiles as $key => $item) { + if ($item instanceof UploadedFileInterface) { + $data[$key] = [ + 'client_filename' => $item->getClientFilename(), + 'client_media_type' => $item->getClientMediaType(), + 'size' => $item->getSize(), + ]; + } elseif (\is_array($item)) { + $data[$key] = $this->parseUploadedFiles($item); + } else { + throw new \InvalidArgumentException('Unrecognized UploadedFiles item, got ' . \gettype($item)); + } + } + + return $data; + } } diff --git a/tests/Integration/RequestIntegrationTest.php b/tests/Integration/RequestIntegrationTest.php index f5dc1e74f..3086dd436 100644 --- a/tests/Integration/RequestIntegrationTest.php +++ b/tests/Integration/RequestIntegrationTest.php @@ -319,11 +319,9 @@ public function applyToEventDataProvider(): \Generator ], 'data' => [ 'foo' => [ - [ - 'client_filename' => 'foo.ext', - 'client_media_type' => 'application/text', - 'size' => 123, - ], + 'client_filename' => 'foo.ext', + 'client_media_type' => 'application/text', + 'size' => 123, ], ], ], From 8abbad2e467d91451ff65f0689140c629f10224c Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 6 Jun 2019 10:51:33 +0200 Subject: [PATCH 3/8] Update the changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 361d91c5d..3a90a136c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Fix the behavior of the `excluded_exceptions` option: now it's used to skip capture of exceptions, not to purge the `exception` data of the event, which resulted in broken or empty chains of exceptions in reported events (#822) +- Fix handling of uploaded files in the `RequestIntegration`, to respect the PSR-7 spec fully (#827) ## 2.1.0 (2019-05-22) From 452658503d7b3c2ed4f12295110fc439c26422f0 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Thu, 6 Jun 2019 10:56:18 +0200 Subject: [PATCH 4/8] Drop duplicated test case --- tests/Integration/RequestIntegrationTest.php | 36 -------------------- 1 file changed, 36 deletions(-) diff --git a/tests/Integration/RequestIntegrationTest.php b/tests/Integration/RequestIntegrationTest.php index 3086dd436..95775585f 100644 --- a/tests/Integration/RequestIntegrationTest.php +++ b/tests/Integration/RequestIntegrationTest.php @@ -403,42 +403,6 @@ public function applyToEventDataProvider(): \Generator ], ]; - yield [ - [ - 'max_request_body_size' => 'always', - ], - (new ServerRequest()) - ->withUploadedFiles([ - 'foo' => [ - new UploadedFile('foo content', 123, UPLOAD_ERR_OK, 'foo.ext', 'application/text'), - new UploadedFile('bar content', 321, UPLOAD_ERR_OK, 'bar.ext', 'application/octet-stream'), - ], - ]) - ->withUri(new Uri('http://www.example.com/foo')) - ->withMethod('POST'), - [ - 'url' => 'http://www.example.com/foo', - 'method' => 'POST', - 'headers' => [ - 'Host' => ['www.example.com'], - ], - 'data' => [ - 'foo' => [ - [ - 'client_filename' => 'foo.ext', - 'client_media_type' => 'application/text', - 'size' => 123, - ], - [ - 'client_filename' => 'bar.ext', - 'client_media_type' => 'application/octet-stream', - 'size' => 321, - ], - ], - ], - ], - ]; - yield [ [ 'max_request_body_size' => 'always', From c13ba2b50881770ec4cb5a372fa0b97455ecd9ee Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Mon, 10 Jun 2019 09:24:56 +0200 Subject: [PATCH 5/8] Address review comments --- src/Integration/RequestIntegration.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Integration/RequestIntegration.php b/src/Integration/RequestIntegration.php index 498ddc332..6a80eae56 100644 --- a/src/Integration/RequestIntegration.php +++ b/src/Integration/RequestIntegration.php @@ -185,6 +185,10 @@ private function captureRequestBody(ServerRequestInterface $serverRequest) return $requestBody->getContents(); } + /** + * Create an array with the same structure as $uploadedFiles, but replacing + * each UploadedFileInterface with an array of info. + */ private function parseUploadedFiles(array $uploadedFiles): array { $data = []; From d2a62395b9184b259896217b624f44a50dbda7fd Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Mon, 10 Jun 2019 09:34:56 +0200 Subject: [PATCH 6/8] Improve exception --- src/Integration/RequestIntegration.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Integration/RequestIntegration.php b/src/Integration/RequestIntegration.php index 6a80eae56..08834f098 100644 --- a/src/Integration/RequestIntegration.php +++ b/src/Integration/RequestIntegration.php @@ -203,7 +203,8 @@ private function parseUploadedFiles(array $uploadedFiles): array } elseif (\is_array($item)) { $data[$key] = $this->parseUploadedFiles($item); } else { - throw new \InvalidArgumentException('Unrecognized UploadedFiles item, got ' . \gettype($item)); + $type = is_object($item) ? \get_class($item) : \gettype($item); + throw new \UnexpectedValueException('Expecting UploadedFileInterface , got ' . $type); } } From 34411c35eb3332679ff1e0e12c275a144540a0b3 Mon Sep 17 00:00:00 2001 From: Alessandro Lai Date: Mon, 10 Jun 2019 10:10:28 +0200 Subject: [PATCH 7/8] Fix PHPDoc --- src/Integration/RequestIntegration.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/Integration/RequestIntegration.php b/src/Integration/RequestIntegration.php index 08834f098..168e5dda4 100644 --- a/src/Integration/RequestIntegration.php +++ b/src/Integration/RequestIntegration.php @@ -188,6 +188,10 @@ private function captureRequestBody(ServerRequestInterface $serverRequest) /** * Create an array with the same structure as $uploadedFiles, but replacing * each UploadedFileInterface with an array of info. + * + * @param array $uploadedFiles The uploaded files info from a PSR-7 server request + * + * @return array The same array with UploadedFileInterface replaced with plain info */ private function parseUploadedFiles(array $uploadedFiles): array { @@ -203,7 +207,7 @@ private function parseUploadedFiles(array $uploadedFiles): array } elseif (\is_array($item)) { $data[$key] = $this->parseUploadedFiles($item); } else { - $type = is_object($item) ? \get_class($item) : \gettype($item); + $type = \is_object($item) ? \get_class($item) : \gettype($item); throw new \UnexpectedValueException('Expecting UploadedFileInterface , got ' . $type); } } From 5e82e97e2e42aeeef815358ed6f6308fce15d93d Mon Sep 17 00:00:00 2001 From: Stefano Arlandini Date: Mon, 10 Jun 2019 11:14:43 +0200 Subject: [PATCH 8/8] Fix CR issues --- src/Integration/RequestIntegration.php | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/Integration/RequestIntegration.php b/src/Integration/RequestIntegration.php index 168e5dda4..c2efc480a 100644 --- a/src/Integration/RequestIntegration.php +++ b/src/Integration/RequestIntegration.php @@ -164,10 +164,10 @@ private function captureRequestBody(ServerRequestInterface $serverRequest) return null; } - $parsedBody = $serverRequest->getParsedBody(); + $requestData = $serverRequest->getParsedBody(); $requestData = array_merge( $this->parseUploadedFiles($serverRequest->getUploadedFiles()), - \is_array($parsedBody) ? $parsedBody : [] + \is_array($requestData) ? $requestData : [] ); if (!empty($requestData)) { @@ -191,27 +191,26 @@ private function captureRequestBody(ServerRequestInterface $serverRequest) * * @param array $uploadedFiles The uploaded files info from a PSR-7 server request * - * @return array The same array with UploadedFileInterface replaced with plain info + * @return array */ private function parseUploadedFiles(array $uploadedFiles): array { - $data = []; + $result = []; foreach ($uploadedFiles as $key => $item) { if ($item instanceof UploadedFileInterface) { - $data[$key] = [ + $result[$key] = [ 'client_filename' => $item->getClientFilename(), 'client_media_type' => $item->getClientMediaType(), 'size' => $item->getSize(), ]; } elseif (\is_array($item)) { - $data[$key] = $this->parseUploadedFiles($item); + $result[$key] = $this->parseUploadedFiles($item); } else { - $type = \is_object($item) ? \get_class($item) : \gettype($item); - throw new \UnexpectedValueException('Expecting UploadedFileInterface , got ' . $type); + throw new \UnexpectedValueException(sprintf('Expected either an object implementing the "%s" interface or an array. Got: "%s".', UploadedFileInterface::class, \is_object($item) ? \get_class($item) : \gettype($item))); } } - return $data; + return $result; } }