No Code Attached Yet bug
avatar angieradtke
angieradtke
24 Aug 2024

Since the last update, there has been an issue with the generation of HTML code in mail templates when using custom fields with defined overrides for formatting. The HTML code is output as text, and equal signs are being converted to "3D" (e.g., = becomes 3D). This seems to be related to quoted-printable encoding.

Steps to reproduce the issue

How I noticed this:

I use the com_contact component and have created custom fields for the email type. If I do not create any overrides, the key and value are displayed in a single string without line breaks, which is hard to read and a usability issue. To improve readability, I created an override for the email fields to format the output in the HTML mail:

/template/html/layouts/com_fields/field/myfieldoverride.php

Codeexample:

<p class="myfield">
<span class="label"><?php echo htmlentities($label, ENT_QUOTES | ENT_IGNORE, 'UTF-8'); ?>:</span>
<span class="value"><?php echo $value; ?></span>
</p>

This had been working perfectly until the last update. Now, it no longer works when sending the contact form to the recipient.

However, when looking at the copy sent to the sender, everything still works as expected. It seems that a filter is being applied, but only partially. There appear to be inconsistencies, as the mail copy functions as before, but the recipient’s version does not.

Expected result

html from the overrides should be output correctly

avatar angieradtke angieradtke - open - 24 Aug 2024
avatar joomla-cms-bot joomla-cms-bot - change - 24 Aug 2024
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 24 Aug 2024
avatar angieradtke angieradtke - change - 24 Aug 2024
Labels Added: bug
avatar angieradtke angieradtke - labeled - 24 Aug 2024
avatar richard67
richard67 - comment - 24 Aug 2024

The part of the issue that the copy to the sender is not handled in the same way as the mail to the receiver can be easily fixed. I've prepared the necessary change already and can make a PR.

The part of the issue with the custom field being escaped due to the recent security fix I could also fix, but I am not sure if we should do that. Will clarify.

avatar angieradtke
angieradtke - comment - 24 Aug 2024

I see: $this->unsafe_tags in the mailtemplate- function
Which tags are safe and which unsafe?

Maybe the safe ones are enough to format the template????

avatar angieradtke
angieradtke - comment - 24 Aug 2024

urrggg the whole customfields-block !
This is a big issue.
Now we are unable to format the output. :-(

Is it not possible to add the fields in a loop and escape only the field-> value ?

avatar brianteeman
brianteeman - comment - 24 Aug 2024

not at my pc to check but does this security fix break the new mail template functionality #43829

avatar angieradtke
angieradtke - comment - 24 Aug 2024

not at my pc to check but does this security fix break the new mail template functionality #43829

yeep

avatar brianteeman
brianteeman - comment - 24 Aug 2024

can someone please add release blocker to this issue as obviously we cant release 5.2 and promote a new feature if it doesnt work

avatar richard67 richard67 - change - 24 Aug 2024
Labels Added: Release Blocker
avatar richard67 richard67 - labeled - 24 Aug 2024
avatar angieradtke
angieradtke - comment - 24 Aug 2024

I think I have misunderstood something. @brianteeman @richard67
The problem is already there it came with the last security release

avatar richard67
richard67 - comment - 24 Aug 2024

I think I have misunderstood something. @brianteeman @richard67 The problem is already there it came with the last security release

@angieradtke Yes, that's clear. Question was if it also breaks the functionality from PR #43829 in the 5.2-dev branch. We understood that you referred to that.

avatar richard67
richard67 - comment - 24 Aug 2024

Anyway, I leave the release blocker label right now, will see if I can check that tomorrow.

avatar richard67
richard67 - comment - 25 Aug 2024

From what I can see after first tests is that the security fixes did not break the functionality added with PR #43829 to the 5.2-dev branch.

However there is something else broken. When I edit the mail template "Contacts: Contact Form Mail" for the first time and apply some changes, a new record for the en-GB language is saved in database. With that new record, the language string "COM_CONTACT_ENQUIRY_TEXT" is not translated in the email. I don't know yet if that issue was introduced by PR #43829 or if it was there before. It needs to check that with 5.2.0-alpha3 where both the security fixes and that PR were not included yet.

Resetting the mail template to the defaults does not work, see issue #39328 .

But for both I do not see any relation to the security fixes.

So I remove the release blocker label which I had added due to the suspicion that the security fixes which cause the issue here have also broken the functionality from PR #43829 .

What I can confirm is that the security fixes cause the issue reported here.

The easy fix would be to exclude the {CUSTOMFIELDS} email template tag from escaping, but that would open an attack vector, so I don't think we should do that.

Is it not possible to add the fields in a loop and escape only the field-> value ?

Due to the way how mail templates handle the custom fields this is not really possible.

avatar richard67 richard67 - change - 25 Aug 2024
Labels Removed: Release Blocker
avatar richard67 richard67 - unlabeled - 25 Aug 2024
avatar richard67
richard67 - comment - 25 Aug 2024

However, when looking at the copy sent to the sender, everything still works as expected. It seems that a filter is being applied, but only partially. There appear to be inconsistencies, as the mail copy functions as before, but the recipient’s version does not.

For this part of this issue see PR #43978 .

@angieradtke Could you test if that PR solved that part of your issue? That would really be appreciated.

avatar richard67
richard67 - comment - 25 Aug 2024

Meanwhile I've created another PR which solves this issue completely: See #43981 .

avatar richard67 richard67 - close - 25 Aug 2024
avatar richard67
richard67 - comment - 25 Aug 2024

Closing as having a pull request. Please test #43981 . Thanks in advance.

@angieradtke If that PR will not be accepted I will reopen this issue.

avatar richard67 richard67 - change - 25 Aug 2024
Status New Closed
Closed_Date 0000-00-00 00:00:00 2024-08-25 12:12:57
Closed_By richard67

Add a Comment

Login with GitHub to post a comment