Skip to content

Commit

Permalink
Add an argument to allow arbitrary html (#100)
Browse files Browse the repository at this point in the history
* add argument to allow arbitrary html in email

* version bump

* added tests

* added a library and code to validate that email template content contains valid html, added tests

* add validation of html if passthrough option is used in get_html_email_body
  • Loading branch information
smcmurtry committed Aug 25, 2021
1 parent 0d8b9f4 commit c878a88
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 4 deletions.
3 changes: 3 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,7 @@ ignore_missing_imports = True
ignore_missing_imports = True

[mypy-phonenumbers.*]
ignore_missing_imports = True

[mypy-py_w3c.*]
ignore_missing_imports = True
15 changes: 12 additions & 3 deletions notifications_utils/template.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
from notifications_utils.take import Take
from notifications_utils.template_change import TemplateChange
from notifications_utils.sanitise_text import SanitiseSMS
from notifications_utils.validate_html import check_if_string_contains_valid_html


template_env = Environment(
Expand Down Expand Up @@ -351,6 +352,7 @@ def __init__(
logo_with_background_colour=False,
brand_name=None,
jinja_path=None,
allow_html=False,
):
super().__init__(template, values, jinja_path=jinja_path)
self.fip_banner_english = fip_banner_english
Expand All @@ -361,6 +363,7 @@ def __init__(
self.brand_colour = brand_colour
self.logo_with_background_colour = logo_with_background_colour
self.brand_name = brand_name
self.allow_html = allow_html
# set this again to make sure the correct either utils / downstream local jinja is used
# however, don't set if we are in a test environment (to preserve the above mock)
if "pytest" not in sys.modules:
Expand Down Expand Up @@ -390,7 +393,7 @@ def __str__(self):

return self.jinja_template.render(
{
"body": get_html_email_body(self.content, self.values),
"body": get_html_email_body(self.content, self.values, html="passthrough" if self.allow_html else "escape"),
"preheader": self.preheader,
"fip_banner_english": self.fip_banner_english,
"fip_banner_french": self.fip_banner_french,
Expand Down Expand Up @@ -423,6 +426,7 @@ def __init__(
brand_name=None,
logo_with_background_colour=None,
asset_domain=None,
allow_html=False,
):
super().__init__(
template,
Expand All @@ -442,6 +446,7 @@ def __init__(
self.brand_text = brand_text
self.brand_name = brand_name
self.asset_domain = asset_domain or "assets.notification.canada.ca"
self.allow_html = allow_html

def __str__(self):
return Markup(
Expand All @@ -451,6 +456,7 @@ def __str__(self):
self.content,
self.values,
redact_missing_personalisation=self.redact_missing_personalisation,
html="passthrough" if self.allow_html else "escape",
),
"subject": self.subject,
"from_name": escape_html(self.from_name),
Expand Down Expand Up @@ -724,14 +730,17 @@ def is_unicode(content):
return set(content) & set(SanitiseSMS.WELSH_NON_GSM_CHARACTERS)


def get_html_email_body(template_content, template_values, redact_missing_personalisation=False):
def get_html_email_body(template_content, template_values, redact_missing_personalisation=False, html="escape"):
if html == "passthrough" and check_if_string_contains_valid_html(template_content) != []:
# template_content contains invalid html, so escape it
html = "escape"

return (
Take(
Field(
template_content,
template_values,
html="escape",
html=html,
markdown_lists=True,
redact_missing_personalisation=redact_missing_personalisation,
)
Expand Down
27 changes: 27 additions & 0 deletions notifications_utils/validate_html.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from py_w3c.validators.html.validator import HTMLValidator


def check_if_string_contains_valid_html(content: str) -> list:
"""
Check if html snippet is valid - returns [] if html is valid.
This is only a partial document, so we expect the Doctype and title to be missing.
"""

allowed_errors = [
"Start tag seen without seeing a doctype first. Expected “<!DOCTYPE html>”.",
"Element “head” is missing a required instance of child element “title”.",
]

# the content can contain markdown as well as html - wrap the content in a div so it has a chance of being valid html
content_in_div = f"<div>{content}</div>"

val = HTMLValidator()
val.validate_fragment(content_in_div)

significant_errors = []
for error in val.errors:
if error["message"] in allowed_errors:
continue
significant_errors.append(error)

return significant_errors
2 changes: 1 addition & 1 deletion notifications_utils/version.py
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
__version__ = "45.0.0"
__version__ = "46.0.0"
# GDS version '34.0.1'
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"pytz==2021.1",
"smartypants==2.0.1",
"pypdf2==1.26.0",
"py_w3c==0.3.1",
# required by both api and admin
"awscli==1.19.58",
"boto3==1.17.58",
Expand Down
22 changes: 22 additions & 0 deletions tests/test_template_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,28 @@ def test_alt_text_with_no_fip_banner(logo_with_background_colour, brand_text, ex
assert expected_alt_text in email


@pytest.mark.parametrize("renderer", [HTMLEmailTemplate, EmailPreviewTemplate])
@pytest.mark.parametrize(
"content, allow_html, expected",
[
("Hello World", True, "Hello World"),
("Hello World", False, "Hello World"),
("<div>Hello World</div>", True, "<div>Hello World</div>"),
("<div>Hello World</div>", False, "&lt;div&gt;Hello World&lt;/div&gt;"),
],
)
def test_allow_html_works(content: str, allow_html: bool, expected: str, renderer):
email = str(
renderer(
{"content": content, "subject": ""},
fip_banner_english=True,
allow_html=allow_html,
)
)

assert expected in email


@pytest.mark.parametrize("complete_html", (True, False))
@pytest.mark.parametrize(
"branding_should_be_present, brand_logo, brand_text, brand_colour",
Expand Down
30 changes: 30 additions & 0 deletions tests/test_validate_html.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import pytest

from notifications_utils.validate_html import check_if_string_contains_valid_html


@pytest.mark.parametrize(
"good_content",
(
"<div>abc</div>",
'<div style="display: none;">abc</div>',
"""<div style="margin: 20px auto 30px auto;">
<img
src="http://google.com"
alt="alt text"
height="10"
width="10"
/>
</div>""",
"abc<div>abc</div>xyz",
),
)
def test_good_content_is_valid(good_content: str):
assert check_if_string_contains_valid_html(good_content) == []


@pytest.mark.parametrize(
"bad_content", ("<div>abc<div>", '<img src="http://google.com">', "abc<div>abc<div>xyz", '<div style=">abc</div>')
)
def test_bad_content_is_invalid(bad_content: str):
assert check_if_string_contains_valid_html(bad_content) != []

0 comments on commit c878a88

Please sign in to comment.