Skip to content

Commit cd5d3c8

Browse files
committed
Strictly obey newline definitions as per RFC 7578 and RFC 2046
1 parent 7ed5a80 commit cd5d3c8

2 files changed

Lines changed: 52 additions & 30 deletions

File tree

src/Io/MultipartParser.php

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
* that resembles PHP's `$_POST` and `$_FILES` superglobals.
1313
*
1414
* @internal
15+
* @link https://tools.ietf.org/html/rfc7578
16+
* @link https://tools.ietf.org/html/rfc2046#section-5.1.1
1517
*/
1618
final class MultipartParser
1719
{
@@ -43,23 +45,34 @@ private function parse()
4345
return $this->request;
4446
}
4547

46-
$this->parseBuffer($matches[1], (string)$this->request->getBody());
48+
$this->parseBody('--' . $matches[1], (string)$this->request->getBody());
4749

4850
return $this->request;
4951
}
5052

51-
private function parseBuffer($boundary, $buffer)
53+
private function parseBody($boundary, $buffer)
5254
{
53-
$chunks = explode('--' . $boundary, $buffer);
54-
array_pop($chunks);
55+
$len = strlen($boundary);
56+
57+
// ignore everything before initial boundary (SHOULD be empty)
58+
$start = strpos($buffer, $boundary . "\r\n");
59+
60+
while ($start !== false) {
61+
// search following boundary (preceded by newline)
62+
// ignore last if not followed by boundary (SHOULD end with "--")
63+
$start += $len + 2;
64+
$end = strpos($buffer, "\r\n" . $boundary, $start);
65+
if ($end === false) {
66+
break;
67+
}
5568

56-
foreach ($chunks as $chunk) {
57-
$chunk = $this->stripTrailingEOL($chunk);
58-
$this->parseChunk($chunk);
69+
// parse one part and continue searching for next
70+
$this->parsePart(substr($buffer, $start, $end - $start));
71+
$start = $end;
5972
}
6073
}
6174

62-
private function parseChunk($chunk)
75+
private function parsePart($chunk)
6376
{
6477
$pos = strpos($chunk, "\r\n\r\n");
6578
if ($pos === false) {
@@ -157,10 +170,13 @@ private function parseHeaders($header)
157170
$headers = array();
158171

159172
foreach (explode("\r\n", trim($header)) as $line) {
160-
list($key, $values) = explode(':', $line, 2);
161-
$key = trim($key);
162-
$key = strtolower($key);
163-
$values = explode(';', $values);
173+
$parts = explode(':', $line, 2);
174+
if (!isset($parts[1])) {
175+
continue;
176+
}
177+
178+
$key = strtolower(trim($parts[0]));
179+
$values = explode(';', $parts[1]);
164180
$values = array_map('trim', $values);
165181
$headers[$key] = $values;
166182
}
@@ -179,15 +195,6 @@ private function getParameterFromHeader(array $header, $parameter)
179195
return null;
180196
}
181197

182-
private function stripTrailingEOL($chunk)
183-
{
184-
if (substr($chunk, -2) === "\r\n") {
185-
return substr($chunk, 0, -2);
186-
}
187-
188-
return $chunk;
189-
}
190-
191198
private function extractPost($postFields, $key, $value)
192199
{
193200
$chunks = explode('[', $key);

tests/Io/MultipartParserTest.php

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ public function testInvalidDoubleContentDispositionUsesLast()
429429
);
430430
}
431431

432-
public function testInvalidMissingNewlineAfterValueDoesNotMatter()
432+
public function testInvalidMissingNewlineAfterValueWillBeIgnored()
433433
{
434434
$boundary = "---------------------------5844729766471062541057622570";
435435

@@ -446,12 +446,7 @@ public function testInvalidMissingNewlineAfterValueDoesNotMatter()
446446
$parsedRequest = MultipartParser::parseRequest($request);
447447

448448
$this->assertEmpty($parsedRequest->getUploadedFiles());
449-
$this->assertSame(
450-
array(
451-
'key' => 'value'
452-
),
453-
$parsedRequest->getParsedBody()
454-
);
449+
$this->assertEmpty($parsedRequest->getParsedBody());
455450
}
456451

457452
public function testInvalidMissingValueWillBeIgnored()
@@ -497,7 +492,27 @@ public function testInvalidContentDispositionMissingWillBeIgnored()
497492
$data = "--$boundary\r\n";
498493
$data .= "Content-Type: text/plain\r\n";
499494
$data .= "\r\n";
500-
$data .= "hello";
495+
$data .= "hello\r\n";
496+
$data .= "--$boundary--\r\n";
497+
498+
$request = new ServerRequest('POST', 'http://example.com/', array(
499+
'Content-Type' => 'multipart/form-data; boundary=' . $boundary,
500+
), $data, 1.1);
501+
502+
$parsedRequest = MultipartParser::parseRequest($request);
503+
504+
$this->assertEmpty($parsedRequest->getUploadedFiles());
505+
$this->assertEmpty($parsedRequest->getParsedBody());
506+
}
507+
508+
public function testInvalidContentDispositionMissingValueWillBeIgnored()
509+
{
510+
$boundary = "---------------------------5844729766471062541057622570";
511+
512+
$data = "--$boundary\r\n";
513+
$data .= "Content-Disposition\r\n";
514+
$data .= "\r\n";
515+
$data .= "value\r\n";
501516
$data .= "--$boundary--\r\n";
502517

503518
$request = new ServerRequest('POST', 'http://example.com/', array(
@@ -517,7 +532,7 @@ public function testInvalidContentDispositionWithoutNameWillBeIgnored()
517532
$data = "--$boundary\r\n";
518533
$data .= "Content-Disposition: form-data; something=\"key\"\r\n";
519534
$data .= "\r\n";
520-
$data .= "value";
535+
$data .= "value\r\n";
521536
$data .= "--$boundary--\r\n";
522537

523538
$request = new ServerRequest('POST', 'http://example.com/', array(

0 commit comments

Comments
 (0)