Make jinja templating more strict

This ensures we actually know when an AttributeError happens.

It also changes most external code to use the correct environment, rather than
simply creating a jinja2.Template, which wouldn't use the more tightened
environment.

qutebrowser/browser/shared.py

@@ -21,10 +21,8 @@
 
 import html
 
-import jinja2
-
 from qutebrowser.config import config
-from qutebrowser.utils import usertypes, message, log, objreg
+from qutebrowser.utils import usertypes, message, log, objreg, jinja
 
 
 class CallSuper(Exception):
@@ -137,7 +135,7 @@ def ignore_certificate_errors(url, errors, abort_on):
         assert error.is_overridable(), repr(error)
 
     if ssl_strict == 'ask':
-        err_template = jinja2.Template("""
+        err_template = jinja.environment.from_string("""
             Errors while loading <b>{{url.toDisplayString()}}</b>:<br/>
             <ul>
             {% for err in errors %}

qutebrowser/config/style.py

@@ -22,12 +22,11 @@
 import functools
 import collections
 
-import jinja2
 import sip
 from PyQt5.QtGui import QColor
 
 from qutebrowser.config import config
-from qutebrowser.utils import log, objreg
+from qutebrowser.utils import log, objreg, jinja
 
 
 @functools.lru_cache(maxsize=16)
@@ -40,7 +39,7 @@ def get_stylesheet(template_str):
     Return:
         The formatted template as string.
     """
-    template = jinja2.Template(template_str)
+    template = jinja.environment.from_string(template_str)
     return template.render(conf=config.val)
 
 

qutebrowser/mainwindow/mainwindow.py

@@ -24,13 +24,13 @@ import base64
 import itertools
 import functools
 
-import jinja2
 from PyQt5.QtCore import pyqtSlot, QRect, QPoint, QTimer, Qt
 from PyQt5.QtWidgets import QWidget, QVBoxLayout, QApplication, QSizePolicy
 
 from qutebrowser.commands import runners, cmdutils
 from qutebrowser.config import config
-from qutebrowser.utils import message, log, usertypes, qtutils, objreg, utils
+from qutebrowser.utils import (message, log, usertypes, qtutils, objreg, utils,
+                               jinja)
 from qutebrowser.mainwindow import tabbedbrowser, messageview, prompt
 from qutebrowser.mainwindow.statusbar import bar
 from qutebrowser.completion import completionwidget, completer
@@ -552,7 +552,7 @@ class MainWindow(QWidget):
                 "download is" if download_count == 1 else "downloads are"))
         # Process all quit messages that user must confirm
         if quit_texts or 'always' in config.val.confirm_quit:
-            msg = jinja2.Template("""
+            msg = jinja.environment.from_string("""
                 <ul>
                 {% for text in quit_texts %}
                    <li>{{text}}</li>

qutebrowser/utils/jinja.py

@@ -76,7 +76,17 @@ class Loader(jinja2.BaseLoader):
         return source, path, lambda: True
 
 
-def _guess_autoescape(template_name):
+class Environment(jinja2.Environment):
+
+    def __init__(self):
+        super().__init__(loader=Loader('html'),
+                         autoescape=self._guess_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
@@ -86,8 +96,7 @@ def _guess_autoescape(template_name):
         ext = template_name.rsplit('.', 1)[1]
         return ext in ['html', 'htm', 'xml']
 
-
+    def _resource_url(self, path):
-def resource_url(path):
         """Load images from a relative path (to qutebrowser).
 
         Arguments:
@@ -96,8 +105,7 @@ def resource_url(path):
         image = utils.resource_filename(path)
         return QUrl.fromLocalFile(image).toString(QUrl.FullyEncoded)
 
-
+    def _data_url(self, path):
-def data_url(path):
         """Get a data: url for the broken qutebrowser logo."""
         data = utils.read_file(path, binary=True)
         filename = utils.resource_filename(path)
@@ -105,11 +113,19 @@ def data_url(path):
         assert mimetype is not None, path
         return urlutils.data_url(mimetype[0], data).toString()
 
+    def getattr(self, obj, attribute):
+        """Override jinja's getattr() to be less clever.
+
+        This means it doesn't fall back to __getitem__, and it doesn't hide
+        AttributeError.
+        """
+        return getattr(obj, attribute)
+
 
 def render(template, **kwargs):
     """Render the given template and pass the given arguments to it."""
     try:
-        return _env.get_template(template).render(**kwargs)
+        return environment.get_template(template).render(**kwargs)
     except jinja2.exceptions.UndefinedError:
         log.misc.exception("UndefinedError while rendering " + template)
         err_path = os.path.join('html', 'undef_error.html')
@@ -118,7 +134,4 @@ def render(template, **kwargs):
         return err_template.format(pagename=template, traceback=tb)
 
 
-_env = jinja2.Environment(loader=Loader('html'), autoescape=_guess_autoescape)
+environment = Environment()
-_env.globals['resource_url'] = resource_url
-_env.globals['file_url'] = urlutils.file_url
-_env.globals['data_url'] = data_url

tests/unit/utils/test_jinja.py

@@ -55,6 +55,9 @@ def patch_read_file(monkeypatch):
         elif path == os.path.join('html', 'undef_error.html'):
             assert not binary
             return real_read_file(path)
+        elif path == os.path.join('html', 'attributeerror.html'):
+            assert not binary
+            return """{{ obj.foobar }}"""
         else:
             raise IOError("Invalid path {}!".format(path))
 
@@ -137,6 +140,12 @@ def test_undefined_function(caplog):
     assert caplog.records[0].msg == "UndefinedError while rendering undef.html"
 
 
+def test_attribute_error():
+    """Make sure accessing an unknown attribute fails."""
+    with pytest.raises(AttributeError):
+        jinja.render('attributeerror.html', obj=object())
+
+
 @pytest.mark.parametrize('name, expected', [
     (None, False),
     ('foo', False),
@@ -147,4 +156,4 @@ def test_undefined_function(caplog):
     ('foo.bar.html', True),
 ])
 def test_autoescape(name, expected):
-    assert jinja._guess_autoescape(name) == expected
+    assert jinja.environment._guess_autoescape(name) == expected