Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Unexpected TypeError when using boolean test inside numeric expression #755

Closed
eli-collins opened this issue Aug 16, 2017 · 6 comments
Closed
Milestone

Comments

@eli-collins
Copy link

@eli-collins eli-collins commented Aug 16, 2017

I got an unexpected TypeError when rendering the template string: {% set x = None %}{{ 2 * (x == "foo")}}.

My best guess is that something in this fragment is being incorrectly compiled to python code.

Full test case:

>>> from jinja2 import Environment
>>> template = Environment().from_string('{% set x = None %}{{ 2 * (x == "foo")}}')
>>> template.render()
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/usr/local/lib/python3.5/dist-packages/jinja2/environment.py", line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "/usr/local/lib/python3.5/dist-packages/jinja2/environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "<template>", line 1, in top-level template code
TypeError: unsupported operand type(s) for *: 'int' and 'NoneType'

Expected behavior is that render would return "0".

Some other similar template strings:

  • {% set x = "bar" %}{{ 2 * (x == "foo") }} unexpectedly renders "False", rather than "0".
  • {% set x = "foo" %}{{ 2 * (x == "foo") }} unexpectedly renders "False", rather than "2".
  • yet {% set x = "foo" %}{{ (x == "foo") }} correctly renders "True".
  • and {{ 2 * True }} correctly renders "2".

Tested this with jinja 2.0 - 2.9.6, under python 2.7 & 3.5.

@eli-collins eli-collins changed the title Unexpected TypeError when using boolean test inside numeric expression [BUG] Unexpected TypeError when using boolean test inside numeric expression Sep 18, 2017
@eli-collins

This comment has been minimized.

Copy link
Author

@eli-collins eli-collins commented Sep 18, 2017

