diff --git a/src/Form.php b/src/Form.php index 8a643126..a242fc3d 100644 --- a/src/Form.php +++ b/src/Form.php @@ -67,37 +67,23 @@ public static function isEmptyValue($value): bool } /** - * Escape reserved chars in the given string + * Return the given string unchanged. * - * The characters '.', ' ' and optionally brackets are converted to unused control characters - * File Separator: ␜, Group Separator: ␝, Record Separator: ␝, and Unit Separator: ␟ respectively. - * - * This is done because: - * PHP converts dots and spaces in form element names to underscores by default in the request data. - * For example, becomes $_REQUEST["a_b"]. - * - * And if an external variable name begins with a valid array syntax, trailing characters are silently ignored. - * For example, becomes $_REQUEST['foo']['bar']. - * See https://www.php.net/manual/en/language.variables.external.php - * - * @param string $string The string to escape - * @param bool $escapeBrackets Whether to escape brackets + * @param string $string + * @param bool $escapeBrackets * * @return string + * @deprecated Escaping element names must be explicitly done. This method does not escape + * anymore and will be removed in a future version. */ public static function escapeReservedChars(string $string, bool $escapeBrackets = true): string { - $escapeMap = [ - '.' => chr(28), // File Separator - ' ' => chr(29) // Group Separator - ]; - - if ($escapeBrackets) { - $escapeMap['['] = chr(30); // Record Separator - $escapeMap[']'] = chr(31); // Unit Separator - } + trigger_error(sprintf( + '%s is deprecated. Escaping element names must be explicitly done.', + __METHOD__ + ), E_USER_DEPRECATED); - return strtr($string, $escapeMap); + return $string; } public function getAction() diff --git a/src/FormElement/BaseFormElement.php b/src/FormElement/BaseFormElement.php index 5b3ff7dd..6bb3c48d 100644 --- a/src/FormElement/BaseFormElement.php +++ b/src/FormElement/BaseFormElement.php @@ -36,9 +36,6 @@ abstract class BaseFormElement extends BaseHtmlElement implements FormElement, V /** @var string Name of the element */ protected $name; - /** @var string Escaped name of the element */ - protected string $escapedName; - /** @var bool Whether the element is ignored */ protected $ignored = false; @@ -130,16 +127,6 @@ public function setName($name) { $this->name = $name; - // TODO: phpdoc always expressed that only strings are accepted, but some usages passed null despite that. - // The isset check is only a compatibility measure to not break BC. Remove once the interface actually - // requires a string. And don't ever dare to allow null (`?string`) due to this check!!!1 - if (isset($this->name)) { - // Name is always escaped - $this->escapedName = Form::escapeReservedChars($name); - } else { - $this->escapedName = ''; - } - return $this; } @@ -162,16 +149,6 @@ public function setIgnored($ignored = true) return $this; } - /** - * Get the escaped name of the element - * - * @return string - */ - public function getEscapedName(): string - { - return $this->escapedName; - } - public function isRequired() { return $this->required; @@ -324,7 +301,7 @@ public function hasBeenValidated() */ public function getNameAttribute() { - return $this->getEscapedName(); + return $this->getName(); } /** @@ -420,7 +397,7 @@ protected function getValueOfNameAttribute() return $name; } - return $this->getEscapedName(); + return $this->getName(); } public function getDecorators(): DecoratorChain diff --git a/src/FormElement/FieldsetElement.php b/src/FormElement/FieldsetElement.php index 978745f4..d8d11c08 100644 --- a/src/FormElement/FieldsetElement.php +++ b/src/FormElement/FieldsetElement.php @@ -116,14 +116,6 @@ protected function onElementRegistered(FormElement $element) $multiple = $element->isMultiple(); } - // This check is required as the getEscapedName method is not implemented in - // the FormElement interface - if ($element instanceof BaseFormElement) { - $name = $element->getEscapedName(); - } else { - $name = $element->getName(); - } - /** * We don't change the {@see BaseFormElement::$name} property of the element, * otherwise methods like {@see FormElements::populate() and {@see FormElements::getElement()} would fail, @@ -132,7 +124,7 @@ protected function onElementRegistered(FormElement $element) return sprintf( '%s[%s]%s', $this->getValueOfNameAttribute(), - $name, + $element->getName(), $multiple ? '[]' : '' ); }); diff --git a/src/FormElement/FileElement.php b/src/FormElement/FileElement.php index c2843686..d9ca8fd9 100644 --- a/src/FormElement/FileElement.php +++ b/src/FormElement/FileElement.php @@ -93,7 +93,7 @@ public function getValueAttribute() public function getNameAttribute() { - $name = $this->getEscapedName(); + $name = $this->getName(); return $this->isMultiple() ? ($name . '[]') : $name; } @@ -260,12 +260,12 @@ public function onRegistered(Form $form) $form->setAttribute('enctype', 'multipart/form-data'); } - $chosenFiles = (array) $form->getPopulatedValue('chosen_file_' . $this->getEscapedName(), []); + $chosenFiles = (array) $form->getPopulatedValue('chosen_file_' . $this->getName(), []); foreach ($chosenFiles as $chosenFile) { $this->files[$chosenFile] = null; } - $this->filesToRemove = (array) $form->getPopulatedValue('remove_file_' . $this->getEscapedName(), []); + $this->filesToRemove = (array) $form->getPopulatedValue('remove_file_' . $this->getName(), []); } protected function addDefaultValidators(ValidatorChain $chain): void diff --git a/src/FormElement/FormElements.php b/src/FormElement/FormElements.php index 0a81036b..f711af89 100644 --- a/src/FormElement/FormElements.php +++ b/src/FormElement/FormElements.php @@ -230,14 +230,6 @@ public function registerElement(FormElement $element) { $name = $element->getName(); - // This check is required as the getEscapedName method is not implemented in - // the FormElement interface - if ($element instanceof BaseFormElement) { - $escapedName = $element->getEscapedName(); - } else { - $escapedName = $name; - } - if ($name === null) { throw new InvalidArgumentException(sprintf( '%s expects the element to provide a name', @@ -247,13 +239,11 @@ public function registerElement(FormElement $element) $this->elements[$name] = $element; - if (array_key_exists($escapedName, $this->populatedValues)) { - $element->setValue( - $this->populatedValues[$escapedName][count($this->populatedValues[$escapedName]) - 1] - ); + if (array_key_exists($name, $this->populatedValues)) { + $element->setValue($this->populatedValues[$name][count($this->populatedValues[$name]) - 1]); if ($element instanceof ValueCandidates) { - $element->setValueCandidates($this->populatedValues[$escapedName]); + $element->setValueCandidates($this->populatedValues[$name]); } } @@ -369,7 +359,7 @@ public function getValues() public function populate($values) { foreach ($values as $name => $value) { - $this->populatedValues[Form::escapeReservedChars($name)][] = $value; + $this->populatedValues[$name][] = $value; if ($this->hasElement($name)) { $this->getElement($name)->setValue($value); } @@ -390,7 +380,6 @@ public function populate($values) */ public function getPopulatedValue($name, $default = null) { - $name = Form::escapeReservedChars($name); return isset($this->populatedValues[$name]) ? $this->populatedValues[$name][count($this->populatedValues[$name]) - 1] : $default; @@ -405,7 +394,6 @@ public function getPopulatedValue($name, $default = null) */ public function clearPopulatedValue($name) { - $name = Form::escapeReservedChars($name); if (isset($this->populatedValues[$name])) { unset($this->populatedValues[$name]); } diff --git a/src/FormElement/SelectElement.php b/src/FormElement/SelectElement.php index 588000f8..afe55731 100644 --- a/src/FormElement/SelectElement.php +++ b/src/FormElement/SelectElement.php @@ -110,7 +110,7 @@ public function getValueAttribute() public function getNameAttribute() { - $name = $this->getEscapedName(); + $name = $this->getName(); return $this->isMultiple() ? ($name . '[]') : $name; } diff --git a/tests/FormTest.php b/tests/FormTest.php index a047d737..0ff93b4b 100644 --- a/tests/FormTest.php +++ b/tests/FormTest.php @@ -3,7 +3,6 @@ namespace ipl\Tests\Html; use ipl\Html\Form; -use ipl\Html\FormElement\BaseFormElement; use ipl\Html\FormElement\FieldsetElement; use ipl\Html\Test\TestCase; use Psr\Http\Message\ServerRequestInterface; @@ -52,18 +51,6 @@ protected function assemble() $form->handleRequest($request2); } - public function testEscapeReservedChars(): void - { - // Reserved chars '.', ' ', '[' and ']' are escaped - $this->assertSame(Form::escapeReservedChars('foo.bar'), 'foo' . chr(28) . 'bar'); - $this->assertSame(Form::escapeReservedChars('foo bar'), 'foo' . chr(29) . 'bar'); - $this->assertSame(Form::escapeReservedChars('foo[bar]'), 'foo' . chr(30) . 'bar' . chr(31)); - - // The string remains the same - $this->assertSame(Form::escapeReservedChars('foo[bar]', false), 'foo[bar]'); - $this->assertSame(Form::escapeReservedChars('foo-bar123'), 'foo-bar123'); - } - public function testValidatePartialOnlyValidatesFieldsetChildrenWithAValue(): void { $fieldset = (new FieldsetElement('set')) @@ -103,38 +90,4 @@ public function testValidatePartialRecursesIntoNestedFieldsets(): void 'Nested fieldset child with a value is not validated during partial validation' ); } - - public function testFormElementsWithReservedCharsInName(): void - { - $form = new Form(); - - // Test that the element name gets escaped as soon as the name is set - /** @var BaseFormElement $el1 */ - $el1 = $form->createElement('text', 'foo.bar'); - $this->assertSame('foo' . chr(28) . 'bar', $el1->getEscapedName()); - - /** @var BaseFormElement $el2 */ - $el2 = $form->createElement('text', 'foo[bar]'); - $this->assertSame('foo' . chr(30) . 'bar' . chr(31), $el2->getEscapedName()); - - $el2->setName('foo bar'); - $this->assertSame('foo' . chr(29) . 'bar', $el2->getEscapedName()); - - $form->registerElement($el1); - $form->registerElement($el2); - - // Test that the element name is escaped when rendered - $this->assertHtml('', $form->getElement('foo.bar')); - $this->assertHtml('', $form->getElement('foo bar')); - - // Test for data population; Initial data is not escaped - $form->populate(['foo.bar' => 'foo', 'foo bar' => 'bar']); - $this->assertSame('foo', $form->getPopulatedValue('foo.bar')); - $this->assertSame('bar', $form->getPopulatedValue('foo bar')); - - // Test for data population; The POST data contains the escaped chars - $form->populate(['foo' . chr(28) . 'bar' => 'foo1', 'foo' . chr(29) . 'bar' => 'bar1']); - $this->assertSame('foo1', $form->getPopulatedValue('foo.bar')); - $this->assertSame('bar1', $form->getPopulatedValue('foo bar')); - } }