From 873fc59e77c2ec31a00cf7acb57288d6570c8d76 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Rie=C3=9F?= Date: Wed, 8 Jul 2026 08:26:11 +0200 Subject: [PATCH 1/6] Deprecate ValueCandidates Although BaseFormElement implements ValueCandidates for every element, PasswordElement is its only consumer, and the upcoming password rework no longer needs it. Mark the interface deprecated before removing the implementation so downstream code gets a heads-up. No replacement is provided. --- src/Contract/ValueCandidates.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Contract/ValueCandidates.php b/src/Contract/ValueCandidates.php index 1cd2d6fd..0d46072d 100644 --- a/src/Contract/ValueCandidates.php +++ b/src/Contract/ValueCandidates.php @@ -2,6 +2,11 @@ namespace ipl\Html\Contract; +/** + * @deprecated Only {@see \ipl\Html\FormElement\PasswordElement} ever used this even + * though {@see \ipl\Html\FormElement\BaseFormElement} implements it. No replacement + * will be provided. + */ interface ValueCandidates { /** From 81b53bd68ac110a657f96fa5daab2aac4bac9448 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Rie=C3=9F?= Date: Tue, 7 Jul 2026 14:48:10 +0200 Subject: [PATCH 2/6] Add hasBeenValidated to the form Add hasBeenValidated(), a pure check of whether $isValid has been set, so callers can peek at an already-known validity without ever triggering it. --- src/Form.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/Form.php b/src/Form.php index a242fc3d..6121bf68 100644 --- a/src/Form.php +++ b/src/Form.php @@ -311,6 +311,18 @@ public function isValid() return $this->isValid; } + /** + * Get whether the form has already been validated + * + * Unlike {@see isValid()}, this never triggers validation. + * + * @return bool + */ + public function hasBeenValidated() + { + return $this->isValid !== null; + } + /** * Validate all elements * From 0db40fa5f043fbb1f108d5f914a1782d9a8bddce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Rie=C3=9F?= Date: Wed, 8 Jul 2026 08:34:51 +0200 Subject: [PATCH 3/6] Call `setValue` in a loop instead of using `ValueCandidates` When an element is registered after values have been populated for its name, replay every accumulated value through setValue() in order instead of applying only the last one and stashing the full list via the removed ValueCandidates interface. Elements that only care about their final value are unaffected, as the last setValue() call wins. The reworked PasswordElement, in turn, can observe the seeded value and the request's own value as distinct assignments. --- src/FormElement/FormElements.php | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/FormElement/FormElements.php b/src/FormElement/FormElements.php index f711af89..1456c356 100644 --- a/src/FormElement/FormElements.php +++ b/src/FormElement/FormElements.php @@ -8,7 +8,6 @@ use ipl\Html\Contract\FormElement; use ipl\Html\Contract\FormElementDecoration; use ipl\Html\Contract\FormElementDecorator; -use ipl\Html\Contract\ValueCandidates; use ipl\Html\Form; use ipl\Html\FormDecoration\DecoratorChain; use ipl\Html\FormDecorator\DecoratorInterface; @@ -240,10 +239,8 @@ public function registerElement(FormElement $element) $this->elements[$name] = $element; 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[$name]); + foreach ($this->populatedValues[$name] as $value) { + $element->setValue($value); } } From d603f8086970bd0fe99dfc302d7b9360ffeb3413 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Rie=C3=9F?= Date: Tue, 7 Jul 2026 15:00:19 +0200 Subject: [PATCH 4/6] Rework password element Replace the ValueCandidates/event-listener based tracking with an element that manages its own state directly, now that setValue() is replayed with every populated value in order (see the preceding two commits): - Preserve the real value across DUMMYPASSWORD resubmissions instead of losing it in an array of candidates. - Track the first value received as a baseline to tell an actual change apart from a resubmission of the unchanged placeholder. - Skip validators entirely for the dummy placeholder; it isn't a real password, so a policy validator must not run against it. - Clear the field (rather than mask it) when a changed value fails validation on an actual submission, but keep echoing it back plainly during partial/autosubmit rounds so live typing isn't lost. - Only mask when there's an actual pre-existing secret seeded before the current request's own value (see $assignments); a plain form's password field (e.g. a login form) never has one and is now treated like an ordinary input instead of appearing pre-filled after submission. --- src/FormElement/PasswordElement.php | 168 ++++++++++++++++++++++------ 1 file changed, 131 insertions(+), 37 deletions(-) diff --git a/src/FormElement/PasswordElement.php b/src/FormElement/PasswordElement.php index 6ef78b49..20284f24 100644 --- a/src/FormElement/PasswordElement.php +++ b/src/FormElement/PasswordElement.php @@ -12,56 +12,150 @@ class PasswordElement extends InputElement protected $type = 'password'; - /** @var bool Status of the form */ - protected $isFormValid = true; + /** @var mixed Stored value, preserved across DUMMYPASSWORD submissions */ + protected mixed $storedValue = null; - /** @var bool Status indicating if the form got submitted */ - protected $isFormSubmitted = false; + /** + * Value from the very first setValue() call, used as the baseline to detect a change + * + * `null` is a legitimate baseline (no pre-existing secret); {@see $seeded} distinguishes that + * from "not captured yet". + * + * @var mixed + */ + protected mixed $initialValue = null; - protected function registerAttributeCallbacks(Attributes $attributes) + /** @var bool Whether {@see $initialValue} has been captured */ + protected bool $seeded = false; + + /** + * Number of setValue() calls received so far, dummy placeholder included + * + * A pre-existing value seeded before the current request's own value lands on top of it takes + * at least two calls; an element that only ever got the request's own value never does, and + * thus has nothing worth masking. + * + * @var int + */ + protected int $assignments = 0; + + /** @var ?Form The form this element is registered with */ + protected ?Form $form = null; + + public function getValue() { - parent::registerAttributeCallbacks($attributes); + if ($this->hasFailedValidation()) { + return null; + } - $attributes->registerAttributeCallback( - 'value', - function () { - if ( - $this->hasValue() - && count($this->getValueCandidates()) === 1 - && $this->isFormValid - && ! $this->isFormSubmitted - ) { - return self::DUMMYPASSWORD; - } - - if (parent::getValue() === self::DUMMYPASSWORD && count($this->getValueCandidates()) > 1) { - return self::DUMMYPASSWORD; - } - - return null; - } - ); + return $this->storedValue; } - public function onRegistered(Form $form) + /** + * Get whether a changed value has failed to validate + * + * Peeks at an already known form validity only, never asks for it. Calling {@see isValid()} + * would trigger validation, which may lead to an infinite loop. + * + * @return bool + */ + protected function hasFailedValidation(): bool { - $form->on(Form::ON_VALIDATE, function ($form) { - $this->isFormValid = $form->isValid(); - }); + $value = parent::getValue(); - $form->on(Form::ON_SENT, function ($form) { - $this->isFormSubmitted = $form->hasBeenSent(); - }); + return $value !== null + && $value !== static::DUMMYPASSWORD + && ( + $this->valid === false + || ($this->form !== null && $this->form->hasBeenValidated() && ! $this->form->isValid()) + ); } - public function getValue() + public function setValue($value) { + $this->assignments++; + parent::setValue($value); $value = parent::getValue(); - $candidates = $this->getValueCandidates(); - while ($value === self::DUMMYPASSWORD) { - $value = array_pop($candidates); + if ($value !== static::DUMMYPASSWORD) { + $this->storedValue = $value; + + if (! $this->seeded) { + $this->seeded = true; + $this->initialValue = $value; + } + } + + return $this; + } + + /** + * Validate the element using all registered validators + * + * The dummy placeholder isn't a real password, just a marker for "unchanged", so it's exempt + * from validators meant for freshly typed input. + * + * @return $this + */ + public function validate(): static + { + $this->ensureAssembled(); + + if (parent::getValue() === static::DUMMYPASSWORD) { + $this->valid = true; + $this->clearMessages(); + + return $this; } - return $value; + return parent::validate(); + } + + /** + * Get the value to render into the `value` attribute + * + * In order: + * - Nothing stored, or a changed value that just failed validation: `null`. + * - No request to compare against yet (e.g. an initial GET render): masked with + * {@see DUMMYPASSWORD}. + * - Nothing was seeded before this request's own value, i.e. no secret to protect (see + * {@see $assignments}): shown while still editing, cleared once actually submitted. + * - Otherwise: masked if unchanged or actually submitted, shown while still editing. + * + * @return ?string + */ + public function getValueAttribute() + { + if ($this->storedValue === null) { + return null; + } + + $stillEditing = $this->form !== null && ! $this->form->hasBeenSubmitted(); + if (! $stillEditing && $this->hasFailedValidation()) { + return null; + } + + if ($this->form === null || ! $this->form->hasBeenSent()) { + return static::DUMMYPASSWORD; + } + + if ($this->assignments < 2) { + return $stillEditing ? $this->storedValue : null; + } + + $unchanged = $this->storedValue === $this->initialValue; + + return $unchanged || ! $stillEditing ? static::DUMMYPASSWORD : $this->storedValue; + } + + public function onRegistered(Form $form) + { + parent::onRegistered($form); + $this->form = $form; + } + + protected function registerAttributeCallbacks(Attributes $attributes) + { + parent::registerAttributeCallbacks($attributes); + $attributes->registerAttributeCallback('value', $this->getValueAttribute(...), $this->setValue(...)); } } From 22295323668e8a25c8e0f5fdd773bc7d41ec217e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Rie=C3=9F?= Date: Wed, 8 Jul 2026 08:26:34 +0200 Subject: [PATCH 5/6] Remove ValueCandidates implementation Stop implementing the deprecated ValueCandidates interface in BaseFormElement and drop the $valueCandidates property along with its getter and setter. Nothing but PasswordElement ever relied on it, and that element is being reworked to track its own state. The interface itself is retained so any external implementers keep working. --- src/FormElement/BaseFormElement.php | 18 +----------------- 1 file changed, 1 insertion(+), 17 deletions(-) diff --git a/src/FormElement/BaseFormElement.php b/src/FormElement/BaseFormElement.php index 6bb3c48d..d49a64c6 100644 --- a/src/FormElement/BaseFormElement.php +++ b/src/FormElement/BaseFormElement.php @@ -8,7 +8,6 @@ use ipl\Html\Contract\DecorableFormElement; use ipl\Html\Contract\FormElement; use ipl\Html\Contract\FormElementDecoration; -use ipl\Html\Contract\ValueCandidates; use ipl\Html\Form; use ipl\Html\FormDecoration\DecoratorChain; use ipl\Html\FormDecoration\FormElementDecorationResult; @@ -22,7 +21,7 @@ * * @phpstan-import-type decoratorsFormat from DecoratorChain */ -abstract class BaseFormElement extends BaseHtmlElement implements FormElement, ValueCandidates, DecorableFormElement +abstract class BaseFormElement extends BaseHtmlElement implements FormElement, DecorableFormElement { use Messages; use Translation; @@ -51,9 +50,6 @@ abstract class BaseFormElement extends BaseHtmlElement implements FormElement, V /** @var mixed Value of the element */ protected $value; - /** @var array Value candidates of the element */ - protected $valueCandidates = []; - /** @var ?DecoratorChain All registered decorators */ protected ?DecoratorChain $decorators = null; @@ -249,18 +245,6 @@ public function setValue($value) return $this; } - public function getValueCandidates() - { - return $this->valueCandidates; - } - - public function setValueCandidates(array $values) - { - $this->valueCandidates = $values; - - return $this; - } - public function onRegistered(Form $form) { } From dce58e474fbffb988378640c9bd2fd0785aaa560 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Rie=C3=9F?= Date: Wed, 8 Jul 2026 09:08:32 +0200 Subject: [PATCH 6/6] Don't move a file twice when setValue is replayed FormElements::registerElement() now replays every populated value through setValue(). FileElement::setValue() moves the uploaded file to its destination, so a name that was populated more than once before the element is registered caused the same upload to be moved twice, throwing "Cannot retrieve stream after it has already been moved". Reuse the already-stored file when it is present on disk instead of moving the (now consumed) upload again. --- src/FormElement/FileElement.php | 10 ++++++ tests/FormElement/FileElementTest.php | 46 +++++++++++++++++++++++++++ 2 files changed, 56 insertions(+) diff --git a/src/FormElement/FileElement.php b/src/FormElement/FileElement.php index d9ca8fd9..1843b641 100644 --- a/src/FormElement/FileElement.php +++ b/src/FormElement/FileElement.php @@ -224,6 +224,16 @@ protected function storeFiles(UploadedFileInterface ...$files): array continue; } + if (isset($this->files[$name]) && is_file($path)) { + // The file has already been stored, e.g. because setValue() was called more than + // once for the same upload ({@see FormElements::registerElement()} replays every + // populated value). Re-moving the original upload would fail as its stream has + // already been moved. + $storedFiles[] = $this->files[$name]; + + continue; + } + $file->moveTo($path); // Re-created to ensure moveTo() still works if called externally diff --git a/tests/FormElement/FileElementTest.php b/tests/FormElement/FileElementTest.php index 8936959b..159052ff 100644 --- a/tests/FormElement/FileElementTest.php +++ b/tests/FormElement/FileElementTest.php @@ -143,6 +143,52 @@ protected function assemble() $this->assertNull($thirdForm->getValue('test')); } + public function testUploadedFileIsStoredOnlyOnceWhenSetValueIsReplayed() + { + // FormElements::registerElement() replays every populated value through setValue(). + // A file element that is populated more than once before it is registered must not + // attempt to move its already-moved upload a second time. + + $form = new class extends Form { + protected function assemble() + { + $this->addElement('file', 'test', [ + 'destination' => sys_get_temp_dir() + ]); + } + }; + + $file = new UploadedFile( + Utils::streamFor('lorem ipsum dolorem'), + 19, + 0, + 'test.txt', + 'text/plain' + ); + + $storedPath = implode(DIRECTORY_SEPARATOR, [sys_get_temp_dir(), sha1('test.txt')]); + @unlink($storedPath); + + // Two populate() calls for the same name accumulate two values, so registerElement() + // calls setValue() twice on the same upload once the element is added during assemble(). + $form->populate(['test' => $file]); + $form->populate(['test' => $file]); + + $form->ensureAssembled(); + + $this->assertSame( + 'test.txt', + $form->getElement('test')->getValue()->getClientFilename() + ); + + $this->assertSame( + 'lorem ipsum dolorem', + $form->getValue('test')->getStream()->getContents() + ); + + @unlink($storedPath); + } + public function testUploadedFileCanBeMoved() { $form = new class extends Form {