Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support malformed multipart body #1733

Merged
merged 4 commits into from
Feb 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
97 changes: 39 additions & 58 deletions src/Event/Http/Psr7Bridge.php
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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);
}

/**
Expand Down
22 changes: 20 additions & 2 deletions tests/Event/Http/CommonHttpTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side-effect bugfix (minor)

$this->assertUploadedFile(
'foo',
'lorem.txt',
Expand Down Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
Original file line number Diff line number Diff line change
@@ -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
}
2 changes: 1 addition & 1 deletion tests/Event/Http/HttpRequestEventTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion tests/Event/Http/Psr7BridgeTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
}
Expand Down
Loading