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

[4.4] Escape unsafe tags in mail copy to sender and notification to admin and don't escape custom email fields #43981

Merged

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Aug 25, 2024

Pull Request for Issue #43971 .

Alternative to PR #43978 .

Summary of Changes

This pull request (PR) removes the customfields mail templates tag from the unsafe tags to be escaped (part 1 of issue #43971 ).

Furthermore, it adds missing escaping of the remaining unsafe tags for the copy sent to the sender of email sent with a contact form (part 2 of issue #43971 ).

Finally, it fixes a similar issue as part 2 for user registration: In the email to the user, the unsafe mail template tags {...} are escaped, but in the notification mails sent out to all superadmins (i.e. users with user creating permissions and receiving system emails) they are not escaped.

The last 2 changes are the same as in PR #43978 , i.e. this PR here is equal to that PR plus the first change.

Testing Instructions

In principle this PR could be checked by code review.

But if you want to do real tests, follow the instructions below.

Code review

  1. In file components/com_contact/src/Controller/ContactController.php in line 274 just before the $mailer->send call the unsafe tags are added:
    $mailer->addUnsafeTags(['name', 'email', 'body', 'customfields']);
    This PR removes the 'customfields' from that call.
    Then a few lines below a new mailer is used for sending the copy. The mailer uses the same template, so the same unsafe tags should be added before line 283 with the $mailer->send call for the copy. That's currently missing. This PR adds that.

  2. In file components/com_users/src/Model/RegistrationModel.php in line 511 just before the $mailer->send call the unsafe tags are added:
    $mailer->addUnsafeTags(['username', 'password_clear', 'name']);
    Later below for the notification mail to the admins, unsafe tags should be added before line 558, but they are currently missing.
    Because for that email a different mail template is used which does not use the {password_clear} tag, the line should be:
    $mailer->addUnsafeTags(['username', 'name']);
    This PR adds that.

Test 1: Custom email fields with HTML layout override

See issue #43971 , section "Steps to reproduce the issue":

Use the com_contact component and create a custom field for the email type. Create an override for that email field to format the output in the HTML mail:

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

Code example:

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

Create a contact linked to a user and a single contact menu item for that contact, and make sure that the previously created custom email field is shown in the contact form and can be entered/changed there.

Go to the contact menu item on the site and send an email with the contact form, having entered a valid email address for the sender and the "Send a copy to yourself" check box checked.

Check the email to the recipient in an email client.

Test 2: Contact form email copy to sender

Prepare in the same way as for test 1.

Enable the "Send Copy to Submitter" option in the mail options of the menu item.

Go to the menu item on the site and send an email with the contact form, having entered a valid email address for the sender and the "Send a copy to yourself" check box checked.

Check the email copy to the sender in your email client.

Test 3: Notification email copy to superadmins on user registration

Enable user registration on your site.

Make sure you (superadmin) get notification emails.

Register a new user on the site.

Check the notification email for you (superadmin) in your email client.

Actual result BEFORE applying this Pull Request

Test 1: Custom email fields with HTML layout override

The custom field is escaped in the email to the recipient.

Test 2: Contact form email copy to sender

The custom field is not escaped in the email copy to the sender.

Test 3: Notification email copy to superadmins on user registration

Works as expected.

Expected result AFTER applying this Pull Request

Test 1: Custom email fields with HTML layout override

The custom field is not escaped in the email to the recipient.

Test 2: Contact form email copy to sender

Still works as expected. The custom field is still not escaped in the email copy to the sender.

Test 3: Notification email copy to superadmins on user registration

Still works as expected.

Link to documentations

Please select:

  • No documentation changes for docs.joomla.org needed

  • No documentation changes for manual.joomla.org needed

@angieradtke
Copy link
Contributor

I have tested this item ✅ successfully on df30f7c

Works like expected .-), Thanks


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43981.

@SniperSister
Copy link
Contributor

I have tested this item ✅ successfully on 6815bff


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43981.

1 similar comment
@SniperSister
Copy link
Contributor

I have tested this item ✅ successfully on 6815bff


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43981.

@richard67
Copy link
Member Author

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43981.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 26, 2024
@MacJoom MacJoom added this to the Joomla! 4.4.8 milestone Aug 26, 2024
@MacJoom MacJoom merged commit 2d7667e into joomla:4.4-dev Aug 26, 2024
4 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 26, 2024
@MacJoom
Copy link
Contributor

MacJoom commented Aug 26, 2024

Thank you for the fix!

@richard67 richard67 deleted the 4.4-dev-fix-mail-templates-2024-08-25 branch August 26, 2024 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants