Commit 72357502 by Robert Raposa Committed by Clinton Blackburn

Add additional JavaScript context tags

parent b125922e
...@@ -235,6 +235,10 @@ class Rules(Enum): ...@@ -235,6 +235,10 @@ class Rules(Enum):
'mako-html-entities', 'mako-html-entities',
"HTML entities should be plain text or wrapped with HTML()." "HTML entities should be plain text or wrapped with HTML()."
) )
mako_unknown_context = (
'mako-unknown-context',
"The context could not be determined."
)
underscore_not_escaped = ( underscore_not_escaped = (
'underscore-not-escaped', 'underscore-not-escaped',
'Expressions should be escaped using <%- expression %>.' 'Expressions should be escaped using <%- expression %>.'
...@@ -1727,6 +1731,12 @@ class MakoTemplateLinter(BaseLinter): ...@@ -1727,6 +1731,12 @@ class MakoTemplateLinter(BaseLinter):
results: A list of results into which violations will be added. results: A list of results into which violations will be added.
""" """
if context == 'unknown':
results.violations.append(ExpressionRuleViolation(
Rules.mako_unknown_context, expression
))
return
# Example: finds "| n, h}" when given "${x | n, h}" # Example: finds "| n, h}" when given "${x | n, h}"
filters_regex = re.compile(r'\|([.,\w\s]*)\}') filters_regex = re.compile(r'\|([.,\w\s]*)\}')
filters_match = filters_regex.search(expression.expression) filters_match = filters_regex.search(expression.expression)
...@@ -1851,15 +1861,25 @@ class MakoTemplateLinter(BaseLinter): ...@@ -1851,15 +1861,25 @@ class MakoTemplateLinter(BaseLinter):
- index: the index of the context. - index: the index of the context.
- type: the context type (e.g. 'html' or 'javascript'). - type: the context type (e.g. 'html' or 'javascript').
""" """
contexts_re = re.compile(r""" contexts_re = re.compile(
<script.*?>| # script tag start r"""
</script>| # script tag end <script.*?> | # script tag start
<%static:require_module.*?>| # require js script tag start </script> | # script tag end
</%static:require_module> # require js script tag end""", re.VERBOSE | re.IGNORECASE) <%static:require_module.*?> | # require js script tag start
</%static:require_module> | # require js script tag end
<%block[ ]*name=['"]requirejs['"]\w*> | # require js tag start
</%block> # require js tag end
""",
re.VERBOSE | re.IGNORECASE
)
media_type_re = re.compile(r"""type=['"].*?['"]""", re.IGNORECASE) media_type_re = re.compile(r"""type=['"].*?['"]""", re.IGNORECASE)
contexts = [{'index': 0, 'type': 'html'}] contexts = [{'index': 0, 'type': 'html'}]
javascript_types = ['text/javascript', 'text/ecmascript', 'application/ecmascript', 'application/javascript'] javascript_types = [
'text/javascript', 'text/ecmascript', 'application/ecmascript', 'application/javascript',
'text/x-mathjax-config', 'json/xblock-args'
]
html_types = ['text/template']
for context in contexts_re.finditer(mako_template): for context in contexts_re.finditer(mako_template):
match_string = context.group().lower() match_string = context.group().lower()
if match_string.startswith("<script"): if match_string.startswith("<script"):
...@@ -1869,18 +1889,15 @@ class MakoTemplateLinter(BaseLinter): ...@@ -1869,18 +1889,15 @@ class MakoTemplateLinter(BaseLinter):
# get media type (e.g. get text/javascript from # get media type (e.g. get text/javascript from
# type="text/javascript") # type="text/javascript")
match_type = match_type.group()[6:-1].lower() match_type = match_type.group()[6:-1].lower()
if match_type not in javascript_types: if match_type in html_types:
# TODO: What are other types found, and are these really
# html? Or do we need to properly handle unknown
# contexts? Only "text/template" is a known alternative.
context_type = 'html' context_type = 'html'
elif match_type not in javascript_types:
context_type = 'unknown'
contexts.append({'index': context.end(), 'type': context_type}) contexts.append({'index': context.end(), 'type': context_type})
elif match_string.startswith("</script"): elif match_string.startswith("</"):
contexts.append({'index': context.start(), 'type': 'html'}) contexts.append({'index': context.start(), 'type': 'html'})
elif match_string.startswith("<%static:require_module"):
contexts.append({'index': context.end(), 'type': 'javascript'})
else: else:
contexts.append({'index': context.start(), 'type': 'html'}) contexts.append({'index': context.end(), 'type': 'javascript'})
return contexts return contexts
......
...@@ -464,9 +464,12 @@ class TestMakoTemplateLinter(TestLinter): ...@@ -464,9 +464,12 @@ class TestMakoTemplateLinter(TestLinter):
mako_template = textwrap.dedent(""" mako_template = textwrap.dedent("""
<%page expression_filter="h"/> <%page expression_filter="h"/>
## switch to JavaScript context
<script> <script>
{expression} {expression}
</script> </script>
## switch back to HTML context
${{x}}
""".format(expression=data['expression'])) """.format(expression=data['expression']))
linter._check_mako_file_is_safe(mako_template, results) linter._check_mako_file_is_safe(mako_template, results)
...@@ -477,7 +480,7 @@ class TestMakoTemplateLinter(TestLinter): ...@@ -477,7 +480,7 @@ class TestMakoTemplateLinter(TestLinter):
{'expression': '${x}', 'rule': Rules.mako_invalid_js_filter}, {'expression': '${x}', 'rule': Rules.mako_invalid_js_filter},
{'expression': '"${x | n, js_escaped_string}"', 'rule': None}, {'expression': '"${x | n, js_escaped_string}"', 'rule': None},
) )
def test_check_mako_expressions_in_require_js(self, data): def test_check_mako_expressions_in_require_module(self, data):
""" """
Test _check_mako_file_is_safe in JavaScript require context provides Test _check_mako_file_is_safe in JavaScript require context provides
appropriate violations appropriate violations
...@@ -487,9 +490,38 @@ class TestMakoTemplateLinter(TestLinter): ...@@ -487,9 +490,38 @@ class TestMakoTemplateLinter(TestLinter):
mako_template = textwrap.dedent(""" mako_template = textwrap.dedent("""
<%page expression_filter="h"/> <%page expression_filter="h"/>
## switch to JavaScript context (after next line)
<%static:require_module module_name="${{x}}" class_name="TestFactory"> <%static:require_module module_name="${{x}}" class_name="TestFactory">
{expression} {expression}
</%static:require_module> </%static:require_module>
## switch back to HTML context
${{x}}
""".format(expression=data['expression']))
linter._check_mako_file_is_safe(mako_template, results)
self._validate_data_rules(data, results)
@data(
{'expression': '${x}', 'rule': Rules.mako_invalid_js_filter},
{'expression': '"${x | n, js_escaped_string}"', 'rule': None},
)
def test_check_mako_expressions_in_require_js(self, data):
"""
Test _check_mako_file_is_safe in JavaScript require js context provides
appropriate violations
"""
linter = MakoTemplateLinter()
results = FileResults('')
mako_template = textwrap.dedent("""
<%page expression_filter="h"/>
# switch to JavaScript context
<%block name="requirejs">
{expression}
</%block>
## switch back to HTML context
${{x}}
""".format(expression=data['expression'])) """.format(expression=data['expression']))
linter._check_mako_file_is_safe(mako_template, results) linter._check_mako_file_is_safe(mako_template, results)
...@@ -497,12 +529,14 @@ class TestMakoTemplateLinter(TestLinter): ...@@ -497,12 +529,14 @@ class TestMakoTemplateLinter(TestLinter):
self._validate_data_rules(data, results) self._validate_data_rules(data, results)
@data( @data(
{'media_type': 'text/javascript', 'expected_violations': 0}, {'media_type': 'text/javascript', 'rule': None},
{'media_type': 'text/ecmascript', 'expected_violations': 0}, {'media_type': 'text/ecmascript', 'rule': None},
{'media_type': 'application/ecmascript', 'expected_violations': 0}, {'media_type': 'application/ecmascript', 'rule': None},
{'media_type': 'application/javascript', 'expected_violations': 0}, {'media_type': 'application/javascript', 'rule': None},
{'media_type': 'text/template', 'expected_violations': 1}, {'media_type': 'text/x-mathjax-config', 'rule': None},
{'media_type': 'unknown/type', 'expected_violations': 1}, {'media_type': 'json/xblock-args', 'rule': None},
{'media_type': 'text/template', 'rule': Rules.mako_invalid_html_filter},
{'media_type': 'unknown/type', 'rule': Rules.mako_unknown_context},
) )
def test_check_mako_expressions_in_script_type(self, data): def test_check_mako_expressions_in_script_type(self, data):
""" """
...@@ -513,14 +547,17 @@ class TestMakoTemplateLinter(TestLinter): ...@@ -513,14 +547,17 @@ class TestMakoTemplateLinter(TestLinter):
mako_template = textwrap.dedent(""" mako_template = textwrap.dedent("""
<%page expression_filter="h"/> <%page expression_filter="h"/>
# switch to JavaScript context
<script type="{}"> <script type="{}">
${{x | n, dump_js_escaped_json}} ${{x | n, dump_js_escaped_json}}
</script> </script>
## switch back to HTML context
${{x}}
""").format(data['media_type']) """).format(data['media_type'])
linter._check_mako_file_is_safe(mako_template, results) linter._check_mako_file_is_safe(mako_template, results)
self.assertEqual(len(results.violations), data['expected_violations']) self._validate_data_rules(data, results)
def test_check_mako_expressions_in_mixed_contexts(self): def test_check_mako_expressions_in_mixed_contexts(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