mirror of
https://github.com/GSA/notifications-admin.git
synced 2026-02-16 00:14:18 -05:00
Stop template subjects getting saved encoded
This is another problem with sanitising HTML, this with with it getting encoded where it shouldn’t be. The result was, when editing a template, the API getting sent an encoded rather than raw version of the subject (for letters and emails). The reason this happened is because BeautifulSoup behaves in an unexpected way. When accessing the `value` attribute of an `input` BeautifulSoup returns an unencoded version of the contents. In other words it returns what the user would see in the page, not what is in the raw HTML of the page. This meant that we were trying too hard to see an `&` instead of a `&` in our tests[1]. So things were actually working fine before adding the call to `escape_html`[2], but from the output of the tests it didn’t look like HTML was getting escaped. So this commit fixes the bug by removing the call to `escape_html` and adding a test that looks at the raw HTML, to complement the existing test which looks at just the `value` attribute. 1. Relevant test added here: https://github.com/alphagov/notifications-admin/pull/1178/files#diff-f2eb304b93cc383727c0ab7fc8fbd464R289 2. Call added here: https://github.com/alphagov/notifications-admin/pull/1178/files#diff-f0af582449ebf426f27f37e38f310057R252
This commit is contained in:
@@ -276,7 +276,7 @@ def edit_service_template(service_id, template_id):
|
||||
if form.process_type.data != template['process_type']:
|
||||
abort_403_if_not_admin_user()
|
||||
|
||||
subject = escape_html(form.subject.data) if hasattr(form, 'subject') else None
|
||||
subject = form.subject.data if hasattr(form, 'subject') else None
|
||||
new_template = get_template({
|
||||
'name': form.name.data,
|
||||
'content': form.template_content.data,
|
||||
|
||||
@@ -360,7 +360,7 @@ def test_should_show_interstitial_when_making_breaking_change(
|
||||
'name': "new name",
|
||||
'template_content': "hello lets talk about ((thing))",
|
||||
'template_type': 'email',
|
||||
'subject': 'reminder & ((name))',
|
||||
'subject': 'reminder \'" <span> & ((name))',
|
||||
'service': service_id,
|
||||
'process_type': 'normal'
|
||||
}
|
||||
@@ -377,12 +377,18 @@ def test_should_show_interstitial_when_making_breaking_change(
|
||||
|
||||
for key, value in {
|
||||
'name': 'new name',
|
||||
'subject': 'reminder & ((name))',
|
||||
'subject': 'reminder \'" <span> & ((name))',
|
||||
'template_content': 'hello lets talk about ((thing))',
|
||||
'confirm': 'true'
|
||||
}.items():
|
||||
assert page.find('input', {'name': key})['value'] == value
|
||||
|
||||
# BeautifulSoup returns the value attribute as unencoded, let’s make
|
||||
# sure that it is properly encoded in the HTML
|
||||
assert str(page.find('input', {'name': 'subject'})) == (
|
||||
"""<input name="subject" type="hidden" value="reminder '" <span> & ((name))"/>"""
|
||||
)
|
||||
|
||||
|
||||
def test_should_not_create_too_big_template(
|
||||
logged_in_client,
|
||||
@@ -450,7 +456,7 @@ def test_should_redirect_when_saving_a_template_email(
|
||||
template_id = fake_uuid
|
||||
name = "new name"
|
||||
content = "template <em>content</em> with & entity ((thing)) ((date))"
|
||||
subject = "subject"
|
||||
subject = "subject & entity"
|
||||
data = {
|
||||
'id': template_id,
|
||||
'name': name,
|
||||
|
||||
Reference in New Issue
Block a user