Bug #89434

Epic #90106: [Extbase] Priority Bugfixes

Action argument values will get lost on validation error

Added by Florian Wessels almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Should have
Category:
Extbase
Target version:
Start date:
2019-10-16
Due date:
% Done:

100%

Estimated time:
TYPO3 Version:
10
PHP Version:
7.2
Tags:
Complexity:
Is Regression:
Yes
Sprint Focus:

Description

Action arguments and values will get lost when given object is invalid.

How to Reproduce

Create an ActionController with at least following actions:

/**
 * @param MyObject $myObject
 * @TYPO3\CMS\Extbase\Annotation\IgnoreValidation("myObject")
 */
public function editAction(MyObject $myObject): void
{
    $this->view->assign('myObject', $myObject);
}

/**
 * @param MyObject $myObject
 */
public function updateAction(MyObject $myObject): void
{
    $this->myObjectRepository->update($myObject);
    $this->redirect('index');
}

The object itself has a validator:

/**
 * @var float
 * @TYPO3\CMS\Extbase\Annotation\Validate("NumberRange", options={"minimum": -1000, "maximum": 100})
 */
protected $value = 0;

If you now try to edit an object and set the value to 1000, following exception will be thrown:

#1298012500 TYPO3\CMS\Extbase\Mvc\Controller\Exception\RequiredArgumentMissingException
Required argument "myObject" is not set for Vendor\Example\Controller\SampleController->edit.

Backtrace:

/var/www/example/web/typo3/sysext/extbase/Classes/Mvc/Controller/AbstractController.php line 399
/var/www/example/web/typo3/sysext/extbase/Classes/Mvc/Controller/ActionController.php line 162
/var/www/example/web/typo3/sysext/extbase/Classes/Mvc/Dispatcher.php line 81
/var/www/example/web/typo3/sysext/extbase/Classes/Mvc/Web/FrontendRequestHandler.php line 92
/var/www/example/web/typo3/sysext/extbase/Classes/Core/Bootstrap.php line 181
/var/www/example/web/typo3/sysext/extbase/Classes/Core/Bootstrap.php line 171

Possible Solution

Modify the renderHiddenReferrerFields method in typo3/sysext/fluid/Classes/ViewHelpers/FormViewHelper.php:

if (!empty($request->getArguments())) {
    $actionRequest += $this->getActionArguments($request, $actionName);
}

Add this method to the same class:

/**
 * Get all valid action arguments and their values.
 *
 * @param Request $request
 * @param string  $actionName
 *
 * @return array
 * @throws \ReflectionException
 */
protected function getActionArguments(Request $request, string $actionName): array
{
    $actionArguments = [];
    $reflectionMethod = new \ReflectionMethod($request->getControllerObjectName(), $actionName . 'Action');
    $reflectionParameters = $reflectionMethod->getParameters();

    foreach ($reflectionParameters as $reflectionParameter) {
        $argumentName = $reflectionParameter->getName();
        if ($request->hasArgument($argumentName)) {
            try {
                $actionArguments[$argumentName] = $request->getArgument($argumentName);
            } catch (NoSuchArgumentException $exception) {
                // Do nothing.
            }
        }
    }

    return $actionArguments;
}

Make sure to add missing use statements.


Files

extbase_arguments.zip (6.15 KB) extbase_arguments.zip Oliver Hader, 2020-02-02 18:20
Bildschirmfoto 2020-04-06 um 15.31.52.png (32.6 KB) Bildschirmfoto 2020-04-06 um 15.31.52.png Torben Hansen, 2020-04-06 15:32

Related issues

Related to TYPO3 Core - Bug #89937: Insecure Deserialization when knowing encryptionKey in ExtbaseClosed2019-12-13

Actions
Related to TYPO3 Core - Bug #88682: Switch to json_encode/json_decode for Extbase argumentsClosed2019-07-03

Actions
Related to TYPO3 Core - Task #88697: Remove dependencies of \TYPO3\CMS\Extbase\Mvc\Web\RequestClosedAlexander Schnitzler2019-07-06

Actions
Related to TYPO3 Core - Task #87550: Use controller classes when registering plugins/modulesClosedAlexander Schnitzler2019-01-25

Actions
Has duplicate TYPO3 Core - Bug #91134: The object of type "<Model>" given to update must be persisted already, but is new.Closed2020-04-19

Actions
#1

Updated by Alexander Schnitzler over 1 year ago

  • Assignee set to Alexander Schnitzler
#2

Updated by Alexander Schnitzler over 1 year ago

  • Is Regression set to Yes

This bug has been introduced with this commit: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61223

#3

