Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Fields Mail Template HTML Breaks #43971

Closed
angieradtke opened this issue Aug 24, 2024 · 14 comments
Closed

Custom Fields Mail Template HTML Breaks #43971

angieradtke opened this issue Aug 24, 2024 · 14 comments

Comments

@angieradtke
Copy link
Contributor

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

@richard67
Copy link
Member

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.

@angieradtke
Copy link
Contributor Author

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????

@richard67
Copy link
Member

@angieradtke
Copy link
Contributor Author

angieradtke commented Aug 24, 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 ?

@brianteeman
Copy link
Contributor

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

@angieradtke
Copy link
Contributor Author

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

yeep

@brianteeman
Copy link
Contributor

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

@angieradtke
Copy link
Contributor Author

angieradtke commented Aug 24, 2024

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

@richard67
Copy link
Member

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.

@richard67
Copy link
Member

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

@richard67
Copy link
Member

richard67 commented Aug 25, 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.

@richard67
Copy link
Member

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.

@richard67
Copy link
Member

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

@richard67
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants