NativeEnv evals constants twice

[Qhesz]

Rendering in native mode performs literal_eval twice on constant values, which leads to unexpected behaviour.

Expected Behavior

>>> NativeEnvironment().from_string(r'''0.000{{ a }}''').render({"a":7})
0.0007
>>> NativeEnvironment().from_string(r'''0.000{{ 7 }}''').render({"a":7})
0.0007

Templates should behave the same way with constants as they do with dynamic variables.

Nothing should be eval'd before the entire template is finished.

Actual Behavior

>>> NativeEnvironment().from_string(r'''0.000{{ a }}''').render({"a":7})
0.07
>>> NativeEnvironment().from_string(r'''0.000{{ 7 }}''').render({"a":7})
0.0007
>>> NativeEnvironment().from_string(r'''{{b}}{{ a }}''').render({"a":7,"b":"0.000"})
0.0007

In the first case the constant 0.000 gets eval'd at template compile time, and truncated to 0.0 before the entire template has been rendered with dynamic data. The template gets eval'd a second time at render time, meaning the constant value has been double eval'd.

In the second case the whole template is constant, and gets eval'd in one go at compile time producing different results.

In the third case the whole template is dynamic, and gets eval'd in one go at render time.

Your Environment

• Python version: 3.7.6
• Jinja version: 2.11.1

Suggested solution

_output_const_repr() should not perform any evals, only render() should perform one eval at the very end. This seems to me like the only sane way to prevent weird double-eval issues like this one.

The preserve_quotes workaround in native_concat would no longer be needed with this change. It appears to have been a previous attempt at fixing this class of problems.

Fixing it this way would also fix the same problem in the underlying root_render_func api:

>>> t = NativeEnvironment().from_string(r'''{{ a }}''')
>>> list(t.root_render_func(t.new_context({"a":"7"}))) 
['7']
>>> t = NativeEnvironment().from_string(r'''{{"7"}}''')
>>> list(t.root_render_func(t.new_context({"a":"7"}))) 
[7]

[mkrizek]

This is very interesting. I have tested the suggested solution [0] (and removing preserve_quotes in addition [1]) and it works and the current test suite passes with the changes. This makes sense to me but I am not able to say if this could break any valid use case.

[0]

diff --git a/src/jinja2/nativetypes.py b/src/jinja2/nativetypes.py
index e0ad94d..4c89998 100644
--- a/src/jinja2/nativetypes.py
+++ b/src/jinja2/nativetypes.py
@@ -61,7 +61,7 @@ class NativeCodeGenerator(CodeGenerator):
         return value
 
     def _output_const_repr(self, group):
-        return repr(native_concat(group))
+        return repr("".join([str(v) for v in group]))
 
     def _output_child_to_const(self, node, frame, finalize):
         const = node.as_const(frame.eval_ctx)
diff --git a/tests/test_nativetypes.py b/tests/test_nativetypes.py
index 947168c..9da1b29 100644
--- a/tests/test_nativetypes.py
+++ b/tests/test_nativetypes.py
@@ -136,3 +136,11 @@ def test_concat_strings_with_quotes(env):
 def test_spontaneous_env():
     t = NativeTemplate("{{ true }}")
     assert isinstance(t.environment, NativeEnvironment)
+
+
+def test_1186(env):
+    from math import isclose
+    t = env.from_string("0.000{{ a }}")
+    result = t.render({"a":7})
+    assert isinstance(result, float)
+    assert isclose(result, 0.0007)

[1]

diff --git a/src/jinja2/nativetypes.py b/src/jinja2/nativetypes.py
index 4c89998..bba4f0a 100644
--- a/src/jinja2/nativetypes.py
+++ b/src/jinja2/nativetypes.py
@@ -1,4 +1,3 @@
-import types
 from ast import literal_eval
 from itertools import chain
 from itertools import islice
@@ -10,17 +9,14 @@ from .environment import Environment
 from .environment import Template
 
 
