diff --git a/src/Event/Http/Psr7Bridge.php b/src/Event/Http/Psr7Bridge.php index 667282016..1745fdd86 100644 --- a/src/Event/Http/Psr7Bridge.php +++ b/src/Event/Http/Psr7Bridge.php @@ -93,34 +93,43 @@ public static function convertResponse(ResponseInterface $response): HttpRespons return new HttpResponse($body, $response->getHeaders(), $response->getStatusCode()); } + /** + * @return array{0: array, 1: array|null} + */ private static function parseBodyAndUploadedFiles(HttpRequestEvent $event): array { - $bodyString = $event->getBody(); - $files = []; - $parsedBody = null; $contentType = $event->getContentType(); - if ($contentType !== null && $event->getMethod() === 'POST') { - if (str_starts_with($contentType, 'application/x-www-form-urlencoded')) { - parse_str($bodyString, $parsedBody); + if ($contentType === null || $event->getMethod() !== 'POST') { + return [[], null]; + } + + if (str_starts_with($contentType, 'application/x-www-form-urlencoded')) { + $parsedBody = []; + parse_str($event->getBody(), $parsedBody); + return [[], $parsedBody]; + } + + // Parse the body as multipart/form-data + $document = new Part("Content-type: $contentType\r\n\r\n" . $event->getBody()); + if (! $document->isMultiPart()) { + return [[], null]; + } + $parsedBody = null; + $files = []; + foreach ($document->getParts() as $part) { + if ($part->isFile()) { + $tmpPath = tempnam(sys_get_temp_dir(), self::UPLOADED_FILES_PREFIX); + if ($tmpPath === false) { + throw new RuntimeException('Unable to create a temporary directory'); + } + file_put_contents($tmpPath, $part->getBody()); + $file = new UploadedFile($tmpPath, filesize($tmpPath), UPLOAD_ERR_OK, $part->getFileName(), $part->getMimeType()); + self::parseKeyAndInsertValueInArray($files, $part->getName(), $file); } else { - $document = new Part("Content-type: $contentType\r\n\r\n" . $bodyString); - if ($document->isMultiPart()) { + if ($parsedBody === null) { $parsedBody = []; - foreach ($document->getParts() as $part) { - if ($part->isFile()) { - $tmpPath = tempnam(sys_get_temp_dir(), self::UPLOADED_FILES_PREFIX); - if ($tmpPath === false) { - throw new RuntimeException('Unable to create a temporary directory'); - } - file_put_contents($tmpPath, $part->getBody()); - $file = new UploadedFile($tmpPath, filesize($tmpPath), UPLOAD_ERR_OK, $part->getFileName(), $part->getMimeType()); - - self::parseKeyAndInsertValueInArray($files, $part->getName(), $file); - } else { - self::parseKeyAndInsertValueInArray($parsedBody, $part->getName(), $part->getBody()); - } - } } + self::parseKeyAndInsertValueInArray($parsedBody, $part->getName(), $part->getBody()); } } return [$files, $parsedBody]; @@ -131,42 +140,14 @@ private static function parseBodyAndUploadedFiles(HttpRequestEvent $event): arra */ private static function parseKeyAndInsertValueInArray(array &$array, string $key, mixed $value): void { - if (! str_contains($key, '[')) { - $array[$key] = $value; - - return; - } - - $parts = explode('[', $key); // files[id_cards][jpg][] => [ 'files', 'id_cards]', 'jpg]', ']' ] - $pointer = &$array; - - foreach ($parts as $k => $part) { - if ($k === 0) { - $pointer = &$pointer[$part]; - - continue; - } - - // Skip two special cases: - // [[ in the key produces empty string - // [test : starts with [ but does not end with ] - if ($part === '' || ! str_ends_with($part, ']')) { - // Malformed key, we use it "as is" - $array[$key] = $value; - - return; - } - - $part = substr($part, 0, -1); // The last char is a ] => remove it to have the real key - - if ($part === '') { // [] case - $pointer = &$pointer[]; - } else { - $pointer = &$pointer[$part]; - } - } - - $pointer = $value; + $parsed = []; + // We use parse_str to parse the key in the same way PHP does natively + // We use "=mock" because the value can be an object (in case of uploaded files) + parse_str(urlencode($key) . '=mock', $parsed); + // Replace `mock` with the actual value + array_walk_recursive($parsed, fn (&$v) => $v = $value); + // Merge recursively into the main array to avoid overwriting existing values + $array = array_merge_recursive($array, $parsed); } /** diff --git a/tests/Event/Http/CommonHttpTest.php b/tests/Event/Http/CommonHttpTest.php index b6cc0e34e..1c4805901 100644 --- a/tests/Event/Http/CommonHttpTest.php +++ b/tests/Event/Http/CommonHttpTest.php @@ -357,6 +357,24 @@ public function test POST request with multipart form data containing ar ]); } + /** + * @dataProvider provide API Gateway versions + */ + public function test POST request with malformed multipart form data(int $version) + { + $this->fromFixture(__DIR__ . "/Fixture/ag-v$version-body-form-multipart-arrays-malformed.json"); + + $this->assertContentType('multipart/form-data; boundary=testBoundary'); + $body = "--testBoundary\r +Content-Disposition: form-data; name=\"key0[key1][key2][\"\r +\r +123\r +--testBoundary--\r +"; + $this->assertBody($body); + $this->assertParsedBody(['key0' => ['key1' => ['key2' => '123']]]); + } + /** * @dataProvider provide API Gateway versions */ @@ -383,7 +401,7 @@ public function test POST request with multipart file uploads(int $version --testBoundary--\r "; $this->assertBody($body); - $this->assertParsedBody([]); + $this->assertParsedBody(null); $this->assertUploadedFile( 'foo', 'lorem.txt', @@ -536,7 +554,7 @@ abstract protected function assertUri(string $expected): void; abstract protected function assertHasMultiHeader(bool $expected): void; - abstract protected function assertParsedBody(array $expected): void; + abstract protected function assertParsedBody(array | null $expected): void; abstract protected function assertSourceIp(string $expected): void; diff --git a/tests/Event/Http/Fixture/ag-v1-body-form-multipart-arrays-malformed.json b/tests/Event/Http/Fixture/ag-v1-body-form-multipart-arrays-malformed.json new file mode 100644 index 000000000..c5fb11bb2 --- /dev/null +++ b/tests/Event/Http/Fixture/ag-v1-body-form-multipart-arrays-malformed.json @@ -0,0 +1,53 @@ +{ + "version": "1.0", + "resource": "/path", + "path": "/path", + "httpMethod": "POST", + "headers": { + "Accept": "*/*", + "Accept-Encoding": "gzip, deflate", + "Cache-Control": "no-cache", + "Content-Type": "multipart/form-data; boundary=testBoundary", + "Host": "example.org", + "User-Agent": "PostmanRuntime/7.20.1", + "X-Amzn-Trace-Id": "Root=1-ffffffff-ffffffffffffffffffffffff", + "X-Forwarded-For": "1.1.1.1", + "X-Forwarded-Port": "443", + "X-Forwarded-Proto": "https" + }, + "queryStringParameters": null, + "pathParameters": null, + "stageVariables": null, + "requestContext": { + "resourceId": "xxxxxx", + "resourcePath": "/path", + "httpMethod": "PUT", + "extendedRequestId": "XXXXXX-xxxxxxxx=", + "requestTime": "24/Nov/2019:18:55:08 +0000", + "path": "/path", + "accountId": "123400000000", + "protocol": "HTTP/1.1", + "stage": "dev", + "domainPrefix": "dev", + "requestTimeEpoch": 1574621708700, + "requestId": "ffffffff-ffff-4fff-ffff-ffffffffffff", + "identity": { + "cognitoIdentityPoolId": null, + "accountId": null, + "cognitoIdentityId": null, + "caller": null, + "sourceIp": "1.1.1.1", + "principalOrgId": null, + "accessKey": null, + "cognitoAuthenticationType": null, + "cognitoAuthenticationProvider": null, + "userArn": null, + "userAgent": "PostmanRuntime/7.20.1", + "user": null + }, + "domainName": "example.org", + "apiId": "xxxxxxxxxx" + }, + "body": "--testBoundary\r\nContent-Disposition: form-data; name=\"key0[key1][key2][\"\r\n\r\n123\r\n--testBoundary--\r\n", + "isBase64Encoded": false +} diff --git a/tests/Event/Http/Fixture/ag-v2-body-form-multipart-arrays-malformed.json b/tests/Event/Http/Fixture/ag-v2-body-form-multipart-arrays-malformed.json new file mode 100644 index 000000000..67209fbb6 --- /dev/null +++ b/tests/Event/Http/Fixture/ag-v2-body-form-multipart-arrays-malformed.json @@ -0,0 +1,41 @@ +{ + "version": "2.0", + "routeKey": "ANY /path", + "rawPath": "/path", + "rawQueryString": "", + "headers": { + "Accept": "*/*", + "Accept-Encoding": "gzip, deflate", + "Cache-Control": "no-cache", + "Content-Type": "multipart/form-data; boundary=testBoundary", + "Host": "example.org", + "User-Agent": "PostmanRuntime/7.20.1", + "X-Amzn-Trace-Id": "Root=1-ffffffff-ffffffffffffffffffffffff", + "X-Forwarded-For": "1.1.1.1", + "X-Forwarded-Port": "443", + "X-Forwarded-Proto": "https" + }, + "queryStringParameters": null, + "stageVariables": null, + "requestContext": { + "accountId": "123400000000", + "apiId": "xxxxxxxxxx", + "domainName": "example.org", + "domainPrefix": "0000000000", + "http": { + "method": "POST", + "path": "/path", + "protocol": "HTTP/1.1", + "sourceIp": "1.1.1.1", + "userAgent": "PostmanRuntime/7.20.1" + }, + "requestId": "JTHoQgr2oAMEPMg=", + "routeId": "47matwk", + "routeKey": "ANY /path", + "stage": "$default", + "time": "24/Nov/2019:18:55:08 +0000", + "timeEpoch": 1574621708700 + }, + "body": "--testBoundary\r\nContent-Disposition: form-data; name=\"key0[key1][key2][\"\r\n\r\n123\r\n--testBoundary--\r\n", + "isBase64Encoded": false +} diff --git a/tests/Event/Http/HttpRequestEventTest.php b/tests/Event/Http/HttpRequestEventTest.php index d1f2f516d..925a324f9 100644 --- a/tests/Event/Http/HttpRequestEventTest.php +++ b/tests/Event/Http/HttpRequestEventTest.php @@ -112,7 +112,7 @@ protected function assertSourceIp(string $expected): void $this->assertEquals($expected, $this->event->getSourceIp()); } - protected function assertParsedBody(array $expected): void + protected function assertParsedBody(array | null $expected): void { // Not applicable here since the class doesn't parse the body } diff --git a/tests/Event/Http/Psr7BridgeTest.php b/tests/Event/Http/Psr7BridgeTest.php index e2ab8de1a..a6d0f921c 100644 --- a/tests/Event/Http/Psr7BridgeTest.php +++ b/tests/Event/Http/Psr7BridgeTest.php @@ -123,7 +123,7 @@ protected function assertHasMultiHeader(bool $expected): void // Not applicable here } - protected function assertParsedBody(array $expected): void + protected function assertParsedBody(array | null $expected): void { $this->assertEquals($expected, $this->request->getParsedBody()); }