I've tried tracking this down myself, but I'm having difficulty figuring out how to get a dump of the python source generated by the jinja compilation process (if that's in fact where the problem lies). I'd definitely appreciate any pointers.

@ricky-undeadcoders

This comment has been minimized.

Copy link

@ricky-undeadcoders ricky-undeadcoders commented Jun 2, 2018

So, I agree with the assessment made here, but if you use the "sameas" function this all works exactly as expected.

Examples:

from jinja2 import Template

this = Template('{% set x = "bar" %}{{ 2 * (x is sameas "foo") }}')
print(this.render())  # correctly prints 0

this = Template('{% set x = "foo" %}{{ 2 * (x is sameas "foo") }}')
print(this.render())  # correctly prints 2

this = Template('{% set x = "foo" %}{{ (x is sameas "foo") }}')
print(this.render())  # correctly prints True

this = Template('{{ 2 * True }}')
print(this.render()) # correctly prints 2

Likewise, both "is eq" and "is equalto" work as expected. It's just the double equals that returns the wrong value.

This doesn't fix this ticket, but if anyone is stuck, a good workaround is to use any of the comparison functions except "==".

@KevinMGranger

This comment has been minimized.

Copy link

@KevinMGranger KevinMGranger commented Nov 16, 2018

I think this might be the cause of the strange behavior I've found here.

from jinja2 import Environment
expr = Environment().compile_expression
ctx = dict(foo="baz", bar="")

# Recreating `bool(string)` behaves as you expect.

expr("(foo | length) != 0")(ctx) # True
expr("(bar | length) != 0")(ctx) # False

# Comparing them, however...

expr("((foo | length) != 0) != ((bar | length) != 0)")(ctx) # False

# Huh? Do I not remember how boolean logic works?

expr("foo != bar")(foo=True, bar=False) # True

# Let's try another syntax, which should be equivalent:

expr("((foo | length) != 0) is not eq((bar | length) != 0)")(ctx) # True

# Strange. Let's also try the ruby/js style of casting to a boolean:

expr("(not not foo) != (not not bar)")(ctx) # True

# And just out of curiosity, what if we applied the non-operator way to the length tests themselves?

expr("((foo | length) is not eq(0)) != ((bar | length) is not eq(0))")(ctx) # True

sameas is the only workaround that worked aside from adding more indirection in Ansible (due to the version of jinja used). Thank you!

@stevenorum

This comment has been minimized.

Copy link
Contributor

@stevenorum stevenorum commented Jan 11, 2019

I did a bit of investigation. It appears that somewhere in the process of converting a statement containing one or more boolean comparisons between a variable and another object into the underlying bytecode, it drops the parens around the comparisons, but it keeps them for entirely numeric operations. For example, it considers the first two equations below identical, but considers the third different (which at least is correct):

(i+1)*(j>3)
# ends up with the same bytecode, stack, etc. as
(i+1)*j>3
# but not the same as
i+1*j>3

It works correctly if the comparison is between two predefined constants.

Here's an incredibly simple test that should probably be added to the unit tests somewhere:

from jinja2.nativetypes import NativeEnvironment
assert 2 == NativeEnvironment().compile_expression("i*(j<5)")({"i":2,"j":3})

(I can do this later, but I have to get back to my actual job so I can't do that now.)

If the expression is too complicated to just break out to not use parentheses, you can get around this by breaking the boolean part into a separate variable (set temp=boolean result, then use temp in the larger equation).

Below is the script I used to determine this, if anyone who knows more about the parsing stack wants to dig further. (I'll try to look into it more later but I've never touched the internals of Jinja2 so I'll probably get bogged down before finding anything.)

#!/usr/bin/env python3

from jinja2 import Environment, BaseLoader
import inspect

def printmembers(x):
    members = inspect.getmembers(x)
    members.sort(key=lambda a: a[0])
    for m in members:
        print(m[0])
        print(m[1])
        print("")

def compare(expr, variables={}):
    variables = dict(variables)
    print("===============")
    print("VARS: {}".format(variables))
    print("EXPR: '{}'".format(expr))
    print("PYVAL: '{}'".format(eval(expr, variables)))
    print("J2VAL: '{}'".format(Environment(loader=BaseLoader()).from_string("{{{{{}}}}}".format(expr)).render(**variables)))
    new_expr = "qwer = ({})".format(expr)
    exec(new_expr, variables)
    print("PEXEC: '{}'".format(variables['qwer']))

exprs = ["t","f","i","t*i","i*t","f*i","i*f","j<3","j>3","(j<3)*i","i*(j<3)","(j>3)*i","j>3*i","i*(j>3)","i*j>3"]
variables = {"t":True,"f":False,"i":5,"j":2, "x":None}
for expr in exprs:
    compare(expr, variables)

comp = Environment().compile_expression
w_parens = comp("(i+1)*(j>3)")
wo_parens = comp("(i+1)*j>3")
members_w = inspect.getmembers(w_parens._template.root_render_func.__code__)
members_wo = inspect.getmembers(wo_parens._template.root_render_func.__code__)
members_w = {x[0]:x[1] for x in members_w}
members_wo = {x[0]:x[1] for x in members_wo}
for x in members_w:
    if x.startswith("co"):
        print("{} : {}".format(x, members_w[x] == members_wo[x]))
@stevenorum

This comment has been minimized.

Copy link
Contributor

@stevenorum stevenorum commented Jan 12, 2019

Did some more debugging, ending up with the following script showing that it lies in the compilation of the parser syntax tree into the underlying code to be executed.

Checked the code, looks like the compiler isn't adding parens when it generates that code in visit_Compare.

Created pull request #938 with the fix.

(first blob is script, second is its output)

#!/usr/bin/env python3

from jinja2 import Environment
from jinja2.parser import Parser
from jinja2.compiler import generate

def indentprint(s, openers="[{(", closers=")}]", newliners=","):
    indent = "  "
    depth = 0
    output = ""
    for c in s:
        if c == "\n":
            output += c + indent*depth
        elif c in newliners:
            output += c + "\n" + indent*depth
        elif c in openers:
            depth += 1
            output += c + "\n" + indent*depth
        elif c in closers:
            depth -= 1
            output += "\n" + indent*depth + c
        else:
            if c in " \t" and output[-1] in " \t\n":
                continue
            else:
                output += c
    output += "\n"
    print(output)

def print_resulting_src(src):
    e = Environment()
    print("EXPRESSION: '{}'".format(src))
    parser = Parser(e, "{{" + src + "}}").parse()
    print("Parsed syntax tree:")
    indentprint(parser.dump())
    print("Generated source:")
    print("="*20)
    print(generate(parser, e, None, None))
    print("="*20)

s1 = "(i+1)*(j>3)"
# Parser.parse will turn s1 into the correct syntax tree
# (with j>3 inside a Compare node)
# compiler.generate will turn that into (a longer form of): ((i + 1) * j > 3)
s2 = "(i+1)*j>3"
# Parser.parse will turn s2 into the correct syntax tree
# (with (i+1)*j inside a Mul node, which is then inside a Compare node)
# compiler.generate will turn that into (a longer form of): ((i + 1) * j) > 3
print_resulting_src(s1)
print_resulting_src(s2)
EXPRESSION: '(i+1)*(j>3)'
Parsed syntax tree:
nodes.Template(
  [
    nodes.Output(
      [
        nodes.Mul(
          nodes.Add(
            nodes.Name(
              'i',
              'load'
            ),
            nodes.Const(
              1
            )
          ),
          nodes.Compare(
            nodes.Name(
              'j',
              'load'
            ),
            [
              nodes.Operand(
                'gt',
                nodes.Const(
                  3
                )
              )
            ]
          )
        )
      ]
    )
  ]
)

Generated source:
====================
from __future__ import division, generator_stop
from jinja2.runtime import LoopContext, TemplateReference, Macro, Markup, TemplateRuntimeError, missing, concat, escape, markup_join, unicode_join, to_string, identity, TemplateNotFound, Namespace
name = None

def root(context, missing=missing, environment=environment):
    resolve = context.resolve_or_missing
    undefined = environment.undefined
    if 0: yield None
    l_0_i = resolve('i')
    l_0_j = resolve('j')
    pass
    yield to_string((((undefined(name='i') if l_0_i is missing else l_0_i) + 1) * (undefined(name='j') if l_0_j is missing else l_0_j) > 3))

blocks = {}
debug_info = '1=12'
====================
EXPRESSION: '(i+1)*j>3'
Parsed syntax tree:
nodes.Template(
  [
    nodes.Output(
      [
        nodes.Compare(
          nodes.Mul(
            nodes.Add(
              nodes.Name(
                'i',
                'load'
              ),
              nodes.Const(
                1
              )
            ),
            nodes.Name(
              'j',
              'load'
            )
          ),
          [
            nodes.Operand(
              'gt',
              nodes.Const(
                3
              )
            )
          ]
        )
      ]
    )
  ]
)

Generated source:
====================
from __future__ import division, generator_stop
from jinja2.runtime import LoopContext, TemplateReference, Macro, Markup, TemplateRuntimeError, missing, concat, escape, markup_join, unicode_join, to_string, identity, TemplateNotFound, Namespace
name = None

def root(context, missing=missing, environment=environment):
    resolve = context.resolve_or_missing
    undefined = environment.undefined
    if 0: yield None
    l_0_i = resolve('i')
    l_0_j = resolve('j')
    pass
    yield to_string((((undefined(name='i') if l_0_i is missing else l_0_i) + 1) * (undefined(name='j') if l_0_j is missing else l_0_j)) > 3)

blocks = {}
debug_info = '1=12'
====================
@davidism davidism added this to the 2.11.0 milestone Oct 7, 2019
@davidism davidism closed this Oct 7, 2019
@davidism

This comment has been minimized.

Copy link
Member

@davidism davidism commented Oct 7, 2019

Fixed by #938

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.