-def native_concat(nodes, preserve_quotes=True):
+def native_concat(nodes):
     """Return a native Python type from the list of compiled nodes. If
     the result is a single node, its value is returned. Otherwise, the
     nodes are concatenated as strings. If the result can be parsed with
     :func:`ast.literal_eval`, the parsed value is returned. Otherwise,
     the string is returned.
 
-    :param nodes: Iterable of nodes to concatenate.
-    :param preserve_quotes: Whether to re-wrap literal strings with
-        quotes, to preserve quotes around expressions for later parsing.
-        Should be ``False`` in :meth:`NativeEnvironment.render`.
+    :param nodes: Generator of nodes to concatenate.
     """
     head = list(islice(nodes, 2))
 
@@ -30,30 +26,17 @@ def native_concat(nodes, preserve_quotes=True):
     if len(head) == 1:
         raw = head[0]
     else:
-        if isinstance(nodes, types.GeneratorType):
-            nodes = chain(head, nodes)
-        raw = "".join([str(v) for v in nodes])
+        raw = "".join([str(v) for v in chain(head, nodes)])
 
     try:
-        literal = literal_eval(raw)
+        return literal_eval(raw)
     except (ValueError, SyntaxError, MemoryError):
         return raw
 
-    # If literal_eval returned a string, re-wrap with the original
-    # quote character to avoid dropping quotes between expression nodes.
-    # Without this, "'{{ a }}', '{{ b }}'" results in "a, b", but should
-    # be ('a', 'b').
-    if preserve_quotes and isinstance(literal, str):
-        quote = raw[0]
-        return f"{quote}{literal}{quote}"
-
-    return literal
-
 
 class NativeCodeGenerator(CodeGenerator):
     """A code generator which renders Python types by not adding
-    ``str()`` around output nodes, and using :func:`native_concat`
-    to convert complex strings back to Python types if possible.
+    ``str()`` around output nodes.
     """
 
     @staticmethod
@@ -101,9 +84,7 @@ class NativeTemplate(Template):
         """
         vars = dict(*args, **kwargs)
         try:
-            return native_concat(
-                self.root_render_func(self.new_context(vars)), preserve_quotes=False
-            )
+            return native_concat(self.root_render_func(self.new_context(vars)))
         except Exception:
             return self.environment.handle_exception()

[davidism]

Happy to consider a PR. This feature came from Ansible, so it would be helpful if @jctanner and @mkrizek could continue to review it.

I do remember looking at why intermediate steps were doing literal_eval when working on the preserve_quotes issue, but can't remember what conclusion I reached. At the time I think I left it in place because I figured it might have an effect on how native types flowed through the render.

[Qhesz]

As a side note, I was originally trying to implement something similar to ansible. The exact thing I was going for was "return the native type if and only if the template had exactly one expression." (I thought I'd to use a native root_render_function, if there is one node return it, otherwise do a normal template concat.)

[davidism]

I created PR #1190 with @mkrizek's patch.

As far as I can tell this doesn't negatively affect anything. Running native_concat on intermediate groups would return a native type, but that would either get concatenated as a string in a subsequent group, or it would be the final node and pass through native_concat anyway.

[davidism]

Just released 2.11.2 with this.

[mkrizek]

@Qhesz @davidism Thanks!

[Jean-Daniel]

This change breaks at least some valid ansible scripts:

I have a script with the following loop:

loop: "{{ [ 'dsa', 'rsa', 'ecdsa', 'ed25519' ] | product([ '', '.pub' ]) | list }}"

Which used to works, but now fails with the error:

fatal: FAILED! => {"msg": "Invalid data passed to 'loop', it requires a list, got this instead: [('dsa', ''), ('dsa', '.pub'), ('rsa', ''), ('rsa', '.pub'), ('ecdsa', ''), ('ecdsa', '.pub'), ('ed25519', ''), ('ed25519', '.pub')]. Hint: If you passed a list/dict of just one element, try adding wantlist=True to your lookup invocation or use q/query instead of lookup."}

I think this is cause by the fact that the template engine now returns a string instead of a list to the concat methods.

[mkrizek]

@Jean-Daniel Thank you for letting us know! I can't reproduce the issue with Jinja2 master branch and Ansible devel branch. I bet it's combination of Ansible and Jinja2 versions that cause your issue. Would you mind filing an issue in ansible/ansible so we can take it and figure it out there? Thanks!

[Jean-Daniel]

You have to make sure jinja2_native = true is enable in ansible config (which is not the default).

Nonetheless, I'm going try to write a minimal playbook and submit an issue in ansible.