Project

General

Profile

Actions

Bug #83733

open

FlashMessage renderers must not escape user content

Added by Alexander Schnitzler over 6 years ago. Updated over 4 years ago.

Status:
New
Priority:
Should have
Assignee:
-
Category:
-
Target version:
-
Start date:
2018-01-30
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
9
PHP Version:
7.0
Tags:
flashmessage, flashmessages
Complexity:
Is Regression:
Sprint Focus:

Description

With https://forge.typo3.org/issues/78477, the flash message handling unfortunatey became worse compared to 7.6 because it tried to do things the right way but it didn't have real life projects in mind.

These are the changes I struggle with a lot:

  • The FlashMessageRendererResolver resolves the Renderer by some hardcoded conditions. Backend, Frontend and CLI-Context. This is not only very breaking because the default rendering was the Bootstrap-Rendering in 7.6, it also takes away the possibility to configure the renderer. It's a slap in the face when migrating templates to 8.7.
  • Due to security reasons, the renderers escape all output-, but the FlashMessageViewHelper is still configure to not escape its output-. This takes the option from me of not escaping stuff for single flash messages in case I need to. This has to change. Renderers must not htmlspecialchar and let the template engine user decide. I understand that we do not want to open the doors for XSS by default (and we can do so by using sane defaults for the renderer configuration letting the flashmessage viewhelper escape by default, which can then be overridden) but there are cases, and it's many, where you want to disable escaping in flash messages.

Related issues 2 (0 open2 closed)

Related to TYPO3 Core - Task #78477: Refactoring of FlashMessage renderingClosedFrank N├Ągler2016-10-28

Actions
Related to TYPO3 Core - Feature #67679: Provide a GUI to create links in image captionsClosed2015-06-22

Actions
Actions #1

Updated by Alexander Schnitzler over 6 years ago

  • Related to Task #78477: Refactoring of FlashMessage rendering added
Actions #2

Updated by Benni Mack over 6 years ago

As this is going back and forth, this should be discussed properly before creating patches (changes of changes of changes again) IMHO. I do understand the use case, of course, but just saying...

Actions #3

Updated by Alexander Schnitzler over 6 years ago

Benni Mack wrote:

As this is going back and forth, this should be discussed properly before creating patches (changes of changes of changes again) IMHO. I do understand the use case, of course, but just saying...

I am in for a discussion. And let me emphasize again. It's not about reverting the changes. I am ok with safety first but I pretty much dislike when the core changes things and my only chance to get back previous features is to xclass while it's quite easy to make things configurable. If I can choose the escaping and the renderer in the view, maybe as arguments of the flash message view helper, I am totally ok with sane, secure defaults. But the current approach is a big no go, UX-wise.

Actions #4

Updated by Alexander Schnitzler over 6 years ago

  • Description updated (diff)
Actions #5

Updated by Moritz Ahl over 6 years ago

By the way: I had the exact same Problem in Image Captions where I want to place a link.
https://forge.typo3.org/issues/67679

Actions #6

Updated by Alexander Schnitzler over 4 years ago

  • Related to Feature #67679: Provide a GUI to create links in image captions added
Actions #7

Updated by Moritz Ahl over 4 years ago

We had a use case where we wanted to have a link inside our Flash Message. We solved this as follows:
1. We created a Custom Renderer "ListHtmlRenderer" which extends the "ListRenderer", doing the same except not htmlspecialcharing.
2. We created a custom flashMessageViewHelper which extends the Core one, registering an argument "allowHtml" which defaults to false. If set to true, it uses our ListHtmlRenderer instead of the ListRenderer.

(Btw., we did not have to extend/replace the FlashMessageRendererResolver because we only use our custom flashMessageViewhelper in the frontend context. Else, it would have been an even greater effort).

What I woud like to propose is to add such an argument like "allowHtml" to the core FlashMessageViewhelper who could then pass this option on to the respecive renderer which deals with it. For the argument description, we could add some kind of warning that one has to properly clean the input. So I'm - like Alexander - in favor of making this configurable.

Actions

Also available in: Atom PDF