Ideas to optimize this getitem/eval call?

[mario]

On Jan 3, 7:16 am, Steven D'Aprano <st... at REMOVE-THIS-
cybersource.com.au> wrote:

> I was about to make a comment about this being a security hole,

Strange that you say this, as you are also implying that *all* the
widely-used templating systems for python are security holes... Well,
you would be right to say that of course ;-) Infact, evoque is really
one of the few (or even the only one?) that was conceived from the
start to support restricted evaluation.

> but I see from here
>
> http://evoque.gizmojo.org/usage/restricted/
>
> that you are aware of at least some of the issues.
>
> I must say though, your choice of builtins to prohibit seems rather
> arbitrary. What is dangerous about (e.g.) id() and isinstance()?

Preventive, probably. I also feel that temlates should have any
business with info such as the memory addressed returnred by id(). For
isinstance, becuase it is somewhat related to __subclasses__ that is
known to be insecure.

Incidentally, I updated the page you point to clarify what is going
on. I also added a link to Brett Cannon's inspiration paper on the
topic of securing the python interpreter...

> >     except:
> >         # KeyError, NameError, AttributeError, SyntaxError,
> >         # ValueError, TypeError, IOError
>
> If you want to capture seven exceptions, then capture seven exceptions,
> not any exception.

Absolutely not. I want to catch ALL evaluation exceptions... it would
actually *be* a secuity hole to allow any exception to bubble. hey
will however be handled appropriately as per the application policy/
config/deployment.

> You should write:
>
>     except (KeyError, NameError, ..., IOError):
>
> instead of a bare except clause. That will capture exceptions that
> represent bugs in your code as well as exceptions that should propbably
> be allowed to propagate, such as KeyboardInterupt and SystemExit.

Again, no. Template presentational logic has no business issuing
SystemExits or so. And, of course, there are no bugs in my code ;-)

> >         # Special case if a KeyError is coming from the self.codes[name]
> >         # lookup (traceback should consist of a single frame only):
> >         if sys.exc_info()[2].tb_next is None:
> >             if sys.exc_info()[0] is KeyError:
> >                 self.codes[expr] = compile(expr, '<string>', 'eval')
> >                     return self[expr]

> That seems awfully complicated for no good reason.

Yes, you are probably right. I wanted to be precise in that if the
KeyError originates strictly from the codes lookup and not from the
actual eval of the expr itself -- in which case the expr should be
compiled and added to codes (yes, this is the "first-time failure" I
referred to in the first message).

I tested the performance of your 2 variations in context, and there
seems to be no noticeable performance gain (something like less than
1% gain). But note the two variations as you code them do not quite do
exactly the same test.

I have adjusted to use this code now:

    def __getitem__(self, expr):
        try:
            return eval(self.codes[expr], self.globals, self.locals)
        except:
            # We want to catch **all** evaluation errors!
            # KeyError, NameError, AttributeError, SyntaxError, V
            # alueError, TypeError, IOError, ...
            #
            # Special case:
            # if KeyError is coming from self.codes[expr] lookup,
            # then we add the compiledentry and try again:
            if not name in self.codes:
                self.codes[expr] = compile(name, '<string>', 'eval')
                return self[expr]
            # handle any other error...

This retains the correctness of the check, and has the same marginal
perf improvement (that is basically negligible, but at least it is not
slower) and has teh advantage that the code is clearer.

Thanks for the remarks!

mario

> --
> Steven