Edgewall Software

Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#13244 closed defect (fixed)

Passing Jinja2 context to callable instance as first argument when the callable instance has __getattr__ method since Jinja2 2.11.0 — at Version 12

Reported by: Jun Omae Owned by: Jun Omae
Priority: normal Milestone: 1.4.1
Component: rendering Version:
Severity: normal Keywords: jinja2
Cc: Branch:
Release Notes:

Skip Jinja2 context instance in Href.__call__ to fix wrong link generated from Jinja2 template since Jinja2 2.11.0.

API Changes:
Internal Changes:

Description (last modified by Jun Omae)

From comment:78:ticket:12130

The callable instance is Href instance.

See also

Jinja2 2.11.0

>>> import jinja2, sys
>>> jinja2.__version__
'2.11.0'
>>> from jinja2 import Template
>>> from trac.web.href import Href
>>> href = Href('/trac.cgi')
>>> Template('{{ href() }}').render(href=href)
"/trac.cgi/%3CContext%20%7B'range'%3A%20%3Cclass%20'range'%3E%2C%20'dict'%3A%20%3Cclass%20'dict'%3E%2C%20'lipsum'%3A%20%3Cfunction%20generate_lorem_ipsum%20at%200x7f0386031310%3E%2C%20'cycler'%3A%20%3Cclass%20'jinja2.utils.Cycler'%3E%2C%20'joiner'%3A%20%3Cclass%20'jinja2.utils.Joiner'%3E%2C%20'namespace'%3A%20%3Cclass%20'jinja2.utils.Namespace'%3E%2C%20'href'%3A%20%3CHref%20'/trac.cgi'%3E%7D%20of%20None%3E"
>>> class Foo(object):
...     def __getattr__(self, name):
...         sys.stderr.write('__getattr__({!r})\n'.format(name))
...         return self
...     def __call__(self, *args, **kwargs):
...         sys.stderr.write('__call__(args={!r}, kwargs={!r})\n'.format(args, kwargs))
...
>>> Template('{{ val() }}').render(val=Foo())
__getattr__('contextfunction')
__call__(args=(<Context {'range': <class 'range'>, 'dict': <class 'dict'>, 'lipsum': <function generate_lorem_ipsum at 0x7f0386031310>, 'cycler': <class 'jinja2.utils.Cycler'>, 'joiner': <class 'jinja2.utils.Joiner'>, 'namespace': <class 'jinja2.utils.Namespace'>, 'val': <__main__.Foo object at 0x7f038a15d160>} of None>,), kwargs={})
'None'

Jinja2 2.10.3

>>> import jinja2, sys
>>> jinja2.__version__
'2.10.3'
>>> from jinja2 import Template
>>> from trac.web.href import Href
>>> href = Href('/trac.cgi')
>>> Template('{{ href() }}').render(href=href)
'/trac.cgi'
>>> class Foo(object):
...     def __getattr__(self, name):
...         sys.stderr.write('__getattr__({!r})\n'.format(name))
...         return self
...     def __call__(self, *args, **kwargs):
...         sys.stderr.write('__call__(args={!r}, kwargs={!r})\n'.format(args, kwargs))
...
>>> Template('{{ val() }}').render(val=Foo())
__call__(args=(), kwargs={})
'None'

Change History (12)

comment:1 by Jun Omae, 4 years ago

Ad hoc patch:

  • trac/web/href.py

    diff --git a/trac/web/href.py b/trac/web/href.py
    index 1d2336952..170e0e3bd 100644
    a b class Href(object):  
    188188        return href
    189189
    190190    def __getattr__(self, name):
     191        if name in ('contextfunction', 'evalcontextfunction',
     192                    'environmentfunction'):
     193            raise AttributeError
    191194        if name not in self._derived:
    192195            self._derived[name] = lambda *args, **kw: self(name, *args, **kw)
    193196        return self._derived[name]

comment:2 by Jun Omae, 4 years ago

Description: modified (diff)

comment:3 by Ryan J Ollos, 4 years ago

Looks similar to Jinja2 issue #1139. Relevant change is 70ea1d3e.

It wouldn't be an issue if Jinja2 tested the value: getattr(__obj, "contextfunction", 0) is True rather than just getattr(__obj, "contextfunction", 0).

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:4 by Jun Omae, 4 years ago

Proposed changes in [332bf1f51/jomae.git].

comment:5 by Ryan J Ollos, 4 years ago

Looks good. Nice solution!

comment:6 by Jun Omae, 4 years ago

Release Notes: modified (diff)
Resolution: fixed
Status: newclosed

Thanks for the review! Committed in [17238] and merged in [17239].

comment:7 by Ryan J Ollos, 4 years ago

I posted a related question in Jinja2 issue #1139.

in reply to:  7 comment:8 by Ryan J Ollos, 4 years ago

Replying to Ryan J Ollos:

I posted a related question in Jinja2 issue #1139.

They may address the issue in Jinja2 issue #1145, targeted to Jinja2 2.11.2. We can probably remove the Href attributes (contextfunction, …) if we ever drop support for Jinja < 2.11.2.

Last edited 4 years ago by Ryan J Ollos (previous) (diff)

comment:9 by Jun Omae, 4 years ago

Another work around without contextfunction attribute: we could simply ignore Jinja2 context instance because the arguments of Href.__call__() are typically instances of basestring, dict, list and tuple.

  • trac/web/href.py

    diff --git a/trac/web/href.py b/trac/web/href.py
    index 02f3cbe76..d27d2879c 100644
    a b  
    1616# Author: Jonas Borgström <jonas@edgewall.com>
    1717#         Christopher Lenz <cmlenz@gmx.de>
    1818
     19from jinja2.runtime import Context as Jinja2Context
    1920import re
    2021
    2122from trac.util.text import unicode_quote, unicode_urlencode
    class Href(object):  
    140141    '/milestone/<look,here%3E?param=%3Chere,too>'
    141142    """
    142143
    143     # Avoid passing Jinja2 context to __call__ (#13244)
    144     contextfunction = 0
    145     evalcontextfunction = 0
    146     environmentfunction = 0
    147 
    148144    def __init__(self, base, path_safe="/!~*'()", query_safe="!~*'()"):
    149145        self.base = base.rstrip('/')
    150146        self.path_safe = path_safe
    class Href(object):  
    165161            elif value is not None:
    166162                params.append((name, value))
    167163
     164        # Skip Jinja2 context (#13244)
     165        if args and isinstance(args[0], Jinja2Context):
     166            args = args[1:]
    168167        if args:
    169168            lastp = args[-1]
    170169            if isinstance(lastp, dict):
Last edited 4 years ago by Jun Omae (previous) (diff)

comment:10 by Jun Omae, 4 years ago

Owner: set to Jun Omae

comment:11 by Ryan J Ollos, 4 years ago

The comment:9 patch seems like a better solution because it doesn't prevent a path like /contextfunction.

One minor tweak is that the third-party import should be placed after the stdlib import:

import re

from jinja2.runtime import Context as Jinja2Context

from trac.util.text import unicode_quote, unicode_urlencode

comment:12 by Jun Omae, 4 years ago

Release Notes: modified (diff)

Thanks. Committed with your suggestion in [17246] and merged in [17247].

Note: See TracTickets for help on using tickets.