Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Always autoescape jinja environments unless overridden
We were only rendering .html files before, so the old _guess_autoescape function
had the effect of always autoescaping .render() (from a file) but never
autoescaping .from_string(). However, most places using .from_string() actually
render (Qt-)HTML via jinja, so they should escape stuff!

Now, we always autoescape, except when the caller uses the
jinja.environment.no_autoescape() context manager, which places rendering
stylesheets now do.

This impacted:

- Confirm quit texts (no HTML here)
- config.py loading errors
  (where this was found because of an error containing - a <keybinding>)
- Certificate error prompts
  (should be fine from what I can tell, as the only user-controllable output is
  the hostname, which cannot contain HTML)
  • Loading branch information
The-Compiler committed Sep 16, 2017
1 parent 337d57b commit 3179e8c
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 24 deletions.
3 changes: 2 additions & 1 deletion qutebrowser/completion/completiondelegate.py
Expand Up @@ -194,7 +194,8 @@ def _get_textdoc(self, index):
color: {{ conf.colors.completion.match.fg }};
}
"""
template = jinja.environment.from_string(stylesheet)
with jinja.environment.no_autoescape():
template = jinja.environment.from_string(stylesheet)
self._doc.setDefaultStyleSheet(template.render(conf=config.val))

if index.parent().isValid():
Expand Down
3 changes: 2 additions & 1 deletion qutebrowser/config/config.py
Expand Up @@ -585,7 +585,8 @@ def _render_stylesheet(stylesheet):
"""Render the given stylesheet jinja template."""
# Imported here to avoid a Python 3.4 circular import
from qutebrowser.utils import jinja
template = jinja.environment.from_string(stylesheet)
with jinja.environment.no_autoescape():
template = jinja.environment.from_string(stylesheet)
return template.render(conf=val)


Expand Down
21 changes: 10 additions & 11 deletions qutebrowser/utils/jinja.py
Expand Up @@ -21,6 +21,7 @@

import os
import os.path
import contextlib
import traceback
import mimetypes
import html
Expand Down Expand Up @@ -82,21 +83,19 @@ class Environment(jinja2.Environment):

def __init__(self):
super().__init__(loader=Loader('html'),
autoescape=self._guess_autoescape,
autoescape=lambda _name: self._autoescape,
undefined=jinja2.StrictUndefined)
self.globals['resource_url'] = self._resource_url
self.globals['file_url'] = urlutils.file_url
self.globals['data_url'] = self._data_url

def _guess_autoescape(self, template_name):
"""Turn auto-escape on/off based on the file type.
Based on http://jinja.pocoo.org/docs/dev/api/#autoescaping
"""
if template_name is None or '.' not in template_name:
return False
ext = template_name.rsplit('.', 1)[1]
return ext in ['html', 'htm', 'xml']
self._autoescape = True

@contextlib.contextmanager
def no_autoescape(self):
"""Context manager to temporarily turn off autoescaping."""
self._autoescape = False
yield
self._autoescape = True

def _resource_url(self, path):
"""Load images from a relative path (to qutebrowser).
Expand Down
20 changes: 9 additions & 11 deletions tests/unit/utils/test_jinja.py
Expand Up @@ -146,14 +146,12 @@ def test_attribute_error():
jinja.render('attributeerror.html', obj=object())


@pytest.mark.parametrize('name, expected', [
(None, False),
('foo', False),
('foo.html', True),
('foo.htm', True),
('foo.xml', True),
('blah/bar/foo.html', True),
('foo.bar.html', True),
])
def test_autoescape(name, expected):
assert jinja.environment._guess_autoescape(name) == expected
@pytest.mark.parametrize('escape', [True, False])
def test_autoescape(escape):
if not escape:
with jinja.environment.no_autoescape():
template = jinja.environment.from_string("{{ v }}")
assert template.render(v='<foo') == '<foo'

template = jinja.environment.from_string("{{ v }}")
assert template.render(v='<foo') == '&lt;foo'

0 comments on commit 3179e8c

Please sign in to comment.