Project

General

Profile

Actions

Bug #89434

closed

Epic #90106: [Extbase] Priority Bugfixes

Action argument values will get lost on validation error

Added by Florian Wessels over 4 years ago. Updated about 4 years 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


Related issues 6 (0 open6 closed)

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
Related to TYPO3 Core - Feature #70384: Make it possible to redirect (instead of forwarding) to referring action on validation errorClosed2015-10-05

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
Actions #1

Updated by Alexander Schnitzler over 4 years ago

  • Assignee set to Alexander Schnitzler
Actions #2

Updated by Alexander Schnitzler over 4 years ago

  • Is Regression set to Yes

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

Actions #3

Updated by Gerrit Code Review over 4 years 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

Actions #4

Updated by Gerrit Code Review over 4 years 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

Actions #5

Updated by Gerrit Code Review over 4 years 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

Actions #6

Updated by Gerrit Code Review over 4 years 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

Actions #7

Updated by Alexander Schnitzler over 4 years ago

  • Parent task set to #90106
Actions #8

Updated by Oliver Hader over 4 years ago

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

Actions #9

Updated by Oliver Hader over 4 years ago

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

Updated by Oliver Hader over 4 years ago

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

Updated by Gerrit Code Review over 4 years 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

Actions #12

Updated by Gerrit Code Review over 4 years 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

Actions #13

Updated by Gerrit Code Review over 4 years 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

Actions #14

Updated by Oliver Hader over 4 years ago

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

Updated by Oliver Hader over 4 years ago

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

Updated by Gerrit Code Review over 4 years 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

Actions #17

Updated by Torben Hansen about 4 years 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.

Actions #18

Updated by Felix Nagel about 4 years 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.

Actions #19

Updated by Torben Hansen about 4 years 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.

Actions #20

Updated by Felix Nagel about 4 years 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.

Actions #21

Updated by Alexander Schnitzler about 4 years ago

  • Target version set to 10 LTS
Actions #22

Updated by Gerrit Code Review about 4 years 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

Actions #23

Updated by Gerrit Code Review about 4 years 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

Actions #24

Updated by Gerrit Code Review about 4 years 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

Actions #25

Updated by Gerrit Code Review about 4 years 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

Actions #26

Updated by Gerrit Code Review about 4 years 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

Actions #27

Updated by Gerrit Code Review about 4 years 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

Actions #28

Updated by Gerrit Code Review about 4 years 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

Actions #29

Updated by Gerrit Code Review about 4 years 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

Actions #30

Updated by Gerrit Code Review about 4 years 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

Actions #31

Updated by Gerrit Code Review about 4 years 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

Actions #32

Updated by Gerrit Code Review about 4 years 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

Actions #33

Updated by Gerrit Code Review about 4 years 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

Actions #34

Updated by Gerrit Code Review about 4 years 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

Actions #35

Updated by Gerrit Code Review about 4 years 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

Actions #36

Updated by Gerrit Code Review about 4 years 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

Actions #37

Updated by Oliver Hader about 4 years ago

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

Updated by Benni Mack about 4 years ago

  • Status changed from Resolved to Closed
Actions #39

Updated by Simon Schaufelberger about 4 years ago

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

Updated by Simon Schaufelberger about 4 years ago

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

Updated by Simon Schaufelberger about 4 years ago

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

Updated by Simon Schaufelberger 4 months ago

  • Related to Feature #70384: Make it possible to redirect (instead of forwarding) to referring action on validation error added
Actions

Also available in: Atom PDF