Updated by Gerrit Code Review over 1 year ago

  • Status changed from New to Under Review

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/62866

#4

Updated by Gerrit Code Review over 1 year ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/62866

#5

Updated by Gerrit Code Review over 1 year ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/62866

#6

Updated by Gerrit Code Review over 1 year ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/62866

#7

Updated by Alexander Schnitzler over 1 year ago

  • Parent task set to #90106
#8

Updated by Oliver Hader over 1 year ago

Demo Extension for "object arguments"... created for v9, should show behavior in v10 as well..

#9

Updated by Oliver Hader over 1 year ago

  • Related to Bug #89937: Insecure Deserialization when knowing encryptionKey in Extbase added
#10

Updated by Oliver Hader over 1 year ago

  • Related to Bug #88682: Switch to json_encode/json_decode for Extbase arguments added
#11

Updated by Gerrit Code Review over 1 year ago

Patch set 1 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63151

#12

Updated by Gerrit Code Review over 1 year ago

Patch set 2 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63151

#13

Updated by Gerrit Code Review over 1 year ago

Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#14

Updated by Oliver Hader over 1 year ago

  • Related to Task #88697: Remove dependencies of \TYPO3\CMS\Extbase\Mvc\Web\Request added
#15

Updated by Oliver Hader over 1 year ago

  • Related to Task #87550: Use controller classes when registering plugins/modules added
#16

Updated by Gerrit Code Review over 1 year ago

Patch set 3 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63151

#17

Updated by Torben Hansen over 1 year ago

Can I somehow help to get this issue solved? I think we really need this working with the first release of TYPO3 10 LTS, since this may be a blocker for people to update. If there is not enough time to get Ollys WIP patch finished before the release, Patchset 3 (https://review.typo3.org/c/Packages/TYPO3.CMS/+/62866/3) should be merged and a new issue about hardening referring arguments could be created for later processing.

#18

Updated by Felix Nagel over 1 year ago

@Torban I experienced problems similar to what has been describe here with my blog extension but with the release of 10.3.0 everything works as expected again.

#19

Updated by Torben Hansen over 1 year ago

felix I'm working on current master and the problem still exist. I just tested your current master branch and also there I can reproduce the problem (see screenshot). Your comment form has HTML5 required fields, which "resolves "the problem with invalid form data on client side, because the user has to enter something in the form. But as soon as you work with pure Extbase validation (remove @required="required" from your form fields), the problem persist.

#20

Updated by Felix Nagel over 1 year ago

Oh boy, thanks for pointing this out. I just did a quick test as I need to rework the email functionality for 10.x anyway and did not run in it again. Seems I was wrong. I agree, this one is a MUST HAVE for the LTS release.

#21

Updated by Alexander Schnitzler over 1 year ago

  • Target version set to 10 LTS
#22

Updated by Gerrit Code Review over 1 year ago

Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#23

Updated by Gerrit Code Review over 1 year ago

Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#24

Updated by Gerrit Code Review over 1 year ago

Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#25

Updated by Gerrit Code Review over 1 year ago

Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#26

Updated by Gerrit Code Review over 1 year ago

Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#27

Updated by Gerrit Code Review over 1 year ago

Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#28

Updated by Gerrit Code Review over 1 year ago

Patch set 4 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63151

#29

Updated by Gerrit Code Review over 1 year ago

Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#30

Updated by Gerrit Code Review over 1 year ago

Patch set 5 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63151

#31

Updated by Gerrit Code Review over 1 year ago

Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#32

Updated by Gerrit Code Review over 1 year ago

Patch set 10 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#33

Updated by Gerrit Code Review over 1 year ago

Patch set 11 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#34

Updated by Gerrit Code Review over 1 year ago

Patch set 12 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#35

Updated by Gerrit Code Review over 1 year ago

Patch set 13 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63152

#36

Updated by Gerrit Code Review over 1 year ago

Patch set 6 for branch 9.5 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/c/Packages/TYPO3.CMS/+/63151

#37

Updated by Oliver Hader over 1 year ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100
#38

Updated by Benni Mack over 1 year ago

  • Status changed from Resolved to Closed
#39

Updated by Simon Schaufelberger over 1 year ago

  • Related to Bug #91134: The object of type "<Model>" given to update must be persisted already, but is new. added
#40

Updated by Simon Schaufelberger over 1 year ago

  • Related to deleted (Bug #91134: The object of type "<Model>" given to update must be persisted already, but is new.)
#41

Updated by Simon Schaufelberger over 1 year ago

  • Has duplicate Bug #91134: The object of type "<Model>" given to update must be persisted already, but is new. added

Also available in: Atom PDF