Skip to content

E
edx-platform
Commit a6ac06f2 by Robert Raposa
Options

Remove outdated safe template recommendations 

- Remove <%include> linting
- Remove dump_html_escaped_json
parent 34cecaee
Showing with 9 additions and 198 deletions
openedx/core/djangolib/js_utils.py
......@@ -74,45 +74,6 @@ def dump_js_escaped_json(obj, cls=EdxJSONEncoder):
return json_string
def dump_html_escaped_json(obj, cls=EdxJSONEncoder):
"""
JSON dumps and escapes objects that are safe to be embedded in HTML.
Use this for anything but strings (e.g. dicts, tuples, lists, bools, and
numbers). For strings, just used the default html filter.
Usage:
Used as follows in a Mako template inside a HTML, like in
a data attribute::
data-obj='${obj | n, dump_html_escaped_json}'
If you must use the cls argument, then use as follows::
data-obj='${dump_html_escaped_json(obj, cls) | n}'
Use the "n" Mako filter above. The default filter will include
html escaping in the future, and this ensures proper ordering of
these calls.
Ensure ascii in json.dumps (ensure_ascii=True) allows safe skipping of
Mako's default filter decode.utf8.
Arguments:
obj: The object soon to become an HTML escaped JSON string. The object
can be anything but strings (e.g. dicts, tuples, lists, bools, and
numbers).
cls (class): The JSON encoder class (defaults to EdxJSONEncoder).
Returns:
(string) Escaped encoded JSON.
"""
json_string = json.dumps(obj, ensure_ascii=True, cls=cls)
json_string = escape(json_string)
return json_string
def js_escaped_string(string_for_js):
"""
Mako filter that escapes text for use in a JavaScript string.
......
openedx/core/djangolib/tests/test_js_utils.py
......@@ -10,7 +10,7 @@ import HTMLParser
from mako.template import Template
from openedx.core.djangolib.js_utils import (
dump_js_escaped_json, dump_html_escaped_json, js_escaped_string
dump_js_escaped_json, js_escaped_string
)
......@@ -68,38 +68,6 @@ class TestJSUtils(TestCase):
escaped_json = dump_js_escaped_json(malicious_dict, cls=self.SampleJSONEncoder)
self.assertEquals(expected_custom_escaped_json, escaped_json)
def test_dump_html_escaped_json_escapes_unsafe_html(self):
"""
Test dump_html_escaped_json properly escapes &, <, and >.
"""
malicious_dict = {"</script><script>alert('hello, ');</script>": "</script><script>alert('&world!');</script>"}
expected_escaped_json = (
"{&#34;&lt;/script&gt;&lt;script&gt;alert(&#39;hello, &#39;);&lt;/script&gt;&#34;: "
"&#34;&lt;/script&gt;&lt;script&gt;alert(&#39;&amp;world!&#39;);&lt;/script&gt;&#34;}"
)
escaped_json = dump_html_escaped_json(malicious_dict)
self.assertEquals(expected_escaped_json, escaped_json)
def test_dump_html_escaped_json_with_custom_encoder_escapes_unsafe_html(self):
"""
Test dump_html_escaped_json first encodes with custom JSNOEncoder before escaping &, <, and >
The test encoder class should first perform the replacement of "<script>" with
"sample-encoder-was-here", and then should escape the remaining &, <, and >.
"""
malicious_dict = {
"</script><script>alert('hello, ');</script>":
self.NoDefaultEncoding("</script><script>alert('&world!');</script>")
}
expected_custom_escaped_json = (
"{&#34;&lt;/script&gt;&lt;script&gt;alert(&#39;hello, &#39;);&lt;/script&gt;&#34;: "
"&#34;&lt;/script&gt;sample-encoder-was-herealert(&#39;&amp;world!&#39;);&lt;/script&gt;&#34;}"
)
escaped_json = dump_html_escaped_json(malicious_dict, cls=self.SampleJSONEncoder)
self.assertEquals(expected_custom_escaped_json, escaped_json)
def test_js_escaped_string_escapes_unsafe_html(self):
"""
Test js_escaped_string escapes &, <, and >, as well as returns a unicode type
......@@ -139,17 +107,18 @@ class TestJSUtils(TestCase):
template = Template(
"""
<%!
import json
from openedx.core.djangolib.js_utils import (
dump_js_escaped_json, dump_html_escaped_json, js_escaped_string
dump_js_escaped_json, js_escaped_string
)
%>
<body>
<div
data-test-dict='${test_dict | n, dump_html_escaped_json}'
data-test-dict='${json.dumps(test_dict)}'
data-test-string='${test_dict["test_string"]}'
data-test-tuple='${test_dict["test_tuple"] | n, dump_html_escaped_json}'
data-test-number='${test_dict["test_number"] | n, dump_html_escaped_json}'
data-test-bool='${test_dict["test_bool"] | n, dump_html_escaped_json}'
data-test-tuple='${json.dumps(test_dict["test_tuple"])}'
data-test-number='${json.dumps(test_dict["test_number"])}'
data-test-bool='${json.dumps(test_dict["test_bool"])}'
></div>
<script>
......
scripts/safe_template_linter.py
......@@ -205,10 +205,6 @@ class Rules(Enum):
'mako-multiple-page-tags',
'A Mako template can only have one <%page> tag.'
)
mako_include_with_violations = (
'mako-include-with-violations',
'Must fix violations in included templates first.'
)
mako_unparsable_expression = (
'mako-unparsable-expression',
'The expression could not be properly parsed.'
......@@ -446,42 +442,6 @@ class FileResults(object):
self.directory = os.path.dirname(full_path)
self.is_file = os.path.isfile(full_path)
self.violations = []
self.includes = []
def resolve_include(self, include, include_results):
"""
Resolves potential include violations and determines if they are real or
not. For real violations, adds the violations into the violation list.
If the include_results is not a file, it will be considered a violation
and will require a disable pragma.
Arguments:
include: The include with the potential violation to be resolved.
include_results: The results of processing the include file.
"""
include_has_violations = (not include_results.is_file) or (len(include_results.violations) > 0)
if include_has_violations:
self.violations.append(include['potential_violation'])
def add_include(self, include_file, potential_violation):
"""
Adds an include which also must have no violations.
Arguments:
include_file: The include file as provided in an include.
potential_violation: Represents the violation of the include, if the
included file has any violations.
"""
include_full_path = os.path.normpath(self.directory + '/' + include_file)
self.includes.append({
'directory': os.path.dirname(include_full_path),
'file_name': os.path.split(include_full_path)[1],
'full_path': include_full_path,
'potential_violation': potential_violation,
})
def prepare_results(self, file_string):
"""
......@@ -494,8 +454,6 @@ class FileResults(object):
string_lines = StringLines(file_string)
for violation in self.violations:
violation.prepare_results(self.full_path, string_lines)
for include in self.includes:
include['potential_violation'].prepare_results(self.full_path, string_lines)
def print_results(self, options, out):
"""
......@@ -524,18 +482,6 @@ class MakoTemplateLinter(object):
_skip_mako_dirs = _skip_dirs
def __init__(self):
"""
Init method.
"""
self.results = {}
def supports_includes(self):
"""
Mako template linter supports linting includes.
"""
return True
def process_file(self, directory, file_name):
"""
Process file to determine if it is a Mako template file and
......@@ -552,13 +498,6 @@ class MakoTemplateLinter(object):
mako_file_full_path = os.path.normpath(directory + '/' + file_name)
results = FileResults(mako_file_full_path)
# don't process the same file twice. this could happen when we process
# files included by another file
if mako_file_full_path in self.results:
return self.results[mako_file_full_path]
self.results[mako_file_full_path] = results
if not results.is_file:
return results
......@@ -633,7 +572,6 @@ class MakoTemplateLinter(object):
if not has_page_default:
results.violations.append(RuleViolation(Rules.mako_missing_default))
self._check_mako_expressions(mako_template, has_page_default, results)
self._check_include_files(mako_template, results)
results.prepare_results(mako_template)
def _is_django_template(self, mako_template):
......@@ -675,30 +613,6 @@ class MakoTemplateLinter(object):
page_match = page_h_filter_regex.search(mako_template)
return page_match
def _check_include_files(self, mako_template, results):
"""
Checks if the Mako template includes other template files. If so, sets
up potential violations that will be checked later.
Arguments:
mako_template: The contents of the Mako template.
"""
regex = re.compile('<%include[^>]*file=(?:"|\')(.+?)(?:"|\')[^>]*/>')
for match in regex.finditer(mako_template):
include_file = match.group(1)
expression = {
'start_index': match.start(),
'end_index': match.end(),
'expression': match.group()
}
results.add_include(
include_file,
ExpressionRuleViolation(
Rules.mako_include_with_violations, expression
)
)
def _check_mako_expressions(self, mako_template, has_page_default, results):
"""
Searches for Mako expressions and then checks if they contain
......@@ -775,9 +689,6 @@ class MakoTemplateLinter(object):
results.violations.append(ExpressionRuleViolation(
Rules.mako_unwanted_html_filter, expression
))
elif (len(filters) == 2) and (filters[0] == 'n') and (filters[1] == 'dump_html_escaped_json'):
# {x | n, dump_html_escaped_json} is valid
pass
else:
results.violations.append(ExpressionRuleViolation(
Rules.mako_invalid_html_filter, expression
......@@ -948,12 +859,6 @@ class UnderscoreTemplateLinter(object):
_skip_underscore_dirs = _skip_dirs + ('test',)
def supports_includes(self):
"""
Underscore template linter does not lint includes.
"""
return False
def process_file(self, directory, file_name):
"""
Process file to determine if it is an Underscore template file and
......@@ -1114,13 +1019,6 @@ def _process_current_walk(current_walk, template_linters, options, out):
walk_file = os.path.normpath(walk_file)
for template_linter in template_linters:
results = template_linter.process_file(walk_directory, walk_file)
if template_linter.supports_includes():
for include in results.includes:
include_results = template_linter.process_file(
include['directory'],
include['file_name']
)
results.resolve_include(include, include_results)
results.print_results(options, out)
......
scripts/tests/templates/includes/include_safe.html deleted 100644 → 0
## Mako template with safe by default page expression
<%page expression_filter="h"/>
scripts/tests/templates/includes/include_unsafe.html deleted 100644 → 0
## Mako template with no safe by default page expression
scripts/tests/templates/test.html
<%include file="./includes/include_safe.html" args="message" />
<%include file="./includes/include_unsafe.html" args="message" />
<%include file="bad_file_name.html" />
## safe-lint: disable=mako-include-with-violations
<%include file="bad_file_name.html" />
## Mako template with no page directive for safe-by-default
scripts/tests/test_safe_template_linter.py
......@@ -36,9 +36,6 @@ class TestSafeTemplateLinter(TestCase):
output = out.getvalue()
self.assertIsNotNone(re.search('test\.html.*mako-missing-default', out.getvalue()))
self.assertIsNotNone(re.search('test\.html.*mako-include-with-violations.*include_unsafe\.html', out.getvalue()))
self.assertIsNotNone(re.search('test\.html.*mako-include-with-violations.*bad_file_name\.html', out.getvalue()))
self.assertIsNotNone(re.search('include_unsafe\.html.*mako-missing-default', out.getvalue()))
@ddt
......@@ -125,7 +122,6 @@ class TestMakoTemplateLinter(TestCase):
${'{{unbalanced-nested'}
${x | n}
${x | h}
${x | n, dump_html_escaped_json}
${x | n, dump_js_escaped_json}
""")
......@@ -247,7 +243,6 @@ class TestMakoTemplateLinter(TestCase):
${'{{unbalanced-nested'}
${x | n}
${x | h}
${x | n, dump_html_escaped_json}
${x | n, dump_js_escaped_json}
"${x-with-quotes | n, js_escaped_string}"
</script>
......@@ -255,7 +250,7 @@ class TestMakoTemplateLinter(TestCase):
linter._check_mako_file_is_safe(mako_template, results)
self.assertEqual(len(results.violations), 5)
self.assertEqual(len(results.violations), 4)
self.assertEqual(results.violations[0].rule, Rules.mako_invalid_js_filter)
self.assertEqual(results.violations[0].expression['expression'], "${x}")
self.assertEqual(results.violations[1].rule, Rules.mako_unparsable_expression)
......@@ -265,8 +260,6 @@ class TestMakoTemplateLinter(TestCase):
self.assertEqual(results.violations[2].expression['expression'], "${x | n}")
self.assertEqual(results.violations[3].rule, Rules.mako_invalid_js_filter)
self.assertEqual(results.violations[3].expression['expression'], "${x | h}")
self.assertEqual(results.violations[4].rule, Rules.mako_invalid_js_filter)
self.assertEqual(results.violations[4].expression['expression'], "${x | n, dump_html_escaped_json}")
def test_check_mako_expressions_in_require_js(self):
"""
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment