TYPO3 Core

This actually IS a problem of the framework.

The behavior makes it almost impossible to use forward with objects as arguments and a forum in the view. The whole object including objectStorage / childobjects is serialized/hashed. In My case the [__referrer][arguments] string is almost 2mb long and crashes the page renderer with <128mb memory_limit.

The behavior is simple wrong since the arguments should be hashed in a simple format.

How to reproduce:

Edit view -> save view -> forward to edit view

Helmut Hummel wrote:

Dimitri Lavrenük wrote:

This issue has been reported in a different context here:
https://forge.typo3.org/issues/57633

we should continue with the discussion there.

After digging a bit i found a huge security issue with this bug. You can get access to all properties of the object and all linked objects.
Typo3 Version: 6.2.3

I agree that this is not nice and should be changed. However this is only a security issue in very specific cases (which are not part of the core), so we can handle that bug in public.

This is in fact a specific issue. It does not affect the core but all the extensions that use forward with object arguments. With an update to Version 6.x an extension that was perfectly safe in 4.5 can provide big security holes without the developers knowing it.

Edit ection -> Save action -> forward to Edit action with the model object as argument

After the save action the form generates a [__referrer][arguments] hidden field with the whole object hashed as base64.

[...]

Why are you using a forward here anyway? To me it makes more sense to use a redirect for this example.

I think its not a good question why to use a functionality if it exists, but i can state my reasons.
- Redirect creates two requests to the server and forward does not. Since 6.x isnt really very fast it makes a different in 40%-50% of the loading time.
- Its not the best way to do validation, but sometimes you need to do some validation in the controller instead of a validator (since extbase cant handle partial validation). In that case you can not use the redirect function if you want to keep the submitted data

i could also state some more reasons, but that are the main ones.

Helmut Hummel wrote:

A solution is to not allow non persisted objects to be passed as arguments in a forward and convert entities to their identity before forwarding to the new action. (which is also pretty much what Flow does).
This makes a forward consistent to the redirect behavior.

I think it is not the best idea, since it changes the whole use of the forward function. Also at the moment you can not forward with ids instead of objects (you will get a mapping error), so the developers would need to change some working functionalities here. Here is my opinion what is the best to handle it:

Since the [__referrer][arguments] is used to track the user activity it should only contain the referrer of the initial request and not the forwarded one. In the case of a validation error of the next request you would get to the same point where you are now since the same request would be handled the same way and forwarded to the same action again

Dimitri Lavrenük wrote:

I think its not a good question why to use a functionality if it exists,

There is often a difference between the creators of a functionality intended it to be used for and the consumers of the functionality use it for.
The only way to find out why and how you are using it is asking ;)

but i can state my reasons.

Thanks.

- Redirect creates two requests to the server and forward does not. Since 6.x isnt really very fast it makes a different in 40%-50% of the loading time.

While I perfectly understand performance concerns, I would neglect this use case. Using a forward has the major downside that the requested URL is different to the action shown. In your example, you have a "/save" URL, while showing the edit form. This creates issues of its own.

- Its not the best way to do validation, but sometimes you need to do some validation in the controller instead of a validator (since extbase cant handle partial validation). In that case you can not use the redirect function if you want to keep the submitted data

Can you elaborate on that, I don't understand this use case.

i could also state some more reasons, but that are the main ones.

Please do so.

Helmut Hummel wrote:

A solution is to not allow non persisted objects to be passed as arguments in a forward and convert entities to their identity before forwarding to the new action. (which is also pretty much what Flow does).
This makes a forward consistent to the redirect behavior.

I think it is not the best idea, since it changes the whole use of the forward function.

True. It is difficult to change that for released branches.

Also at the moment you can not forward with ids instead of objects (you will get a mapping error), so the developers would need to change some working functionalities here.

If you know the internals, you can forward ids as arguments. This works:

$this->forward('edit', NULL, NULL, array('object' => (string)$object->getUid()));

The idea now is not to expose this as API, but change forward that the below code acts like the code above (which is currently not the case):

