BUGFIX: Prevent committing values while going back and empty invalid resubmitted steps

Classes/Runtime/Domain/FormState.php

@@ -13,43 +13,43 @@
  * source code.
  */
 
-class FormState
+use Neos\Utility\Arrays;
+
+class FormState implements \JsonSerializable
 {
     /**
-     * @var mixed[][]
+     * @var FormStatePart[]
      */
     protected $parts = [];
 
     /**
-     * FormState constructor.
-     * @param mixed[][] $parts
+     * @param string $partName
+     * @param null|bool $finished
+     * @return bool
      */
-    public function __construct(array $parts = [])
+    public function hasPart(string $partName, ?bool $finished = null): bool
     {
-        foreach ($parts as $key => $value) {
-            // ensure only strings are used as partNames
-            if (is_string($key)) {
-                $this->parts[$key] = $value;
-            }
-        }
+        return array_key_exists($partName, $this->parts);
     }
 
     /**
      * @param string $partName
      * @return bool
      */
-    public function hasPart(string $partName): bool
+    public function isPartFinished(string $partName): bool
     {
-        return array_key_exists($partName, $this->parts);
+        return array_key_exists($partName, $this->parts) && ($this->parts[$partName]->isFinished());
     }
 
     /**
      * @param string $partName
      * @param mixed[] $partData
+     * @param bool $finished
      */
-    public function commitPart(string $partName, array $partData = []): void
+    public function commitPart(string $partName, array $partData = [], bool $finished = false): void
     {
-        $this->parts[$partName] = $partData;
+        $part = new FormStatePart($partData, $finished);
+        $this->parts[$partName] = $part;
     }
 
     /**
@@ -58,7 +58,19 @@ public function commitPart(string $partName, array $partData = []): void
      */
     public function getPart(string $partName): ?array
     {
-        return $this->parts[$partName] ?? null;
+        return $this->getPartData($partName);
+    }
+
+    /**
+     * @param string $partName
+     * @return mixed[]|null
+     */
+    public function getPartData(string $partName): ?array
+    {
+        if (array_key_exists($partName, $this->parts)) {
+            return $this->parts[$partName]->getData();
+        }
+        return null;
     }
 
     /**
@@ -77,10 +89,29 @@ function ($item) {
     }
 
     /**
-     * @return mixed[][]
+     * @return mixed[]
      */
-    public function getAllParts(): array
+    public function getData(): array
     {
-        return $this->parts;
+        $data = [];
+        foreach ($this->parts as $part) {
+            $data = Arrays::arrayMergeRecursiveOverrule(
+                $data,
+                $part->getData()
+            );
+        }
+        return $data;
+    }
+
+    public function jsonSerialize()
+    {
+        $json = [];
+        foreach ($this->parts as $name => $part) {
+            $json[$name] = [
+                'data' => $part->getData(),
+                'finished' => $part->isFinished()
+            ];
+        }
+        return $json;
     }
 }

Classes/Runtime/Domain/FormStatePart.php

@@ -0,0 +1,55 @@
+<?php
+declare(strict_types=1);
+
+namespace Neos\Fusion\Form\Runtime\Domain;
+
+/*
+ * This file is part of the Neos.Fusion.Form package.
+ *
+ * (c) Contributors of the Neos Project - www.neos.io
+ *
+ * This package is Open Source Software. For the full copyright and license
+ * information, please view the LICENSE file which was distributed with this
+ * source code.
+ */
+
+use Exception;
+
+class FormStatePart
+{
+    /**
+     * @var mixed[]
+     */
+    protected $data = [];
+
+    /**
+     * @var bool
+     */
+    protected $finished = false;
+
+    /**
+     * @param mixed[] $data
+     * @param bool $finished
+     */
+    public function __construct(array $data, bool $finished)
+    {
+        $this->data = $data;
+        $this->finished = $finished;
+    }
+
+    /**
+     * @return mixed[]
+     */
+    public function getData(): array
+    {
+        return $this->data;
+    }
+
+    /**
+     * @return bool
+     */
+    public function isFinished(): bool
+    {
+        return $this->finished;
+    }
+}

Classes/Runtime/Domain/FormStateService.php

