"zope.tales.tales.Context" has bad "getValue"

[d-maurer]

zope.tales.tales.Context sets up (among others) attributes vars (the current "variable -> value" mapping) and _vars_stack (a stack of such mappings). Thereby, _vars_stack[0] represents the initial and _vars_stack[-1] the most recent variable mapping. One would expect that getValue(self, name, default) is defined as self.vars.get(name, default) but the definition instead looks like:

    def getValue(self, name, default=None):
        value = default
        for vars in self._vars_stack:
            value = vars.get(name, default)
            if value is not default:
                break
        return value

i.e. it prefers older bindings over newer ones.

[mgedmin]

How did this bug went undiscovered for so long?

Do you have a high-level example (e.g. a page template) of where it shows up?

[d-maurer]

Marius Gedminas wrote at 2020-3-26 02:03 -0700:

How did this bug went undiscovered for so long?

Templates do not use getValue (but directly vars).

The bug would affect only applications/components which use the official API (the interface describes getValue but not vars) and not the implementation details (vars).

Do you have a high-level example (e.g. a page template) of where it shows up?

No.

I currently discuss with @malthe the differences between chameleons Scope and its equivalent zope.tales.tales.Context (see conversation around "https://github.com/malthe/chameleon/issues/301#issuecomment-604264247"). In this context, I looked at the code and found the unexpected getValue definition.

This discussion also indicates that Context implements setGlobal in a peculiar way: a setGlobal is able to override a local variable binding. I believe that this is buggy.

[jamadden]

How did this bug went undiscovered for so long?

https://github.com/zopefoundation/zope.tales/pull/20/files/1130a2352beaccc106d360a70d89c70d2200052e#diff-99a3b96df81fef7a2df80eb23ff4733c describes how the current scope is always a complete copy of all previous contexts, so the only answer we need is in the current scope.

This discussion also indicates that Context implements setGlobal in a peculiar way: a setGlobal is able to override a local variable binding. I believe that this is buggy.

That same comment should show that the changes in that PR make this behavior of setGlobal critical and definitely not a bug. If it stops doing what it's doing, then copying vars into every scope stops working correctly (or vice versa; the two behaviours are linked).

[malte]

Hello, please take me out of this email-List, i am mistaken for another "Malte" all the best, Malte Euler

Am Fr., 27. März 2020 um 12:12 Uhr schrieb Jason Madden < notifications@github.com>:

How did this bug went undiscovered for so long?

https://github.com/zopefoundation/zope.tales/pull/20/files/1130a2352beaccc106d360a70d89c70d2200052e#diff-99a3b96df81fef7a2df80eb23ff4733c describes how the current scope is always a complete copy of all previous contexts, so the only answer we need is in the current scope.

This discussion also indicates that Context implements setGlobal in a peculiar way: a setGlobal is able to override a local variable binding. I believe that this is buggy.

That same comment should show that the changes in that PR make this behavior of setGlobal critical and definitely not a bug. If it stops doing what it's doing, then copying vars into every scope stops working correctly (or vice versa; the two behaviours are linked).

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/zopefoundation/zope.tales/issues/19#issuecomment-604943699, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEMC2LCHUQPFRMI3FFEFDRJSC2JANCNFSM4LUANOZQ .

--

Malte Euler Karl-Marx-Straße 15, 12043 Berlin Mob 0179 / 66666 14 www.malteeuler.com

[jamadden]

@malte I've updated the tag to refer to the intended @malthe, but you may need to manually unsubscribe from this issue in Github (there is an 'unsubscribe' link at the bottom of email notifications). Sorry for the confusion!

[d-maurer]

Jason Madden wrote at 2020-3-27 04:12 -0700:

...

This discussion also indicates that Context implements setGlobal in a peculiar way: a setGlobal is able to override a local variable binding. I believe that this is buggy.

That same comment should show that the changes in that PR make this behavior of setGlobal critical and definitely not a bug.

It is a "behavioural" bug as it allows a macro use to effectively change a (local) variable value - which can lead to real surprises.

If it stops doing what it's doing, then copying vars into every scope stops working correctly (or vice versa; the two behaviours are linked).

I am aware that it cannot be easily changed (and I did not try).