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

& in the title becomes & after sending #315

Open
gabn88 opened this issue Mar 26, 2020 · 6 comments
Open

& in the title becomes & after sending #315

gabn88 opened this issue Mar 26, 2020 · 6 comments

Comments

@gabn88
Copy link
Contributor

gabn88 commented Mar 26, 2020

I think the title is descriptive enough. I have send a newsletter with & in the title and it became & in everyone's mailbox. It seems unicode is not supported?

@studybuffalo
Copy link

Seems more likely that Django is escaping & to its equivalent HTML entity (&) because it is a reserved character. You could probably mark the title as safe in the impacted templates, which should resolve the issue (https://docs.djangoproject.com/en/3.0/ref/templates/builtins/#safe).

@gabn88
Copy link
Contributor Author

gabn88 commented Mar 26, 2020

Ah, but the strang thing is that I did not use a template. I was using a title (email subject) saved to the database, not a template.

@studybuffalo
Copy link

I would need to dig more, but I seem to recall the subject is rendered as a template, so the ampersand will still be escaped automatically by Django. You could override this template snippet to flag the content as safe. Relevant template here: https://github.com/dokterbob/django-newsletter/blob/master/newsletter/templates/newsletter/message/message_subject.txt

Whether this should be updated in the package is probably a call for the maintainers.

@woodz-
Copy link

woodz- commented Apr 30, 2020

I can confirm that if you do the overriding as suggested by @studybuffalo of that simple template like this:
{{ newsletter.title|safe }} - {{ message.title|safe }}
escaping is successfully suppressed.
It affects by the way the title of the newsletter and the title of the message

Question to the maintainer @dokterbob: would it be a risk to hard code it in the original template?
If the answer is yes, then we could unhesitatingly close this issue and commonly tell:
overriding is the way to go.

@dokterbob
Copy link
Collaborator

Similarly to the discussion on the escaping of recipient names here, email headers do need their proper encoding, or else there remains the possibility of header injection attacks (and other erroneous or unexpected behaviour).

The question is, whether Django already properly encodes subject headers, in which case, in deed, we can consider the template as safe. To which, the answer is: YES!
https://docs.djangoproject.com/id/3.0/_modules/django/core/mail/message/

As opposed to adding safe to the template, I would recommend wrapping it in a safe block. It might make sense to do the same for the text content of emails.

Ref: https://django.readthedocs.io/en/stable/ref/templates/language.html#for-template-blocks

@woodz-
Copy link

woodz- commented Jan 24, 2022

From your linked doc
one can see that django correctly escapes the subject via Header(val).encode().
So it's guaranteed to have automatically proper encoded header and no worries of injection attacks.

To your safe block suggestion: have you got some arguments why a block shall be used over the safe keyword?
Escaping the whole body text of an email might make sense for HTML only mails but not for plain text. So new issues arise when meticulously thinking outside the box.

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

No branches or pull requests

4 participants