$this->forward('edit', NULL, NULL, array('object' => $object));

Here is my opinion what is the best to handle it:

Since the [__referrer][arguments] is used to track the user activity it should only contain the referrer of the initial request and not the forwarded one. In the case of a validation error of the next request you would get to the same point where you are now since the same request would be handled the same way and forwarded to the same action again

This will work in your described use case, as the edit action would get the persisted object as argument. For this case this would effectively lead to the same behavior as converting identities (objects) to their string representation (which is my proposal).

However if you forward other (non object) arguments, you will end up in a different state after failed validation because the forwarded arguments are not present any more. We can't do that.

Helmut Hummel wrote:

If you know the internals, you can forward ids as arguments.

This change¹ eases the pain a bit, by making the following possible:

$this->forward('edit', NULL, NULL, array('object' => $object->getUid()));

[1]http://review.typo3.org/32543

Helmut Hummel wrote:

Using a forward has the major downside that the requested URL is different to the action shown. In your example, you have a "/save" URL, while showing the edit form. This creates issues of its own.

This is true in most cases, however it does not matter if the functionality is based on ajax. Also you can have an indexAction where you decide what to show to the user without the need of changing the url

Can you elaborate on that, I don't understand this use case.

The use case is if you want to do custom validation or errorhandling in the controller itself. Easy example is if you validate the name/size of an uploaded file with an extbase validator and then upload it to a remote filestorage in the controller. If the upload failes you can forward back to the edit action and keep the posted data. Same goes if you send an email and in case there was an error while sending forward back to the edit form with some message.
If forward would be changed to only accept persisted data, you would need to do render the edit view with render() and thats really ugly.

i could also state some more reasons, but that are the main ones.

Please do so.

- storing a message in a static variable instead of session to show it on the forwarded view
- forwarding the index action to an action that fits the previlegies / some state of the user without the need of changing the url

Since the [__referrer][arguments] is used to track the user activity it should only contain the referrer of the initial request and not the forwarded one. In the case of a validation error of the next request you would get to the same point where you are now since the same request would be handled the same way and forwarded to the same action again

This will work in your described use case, as the edit action would get the persisted object as argument. For this case this would effectively lead to the same behavior as converting identities (objects) to their string representation (which is my proposal).

However if you forward other (non object) arguments, you will end up in a different state after failed validation because the forwarded arguments are not present any more. We can't do that.

actually this is exactly how extbase in typo3 4.5 (should be the same till 4.7) handles it and it works:
typo3\sysext\extbase\Classes\MVC\Controller\ActionController.php line 396
part of the error handling in the errorAction()

    $referrer = $this->request->getArgument('__referrer');
    $this->forward($referrer['actionName'], $referrer['controllerName'], $referrer['extensionName'], $this->request->getArguments());

Of course extbase has developed since then, but i stick to "never change a winning team".

I think we both agree that you cant output a serialized complex object, so the question is how to handle the forwarded data without changing the present functionality too much.

Hi!

I'm not so deep into Extbase in general, but regarding your forward/redirect argumentation I've to say that IMO it is simply wrong to deliver different content under the same URL.
Hence some arguments you posted for using forward are actually invalid, as it would violate this rule. (idempotent URLs)

- Redirect creates two requests to the server and forward does not. Since 6.x isnt really very fast it makes a different in 40%-50% of the loading time.

URL problem

- Its not the best way to do validation, but sometimes you need to do some validation in the controller instead of a validator (since extbase cant handle partial validation). In that case you can not use the redirect function if you want to keep the submitted data

I'm not much into extbase, so I don't understand that point.

- storing a message in a static variable instead of session to show it on the forwarded view

Accepted

- forwarding the index action to an action that fits the previlegies / some state of the user without the need of changing the url

So you have an action, but different "processors" for this action. Maybe the forward is not the right tool for this architecture then.

why not storing an "forward id" in the form and store all the arguments in the session, this would reduce the risk of manipulated data and dramtically reduces the number of objects, which are transferred ...