shopsys icon indicating copy to clipboard operation
shopsys copied to clipboard

Systematic escaping of e-mail variable replacements

Open PetrHeinz opened this issue 6 years ago • 6 comments

What is happening

In #1120, we escape specific variable replacements in e-mails. This is working solution, but it's relatively brittle - there is a possibility that developers extend the mail classes and forget to escape the values correctly.

Expected result

Changes are made so that variable replacements are escaped by default. HTML can still be used in variable replacements, but it must be explicitly allowed by the developer (similarly to the |raw filter in Twig).

I have two possible implementations in mind:

  1. The replacements are encapsulated in a data object VariableReplacement that has three properties: string $variableName, string $value, bool $escapeValue. $escapeValue is true by default. The property MessageData::$variablesReplacementsForBody is of type VariableReplacement[] and Mailer escapes the values according to the boolean property.
    The property MessageData::$variablesReplacementsForSubject can use the same type for consistency even though there is no need for escaping in the plaintext subject...

  2. We use a templating engine for e-mail templates (Twig, probably). This would provide a common solution to escaping in both regular templates and e-mail templates. The disadvantage would be harder editing of these templates in administration.
    Admins would either come into contact with Twig (and HTML) directly, which could be very problematic for them if they are not experienced with coding, or some kind of easy-to-use WYSIWYG Twig editor would have to be found and integrated (or implemented by us).
    The third option here would be that the templates would not longer be editable in the administration, which might make sense (store owners often want to send out nicely designed e-mails without webmail compatibility issues, which is a work for a coder/designer, not something to be done in a WYSIWYG).

PetrHeinz avatar Jun 17 '19 09:06 PetrHeinz

This issue has been automatically marked as stale because there was no activity within the last 4 months (and it is quite a long time). It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Oct 15 '19 13:10 stale[bot]

up

pk16011990 avatar Nov 04 '19 14:11 pk16011990

Hello @pk16011990, what variant you prefer?

pesektomas avatar Nov 18 '19 08:11 pesektomas

This issue has been automatically marked as stale because there was no activity within the last 4 months (and it is quite a long time). It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Mar 17 '20 09:03 stale[bot]

This issue has been automatically marked as stale because there was no activity within the last 4 months (and it is quite a long time). It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Jul 18 '20 05:07 stale[bot]

This issue has been automatically marked as stale because there was no activity within the last 4 months (and it is quite a long time). It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar Dec 19 '20 08:12 stale[bot]