Fix HTML showing up on the breaking change page

41fa158635 added a proper Jinja filter
for formatting lists, which was better than the previous macro-based
solution.

It didn’t, however, account for HTML properly. It  did the default Jinja
thing of escaping everything. Since we render lists of placeholders
by putting HTML before and after each item, this didn’t work (the HTML
got escaped and appeared on the page).

So this commit does the escaping of HTML outside Jinja, in the
user-submitted bits of the input only, then passes the whole thing
through as a `Markup` instance which doesn’t get escaped by Jinja.
This commit is contained in:
Chris Hill-Scott
2017-03-06 12:54:27 +00:00
parent cd21b39d3f
commit 536d091d85
2 changed files with 12 additions and 4 deletions

View File

@@ -30,6 +30,7 @@ from notifications_python_client.errors import HTTPError
from notifications_utils import logging, request_id, formatters from notifications_utils import logging, request_id, formatters
from notifications_utils.clients.statsd.statsd_client import StatsdClient from notifications_utils.clients.statsd.statsd_client import StatsdClient
from notifications_utils.recipients import validate_phone_number, InvalidPhoneError from notifications_utils.recipients import validate_phone_number, InvalidPhoneError
from notifications_utils.field import escape_html
from pygments import highlight from pygments import highlight
from pygments.formatters.html import HtmlFormatter from pygments.formatters.html import HtmlFormatter
from pygments.lexers.javascript import JavascriptLexer from pygments.lexers.javascript import JavascriptLexer
@@ -363,17 +364,17 @@ def formatted_list(
if prefix_plural: if prefix_plural:
prefix_plural += ' ' prefix_plural += ' '
items = list(items) items = list(map(escape_html, items))
if len(items) == 1: if len(items) == 1:
return '{prefix}{before_each}{items[0]}{after_each}'.format(**locals()) return Markup('{prefix}{before_each}{items[0]}{after_each}'.format(**locals()))
elif items: elif items:
formatted_items = ['{}{}{}'.format(before_each, item, after_each) for item in items] formatted_items = ['{}{}{}'.format(before_each, item, after_each) for item in items]
first_items = separator.join(formatted_items[:-1]) first_items = separator.join(formatted_items[:-1])
last_item = formatted_items[-1] last_item = formatted_items[-1]
return ( return Markup((
'{prefix_plural}{first_items} {conjunction} {last_item}' '{prefix_plural}{first_items} {conjunction} {last_item}'
).format(**locals()) ).format(**locals()))
def nl2br(value): def nl2br(value):

View File

@@ -1,5 +1,6 @@
import pytest import pytest
from flask import Markup
from app import formatted_list from app import formatted_list
@@ -11,6 +12,12 @@ from app import formatted_list
([1], {'prefix': 'foo', 'prefix_plural': 'bar'}, 'foo 1'), ([1], {'prefix': 'foo', 'prefix_plural': 'bar'}, 'foo 1'),
([1, 2, 3], {'before_each': 'a', 'after_each': 'b'}, 'a1b, a2b and a3b'), ([1, 2, 3], {'before_each': 'a', 'after_each': 'b'}, 'a1b, a2b and a3b'),
([1, 2, 3], {'conjunction': 'foo'}, '1, 2 foo 3'), ([1, 2, 3], {'conjunction': 'foo'}, '1, 2 foo 3'),
(['&'], {'before_each': '<i>', 'after_each': '</i>'}, '<i>&amp;</i>'),
([1, 2, 3], {'before_each': '<i>', 'after_each': '</i>'}, '<i>1</i>, <i>2</i> and <i>3</i>'),
]) ])
def test_formatted_list(items, kwargs, expected_output): def test_formatted_list(items, kwargs, expected_output):
assert formatted_list(items, **kwargs) == expected_output assert formatted_list(items, **kwargs) == expected_output
def test_formatted_list_returns_markup():
assert isinstance(formatted_list([0]), Markup)