Skip to content

Commit 3d09f43

Browse files
committed
Parse Content-Type parameters with quoted-string awareness
Empty quoted boundary (`boundary=""`) left $matches[2] unset and became a PHP warning. The substring match also picked `boundary=` inside other parameter names and quoted values. Split parameters only outside quoted-strings, reject an empty boundary, and cover those cases in tests.
1 parent 30ce483 commit 3d09f43

3 files changed

Lines changed: 98 additions & 6 deletions

File tree

framework/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ Yii Framework 2 Change Log
44
2.0.56 under development
55
------------------------
66

7+
- Bug #21073: Fix `MultipartFormDataParser` boundary extraction when `Content-Type` has additional parameters (iliaal)
78
- Bug #21020: Fix duplicate `@return` annotation for `yii\db\ActiveRecord::hasOne()` (nazard)
89
- Bug #20873: Fix PHPDoc annotations for the `yii\log\Target::$enabled` (mspirkov)
910
- Enh #20875: Clarify the type of the `yii\base\Model::$errors` (mspirkov)

framework/web/MultipartFormDataParser.php

Lines changed: 91 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,8 @@ public function parse($rawBody, $contentType)
142142
return [];
143143
}
144144

145-
if (!preg_match('/boundary=(?:"([^"]*)"|([^;\s]*))/is', $contentType, $matches)) {
146-
return [];
147-
}
148-
149-
$boundary = isset($matches[1]) && $matches[1] !== '' ? $matches[1] : $matches[2];
150-
if ($boundary === '') {
145+
$boundary = $this->getMimeParameter($contentType, 'boundary');
146+
if ($boundary === null || $boundary === '') {
151147
return [];
152148
}
153149

@@ -214,6 +210,95 @@ public function parse($rawBody, $contentType)
214210
return $bodyParams;
215211
}
216212

213+
/**
214+
* Returns a MIME header parameter (RFC 2045), or `null` if it is absent.
215+
*
216+
* `;` and `name=value` inside quoted-strings are not treated as delimiters.
217+
*
218+
* @param string $header Content-Type (or similar) header value
219+
* @param string $name parameter name (matched case-insensitively)
220+
* @return string|null
221+
*/
222+
private function getMimeParameter($header, $name)
223+
{
224+
$name = strtolower($name);
225+
$length = strlen($header);
226+
$i = 0;
227+
228+
while ($i < $length) {
229+
if ($header[$i] === '"') {
230+
$i++;
231+
while ($i < $length) {
232+
if ($header[$i] === '\\') {
233+
$i += ($i + 1 < $length) ? 2 : 1;
234+
continue;
235+
}
236+
if ($header[$i] === '"') {
237+
$i++;
238+
break;
239+
}
240+
$i++;
241+
}
242+
continue;
243+
}
244+
245+
if ($header[$i] !== ';') {
246+
$i++;
247+
continue;
248+
}
249+
250+
$i++;
251+
while ($i < $length && ($header[$i] === ' ' || $header[$i] === "\t")) {
252+
$i++;
253+
}
254+
255+
$attrStart = $i;
256+
while ($i < $length && $header[$i] !== '=' && $header[$i] !== ';') {
257+
$i++;
258+
}
259+
$attr = strtolower(trim(substr($header, $attrStart, $i - $attrStart)));
260+
261+
if ($attr === '' || $i >= $length || $header[$i] !== '=') {
262+
continue;
263+
}
264+
265+
$i++;
266+
while ($i < $length && ($header[$i] === ' ' || $header[$i] === "\t")) {
267+
$i++;
268+
}
269+
270+
if ($i < $length && $header[$i] === '"') {
271+
$i++;
272+
$value = '';
273+
while ($i < $length) {
274+
if ($header[$i] === '\\' && $i + 1 < $length) {
275+
$value .= $header[$i + 1];
276+
$i += 2;
277+
continue;
278+
}
279+
if ($header[$i] === '"') {
280+
$i++;
281+
break;
282+
}
283+
$value .= $header[$i];
284+
$i++;
285+
}
286+
} else {
287+
$valueStart = $i;
288+
while ($i < $length && $header[$i] !== ';' && $header[$i] !== ' ' && $header[$i] !== "\t") {
289+
$i++;
290+
}
291+
$value = substr($header, $valueStart, $i - $valueStart);
292+
}
293+
294+
if ($attr === $name) {
295+
return $value;
296+
}
297+
}
298+
299+
return null;
300+
}
301+
217302
/**
218303
* Parses content part headers.
219304
* @param string $headerContent headers source content

tests/framework/web/MultipartFormDataParserTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ public function testParseWithBoundaryFollowedByAdditionalParameters(): void
2525
[
2626
'multipart/form-data; boundary=' . $boundary . '; charset=utf-8',
2727
'multipart/form-data; boundary="' . $boundary . '"; charset=utf-8',
28+
'multipart/form-data; x-boundary=wrong; boundary=' . $boundary,
29+
'multipart/form-data; foo="a; boundary=wrong"; boundary=' . $boundary,
30+
'multipart/form-data; charset="boundary=abc"; boundary=' . $boundary,
31+
'multipart/form-data; charset="boundary=abc"; boundary="' . $boundary . '"',
2832
] as $contentType
2933
) {
3034
$bodyParams = $parser->parse($rawBody, $contentType);
@@ -38,6 +42,8 @@ public function testParseWithoutBoundary(): void
3842

3943
$this->assertSame([], $parser->parse("--irrelevant\r\n\r\ndata", 'multipart/form-data'));
4044
$this->assertSame([], $parser->parse('--irrelevant', 'multipart/form-data; boundary='));
45+
$this->assertSame([], $parser->parse('--irrelevant', 'multipart/form-data; boundary=""'));
46+
$this->assertSame([], $parser->parse('--irrelevant', 'multipart/form-data; boundary=""; charset=utf-8'));
4147
}
4248

4349
public function testParse(): void

0 commit comments

Comments
 (0)