Skip to content

Commit 2942434

Browse files
committed
Improve multipart limits test
The PR introducing this test assumed time would be enough to accurately predict behavior. This commit changes it to using introspection to check the exact state of the parser and expected state once finished parsing a multipart request body. To accomplish that it was required to make the cursor in the file an object property, so it can be inspected using reflection.
1 parent 9681f76 commit 2942434

File tree

2 files changed

+30
-13
lines changed

2 files changed

+30
-13
lines changed

src/Io/MultipartParser.php

+8-6
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ final class MultipartParser
7373
private $postCount = 0;
7474
private $filesCount = 0;
7575
private $emptyCount = 0;
76+
private $cursor = 0;
7677

7778
/**
7879
* @param int|string|null $uploadMaxFilesize
@@ -112,6 +113,7 @@ public function parse(ServerRequestInterface $request)
112113
$request = $this->request;
113114
$this->request = null;
114115
$this->multipartBodyPartCount = 0;
116+
$this->cursor = 0;
115117
$this->postCount = 0;
116118
$this->filesCount = 0;
117119
$this->emptyCount = 0;
@@ -125,20 +127,20 @@ private function parseBody($boundary, $buffer)
125127
$len = \strlen($boundary);
126128

127129
// ignore everything before initial boundary (SHOULD be empty)
128-
$start = \strpos($buffer, $boundary . "\r\n");
130+
$this->cursor = \strpos($buffer, $boundary . "\r\n");
129131

130-
while ($start !== false) {
132+
while ($this->cursor !== false) {
131133
// search following boundary (preceded by newline)
132134
// ignore last if not followed by boundary (SHOULD end with "--")
133-
$start += $len + 2;
134-
$end = \strpos($buffer, "\r\n" . $boundary, $start);
135+
$this->cursor += $len + 2;
136+
$end = \strpos($buffer, "\r\n" . $boundary, $this->cursor);
135137
if ($end === false) {
136138
break;
137139
}
138140

139141
// parse one part and continue searching for next
140-
$this->parsePart(\substr($buffer, $start, $end - $start));
141-
$start = $end;
142+
$this->parsePart(\substr($buffer, $this->cursor, $end - $this->cursor));
143+
$this->cursor = $end;
142144

143145
if (++$this->multipartBodyPartCount > $this->maxMultipartBodyParts) {
144146
break;

tests/Io/MultipartParserTest.php

+22-7
Original file line numberDiff line numberDiff line change
@@ -1029,26 +1029,41 @@ public function testPostMaxFileSizeIgnoredByFilesComingBeforeIt()
10291029

10301030
public function testWeOnlyParseTheAmountOfMultiPartChunksWeConfigured()
10311031
{
1032+
$chunkCount = 5000000;
10321033
$boundary = "---------------------------12758086162038677464950549563";
10331034

10341035
$chunk = "--$boundary\r\n";
10351036
$chunk .= "Content-Disposition: form-data; name=\"f\"\r\n";
10361037
$chunk .= "\r\n";
10371038
$chunk .= "u\r\n";
10381039
$data = '';
1039-
for ($i = 0; $i < 5000000; $i++) {
1040-
$data .= $chunk;
1041-
}
1040+
$data .= str_repeat($chunk, $chunkCount);
10421041
$data .= "--$boundary--\r\n";
10431042

10441043
$request = new ServerRequest('POST', 'http://example.com/', array(
10451044
'Content-Type' => 'multipart/form-data; boundary=' . $boundary,
10461045
), $data, 1.1);
10471046

10481047
$parser = new MultipartParser();
1049-
$startTime = microtime(true);
1050-
$parser->parse($request);
1051-
$runTime = microtime(true) - $startTime;
1052-
$this->assertLessThan(1, $runTime);
1048+
1049+
$reflectecClass = new \ReflectionClass('\React\Http\Io\MultipartParser');
1050+
$requestProperty = $reflectecClass->getProperty('request');
1051+
$requestProperty->setAccessible(true);
1052+
$cursorProperty = $reflectecClass->getProperty('cursor');
1053+
$cursorProperty->setAccessible(true);
1054+
$multipartBodyPartCountProperty = $reflectecClass->getProperty('multipartBodyPartCount');
1055+
$multipartBodyPartCountProperty->setAccessible(true);
1056+
$maxMultipartBodyPartsProperty = $reflectecClass->getProperty('maxMultipartBodyParts');
1057+
$maxMultipartBodyPartsProperty->setAccessible(true);
1058+
$parseBodyMethod = $reflectecClass->getMethod('parseBody');
1059+
$parseBodyMethod->setAccessible(true);
1060+
1061+
$this->assertSame(0, $cursorProperty->getValue($parser));
1062+
1063+
$requestProperty->setValue($parser, $request);
1064+
$parseBodyMethod->invoke($parser, '--' . $boundary, $data);
1065+
1066+
$this->assertSame(strlen(str_repeat($chunk, $multipartBodyPartCountProperty->getValue($parser))), $cursorProperty->getValue($parser) + 2);
1067+
$this->assertSame($multipartBodyPartCountProperty->getValue($parser), $maxMultipartBodyPartsProperty->getValue($parser) + 1);
10531068
}
10541069
}

0 commit comments

Comments
 (0)