Skip to content

Commit e17230b

Browse files
committed
Fix decoding parameters in multipart/formdata
1 parent 4e6859e commit e17230b

File tree

4 files changed

+26
-13
lines changed

4 files changed

+26
-13
lines changed

ChangeLog.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@ Web change log
33

44
## ?.?.? / ????-??-??
55

6+
* Fixed multipart/formdata field names and values decoding, these are not
7+
urlencoded. Ignore the specification which states `"`, `\r` and `\n`
8+
need to be escaped for consistency with PHP, see php/php-src#8206
9+
(@thekid)
10+
611
## 4.5.0 / 2024-09-15
712

813
* Fixed issue #119: Array parameter inconsistency with multipart/formdata

src/main/php/web/Headers.class.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,9 @@ protected function next($input, &$offset) {
157157
throw new FormatException('Unclosed string in parameter "'.$name.'"');
158158
}
159159
} while ('\\' === $input[$p++ - 1]);
160+
161+
// Ignore the spec stating `"`, `\r` and `\n` should be percent-encoded for
162+
// consistency with PHP, see https://github.com/php/php-src/issues/8206
160163
$parameters[$name]= strtr(substr($input, $offset + 1, $p - $offset - 2), ['\"' => '"']);
161164
$offset= $p + 1;
162165
} else {

src/main/php/web/io/Param.class.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,23 +41,23 @@ public static function from($name, $value) {
4141
* @throws lang.FormatException When input variable nesting level exceeded
4242
*/
4343
public static function parse($name, $chunks) {
44-
$encoded= '';
44+
$value= '';
4545
foreach ($chunks as $chunk) {
46-
$encoded.= $chunk;
46+
$value.= $chunk;
4747
}
4848

4949
// Trim leading spaces, replace '.' and ' ' inbetween with underscores, see
5050
// https://github.com/php/php-src/blob/php-8.4.0beta5/main/php_variables.c#L133
5151
if (false === ($p= strpos($name, '['))) {
52-
$self= new self(urldecode(strtr(ltrim($name, ' '), '. ', '__')));
52+
$self= new self(strtr(ltrim($name, ' '), '. ', '__'));
5353
} else if (substr_count($name, '[') <= self::$nesting) {
54-
$self= new self(urldecode(strtr(ltrim(substr($name, 0, $p), ' '), '. ', '__')));
55-
$self->array= urldecode(substr($name, $p));
54+
$self= new self(strtr(ltrim(substr($name, 0, $p), ' '), '. ', '__'));
55+
$self->array= substr($name, $p);
5656
} else {
5757
throw new FormatException('Cannot parse '.$name.' (nesting level > '.self::$nesting.')');
5858
}
5959

60-
$self->value= urldecode($encoded);
60+
$self->value= $value;
6161
return $self;
6262
}
6363

src/test/php/web/unittest/ParamsTest.class.php

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,7 @@ private function params() {
1515
yield ['key[0][a]', [['a' => 'value']]];
1616
yield ['key[a]', ['a' => 'value']];
1717
yield ['key[a][b]', ['a' => ['b' => 'value']]];
18-
yield ['key[%C3%BC]', ['ü' => 'value']];
19-
yield ['key[%C3%BC]', ['ü' => 'value']];
18+
yield ['key[ü]', ['ü' => 'value']];
2019
}
2120

2221
/** @return iterable */
@@ -30,8 +29,6 @@ private function merge() {
3029
['key.name', 'value'],
3130
['color name', 'green'],
3231
[' price', '12.99'],
33-
['gr%c3%bcn', 'de'],
34-
['de', 'gr%c3%bcn'],
3532
]];
3633
yield [[
3734
['accepted', 'true'],
@@ -107,10 +104,18 @@ public function merging_consistent_with_parse_str($params) {
107104
Assert::equals($expected, $outcome);
108105
}
109106

107+
#[Test, Values(['a&b', 'a+b', 'a%5B%5Db'])]
108+
public function name_used_as_is($name) {
109+
Assert::equals($name, Param::parse($name, ['value'])->name());
110+
}
111+
112+
#[Test, Values(['a&b', 'a+b', 'a%5B%5Db'])]
113+
public function value_used_as_is($value) {
114+
Assert::equals($value, Param::parse('key', [$value])->value());
115+
}
116+
110117
#[Test]
111118
public function encoded_brackets_in_offset() {
112-
113-
// IMO this should be ["[]" => "value"] but we'll be consistent with PHP here
114-
Assert::equals(['[' => ['value']], Param::parse('key[%5B%5D]', ['value'])->value());
119+
Assert::equals(['%5B%5D' => 'value'], Param::parse('key[%5B%5D]', ['value'])->value());
115120
}
116121
}

0 commit comments

Comments
 (0)