From 3dabcdb6abae2d93e04bc220b52f5ac4cec66e40 Mon Sep 17 00:00:00 2001 From: Walid Said Date: Mon, 5 Oct 2020 12:08:32 +0200 Subject: [PATCH] Fix #18313: Fix multipart form data parse with double quotes --- framework/CHANGELOG.md | 2 +- framework/web/MultipartFormDataParser.php | 5 ++- .../web/MultipartFormDataParserTest.php | 39 +++++++++++++++++++ 3 files changed, 43 insertions(+), 3 deletions(-) diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md index 943cefb103..d50abcde07 100644 --- a/framework/CHANGELOG.md +++ b/framework/CHANGELOG.md @@ -10,7 +10,7 @@ Yii Framework 2 Change Log - Bug #18303: Fix creating migration issue for column methods used after defaultValues (wsaid) - Bug #18287: Fix for OUTPUT INSERTED and computed columns. Added flag to computed values in table schema (darkdef) - Bug #18308: Fixed `\yii\base\Model::getErrorSummary()` reverse order (DrDeath72) - +- Bug #18313: Fix multipart form data parse with double quotes (wsaid) 2.0.38 September 14, 2020 ------------------------- diff --git a/framework/web/MultipartFormDataParser.php b/framework/web/MultipartFormDataParser.php index 0685160f1b..c6f8c8ccaa 100644 --- a/framework/web/MultipartFormDataParser.php +++ b/framework/web/MultipartFormDataParser.php @@ -141,10 +141,11 @@ class MultipartFormDataParser extends BaseObject implements RequestParserInterfa return []; } - if (!preg_match('/boundary=(.*)$/is', $contentType, $matches)) { + if (!preg_match('/boundary="?(.*)"?$/is', $contentType, $matches)) { return []; } - $boundary = $matches[1]; + + $boundary = trim($matches[1], '"'); $bodyParts = preg_split('/\\R?-+' . preg_quote($boundary, '/') . '/s', $rawBody); array_pop($bodyParts); // last block always has no data, contains boundary ending like `--` diff --git a/tests/framework/web/MultipartFormDataParserTest.php b/tests/framework/web/MultipartFormDataParserTest.php index 1212c72534..4447defec2 100644 --- a/tests/framework/web/MultipartFormDataParserTest.php +++ b/tests/framework/web/MultipartFormDataParserTest.php @@ -52,6 +52,45 @@ class MultipartFormDataParserTest extends TestCase $this->assertStringEqualsFile($_FILES['Item']['tmp_name']['file'], 'item file content'); } + public function testParseWithDoubleQuotes() + { + if (defined('HHVM_VERSION')) { + static::markTestSkipped('Can not test on HHVM because it does not support proper handling of the temporary files.'); + } + + $parser = new MultipartFormDataParser(); + + $boundary = '---------------------------22472926011618'; + $contentType = 'multipart/form-data; boundary="' . $boundary . '"'; + $rawBody = "--{$boundary}\nContent-Disposition: form-data; name=\"title\"\r\n\r\ntest-title"; + $rawBody .= "\r\n--{$boundary}\nContent-Disposition: form-data; name=\"Item[name]\"\r\n\r\ntest-name"; + $rawBody .= "\r\n--{$boundary}\nContent-Disposition: form-data; name=\"someFile\"; filename=\"some-file.txt\"\nContent-Type: text/plain\r\n\r\nsome file content"; + $rawBody .= "\r\n--{$boundary}\nContent-Disposition: form-data; name=\"Item[file]\"; filename=\"item-file.txt\"\nContent-Type: text/plain\r\n\r\nitem file content"; + $rawBody .= "\r\n--{$boundary}--"; + + $bodyParams = $parser->parse($rawBody, $contentType); + + $expectedBodyParams = [ + 'title' => 'test-title', + 'Item' => [ + 'name' => 'test-name', + ], + ]; + $this->assertEquals($expectedBodyParams, $bodyParams); + + $this->assertNotEmpty($_FILES['someFile']); + $this->assertEquals(UPLOAD_ERR_OK, $_FILES['someFile']['error']); + $this->assertEquals('some-file.txt', $_FILES['someFile']['name']); + $this->assertEquals('text/plain', $_FILES['someFile']['type']); + $this->assertStringEqualsFile($_FILES['someFile']['tmp_name'], 'some file content'); + + $this->assertNotEmpty($_FILES['Item']); + $this->assertNotEmpty($_FILES['Item']['name']['file']); + $this->assertEquals(UPLOAD_ERR_OK, $_FILES['Item']['error']['file']); + $this->assertEquals('item-file.txt', $_FILES['Item']['name']['file']); + $this->assertEquals('text/plain', $_FILES['Item']['type']['file']); + $this->assertStringEqualsFile($_FILES['Item']['tmp_name']['file'], 'item file content'); + } /** * @depends testParse */