Merge pull request #1733 from brefphp/multipart-arrays-malformed

Support malformed multipart body
brefphp · Feb 1, 2024 · 535ad5f · 535ad5f
2 parents 9868c17 + 56df95e
commit 535ad5f
Show file tree

Hide file tree

Showing 6 changed files with 155 additions and 62 deletions.
diff --git 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<string, UploadedFile>, 1: array<string, mixed>|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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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());
     }