@@ -33,7 +33,7 @@ class FormStateService
     public function unserializeState(string $string): FormState
     {
         $validatedState = $this->hashService->validateAndStripHmac($string);
-        return unserialize(base64_decode($validatedState), ['allowed_classes' => [FormState::class]]);
+        return unserialize(base64_decode($validatedState), ['allowed_classes' => [FormState::class, FormStatePart::class]]);
     }
 
     /**

Classes/Runtime/FusionObjects/MultiStepProcessImplementation.php

@@ -51,6 +51,11 @@ class MultiStepProcessImplementation extends AbstractFusionObject implements Pro
      */
     protected $targetSubProcessKey;
 
+    /**
+     * @var bool
+     */
+    protected $attemptFinishing = false;
+
     /**
      * Return reference to self during fusion evaluation
      * @return $this
@@ -69,12 +74,14 @@ public function handle(ActionRequest $request, array $data = []): void
         $this->data = $data;
 
         $internalArguments = $request->getInternalArguments();
+        $stateArgument = $internalArguments['__state'] ?? null;
+        $currentStep = $internalArguments['__current'] ?? null;
+        $targetStep = $internalArguments['__target'] ?? null;
+        $finishProcess = $internalArguments['__finish'] ?? null;
 
         // restore state
-        if (array_key_exists('__state', $internalArguments)
-            && $internalArguments['__state']
-        ) {
-            $this->state = $this->formStateService->unserializeState($internalArguments['__state']);
+        if ($stateArgument) {
+            $this->state = $this->formStateService->unserializeState($stateArgument);
         }
 
         // make the current `data` available to the context before sub processes are evaluated
@@ -84,46 +91,73 @@ public function handle(ActionRequest $request, array $data = []): void
         // evaluate the subprocesses this has to be done after the state was restored
         // as the current data may affect @if conditions
         $subProcesses = $this->getSubProcesses();
+        $subProcessKeys = array_keys($subProcesses);
+        $firstSubProcessKey = (string)reset($subProcessKeys);
 
-        // select current subprocess
-        if (array_key_exists('__current', $internalArguments)
-            && $internalArguments['__current']
-        ) {
-            $this->currentSubProcessKey = (string)$internalArguments['__current'];
-        } else {
-            $subProcessKeys = array_keys($subProcesses);
-            $this->currentSubProcessKey = (string)reset($subProcessKeys);
-        }
-
-        // store target subprocess, but only if it already was submitted
-        if (array_key_exists('__target', $internalArguments)
-            && $internalArguments['__target']
-            && $this->state
-            && $this->state->hasPart($internalArguments['__target'])
-        ) {
-            $this->targetSubProcessKey = (string)$internalArguments['__target'];
-        }
-
-        // find current and handle
-        $currentSubProcess = $subProcesses[$this->currentSubProcessKey];
-        $currentSubProcess->handle($request, $this->data);
 
-        if ($currentSubProcess->isFinished()) {
+        // find current subprocess and handle the request
+        if ($currentStep) {
+            $this->currentSubProcessKey = $currentStep;
+            $currentSubProcess = $subProcesses[$this->currentSubProcessKey];
+            $currentSubProcess->handle($request, $this->data);
             if (!$this->state) {
                 $this->state = new FormState();
             }
-
             $this->state->commitPart(
                 $this->currentSubProcessKey,
-                $currentSubProcess->getData()
+                $currentSubProcess->getData(),
+                $currentSubProcess->isFinished()
             );
         } else {
-            if ($this->targetSubProcessKey) {
-                $request->setArgument('__submittedArguments', []);
-                $request->setArgument('__submittedArgumentValidationResults', new Result());
+            $this->currentSubProcessKey = $firstSubProcessKey;
+            $currentSubProcess = $subProcesses[$this->currentSubProcessKey];
+        }
+
+        // find target subprocess, but only if it already was submitted
+        if ($targetStep && $this->state) {
+            if ($currentSubProcess->isFinished()) {
+                $this->targetSubProcessKey = $targetStep;
+            } else {
+                if ($this->state->hasPart($targetStep)) {
+                    $this->targetSubProcessKey = $targetStep;
+                }
             }
         }
 
+        // ensure no unfinished subprocesses are before the target
+        if ($this->state && $this->targetSubProcessKey) {
+            foreach ($subProcesses as $subProcessKey => $subProcesses) {
+                if ($subProcessKey === $this->targetSubProcessKey) {
+                    break;
+                }
+                if (!$this->state->isPartFinished($subProcessKey)) {
+                    $this->targetSubProcessKey = $subProcessKey;
+                }
+                if ($subProcessKey === $this->currentSubProcessKey) {
+                    break;
+                }
+            }
+        }
+
+        // determine target if none was defined yet
+        if (!$this->targetSubProcessKey) {
+            if (!$this->state) {
+                $this->targetSubProcessKey = $firstSubProcessKey;
+            } else {
+                foreach ($subProcesses as $subProcessKey => $subProcess) {
+                    if (!$this->state->isPartFinished($subProcessKey)) {
+                        $this->targetSubProcessKey = $subProcessKey;
+                        break;
+                    }
+                }
+            }
+        }
+
+        // is this an attempt to finish the process
+        if ($finishProcess) {
+            $this->attemptFinishing = true;
+        }
+
         // restore fusion context to the state before data was pushed
         $this->runtime->popContext();
     }
@@ -137,14 +171,13 @@ public function isFinished(): bool
             return false;
         }
 
-        if ($this->targetSubProcessKey) {
+        if (!$this->attemptFinishing) {
             return false;
         }
 
         $subProcesses = $this->getSubProcesses();
-
         foreach ($subProcesses as $subProcessKey => $subProcess) {
-            if ($this->state->hasPart($subProcessKey) == false) {
+            if ($this->state->isPartFinished($subProcessKey) == false) {
                 return false;
             }
         }
@@ -157,22 +190,12 @@ public function isFinished(): bool
     public function render(): string
     {
         $subProcesses = $this->getSubProcesses();
-        if ($this->targetSubProcessKey) {
-            $renderSubProcessKey = $this->targetSubProcessKey;
-        } else {
-            foreach ($subProcesses as $subProcessKey => $subProcess) {
-                if (!$this->state || !$this->state->hasPart($subProcessKey)) {
-                    $renderSubProcessKey = $subProcessKey;
-                    break;
-                }
-            }
-        }
 
-        if (isset($renderSubProcessKey) && $renderSubProcessKey && array_key_exists($renderSubProcessKey, $subProcesses)) {
-            $this->getRuntime()->pushContext('process', $this->prepareProcessInformation($renderSubProcessKey, $subProcesses));
+        if (isset($this->targetSubProcessKey) && $this->targetSubProcessKey && array_key_exists($this->targetSubProcessKey, $subProcesses)) {
+            $this->getRuntime()->pushContext('process', $this->prepareProcessInformation($this->targetSubProcessKey, $subProcesses));
             $hiddenFields = $this->runtime->evaluate($this->path . '/hiddenFields') ?? '';
             $header = $this->runtime->evaluate($this->path . '/header') ?? '';
-            $body = $subProcesses[$renderSubProcessKey]->render();
+            $body = $subProcesses[$this->targetSubProcessKey]->render();
             $footer = $this->runtime->evaluate($this->path . '/footer') ?? '';
             $this->getRuntime()->popContext();
             return $hiddenFields . $header . $body . $footer;
@@ -191,12 +214,7 @@ public function getData(): array
 
         // add subprocess data from state
         if ($this->state) {
-            foreach ($this->state->getAllParts() as $subProcessKey => $subProcessData) {
-                $data = Arrays::arrayMergeRecursiveOverrule(
-                    $data,
-                    $subProcessData
-                );
-            }
+            $data = Arrays::arrayMergeRecursiveOverrule($data, $this->state->getData());
         }
 
         return $data;

Classes/Runtime/FusionObjects/SingleStepProcessImplementation.php

@@ -21,6 +21,7 @@
 use Neos\Fusion\Form\Runtime\Domain\ProcessInterface;
 use Neos\Fusion\Form\Runtime\Domain\SchemaInterface;
 use Neos\Fusion\FusionObjects\AbstractFusionObject;
+use Neos\Utility\Arrays;
 
 class SingleStepProcessImplementation extends AbstractFusionObject implements ProcessInterface
 {
@@ -62,9 +63,15 @@ public function handle(ActionRequest $request, array $data = []): void
         if ($arguments || $internalArguments) {
             $data = $schema->convert($arguments);
             $result = $schema->validate($data);
-            $request->setArgument('__submittedArguments', $data);
+            $request->setArgument('__submittedArguments', $arguments);
             $request->setArgument('__submittedArgumentValidationResults', $result);
-            if (!$result->hasErrors()) {
+            if ($result->hasErrors()) {
+                foreach ($result->getFlattenedErrors() as $path => $error) {
+                    $data = Arrays::unsetValueByPath($data, $path);
+                }
+                $this->data = $data;
+                $this->isFinished = false;
+            } else {
                 $this->data = $data;
                 $this->isFinished = true;
             }