From f5258984a4b849f4e05eb4e58d6bef1bc4d0c318 Mon Sep 17 00:00:00 2001 From: stephan-hof Date: Fri, 3 Mar 2017 08:07:23 +0100 Subject: [PATCH 1/8] 'RestrictingNodeTransformer' reports now the used names. --- src/RestrictedPython/transformer.py | 16 +++++++++------- tests/test_compile.py | 8 ++++++++ tests/transformer/test_transformer.py | 6 +----- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/RestrictedPython/transformer.py b/src/RestrictedPython/transformer.py index 7235a28..454ef4c 100644 --- a/src/RestrictedPython/transformer.py +++ b/src/RestrictedPython/transformer.py @@ -90,7 +90,13 @@ def __init__(self, errors=None, warnings=None, used_names=None): super(RestrictingNodeTransformer, self).__init__() self.errors = [] if errors is None else errors self.warnings = [] if warnings is None else warnings - self.used_names = [] if used_names is None else used_names + + # All the variables used by the incoming source. + # Internal names/variables, like the ones from 'gen_tmp_name', don't + # have to be added. + # 'used_names' is for example needed by 'RestrictionCapableEval' to + # know wich names it has to supply when calling the final code. + self.used_names = {} if used_names is None else used_names # Global counter to construct temporary variable names. self._tmp_idx = 0 @@ -116,12 +122,6 @@ def warn(self, node, info): self.warnings.append( 'Line {lineno}: {info}'.format(lineno=lineno, info=info)) - def use_name(self, node, info): - """Record a security error discovered during transformation.""" - lineno = getattr(node, 'lineno', None) - self.used_names.append( - 'Line {lineno}: {info}'.format(lineno=lineno, info=info)) - def guard_iter(self, node): """ Converts: @@ -585,6 +585,8 @@ def visit_Name(self, node): copy_locations(new_node, node) return new_node + self.used_names[node.id] = True + self.check_name(node, node.id) return node diff --git a/tests/test_compile.py b/tests/test_compile.py index d62a783..2e3d5cb 100644 --- a/tests/test_compile.py +++ b/tests/test_compile.py @@ -147,3 +147,11 @@ def test_compile__compile_restricted_eval__1(c_eval): def test_compile__compile_restricted_eval__2(e_eval): """It compiles code as an Expression.""" assert e_eval('4 * 6') == 24 + + +@pytest.mark.parametrize(*c_eval) +def test_compile__compile_restricted_eval__used_names(c_eval): + result = c_eval("a + b + func(x)") + assert result.errors == () + assert result.warnings == [] + assert result.used_names == {'a': True, 'b': True, 'x': True, 'func': True} diff --git a/tests/transformer/test_transformer.py b/tests/transformer/test_transformer.py index 41651a2..9080c50 100644 --- a/tests/transformer/test_transformer.py +++ b/tests/transformer/test_transformer.py @@ -62,11 +62,7 @@ def test_transformer__RestrictingNodeTransformer__visit_Call__1(c_exec): loc = {} exec(result.code, {}, loc) assert loc['a'] == 3 - if c_exec is RestrictedPython.compile.compile_restricted_exec: - # The new version not yet supports `used_names`: - assert result.used_names == {} - else: - assert result.used_names == {'max': True} + assert result.used_names == {'max': True} YIELD = """\ From 9ab2d69f9411bca8de17730cdcd2240c9320699b Mon Sep 17 00:00:00 2001 From: stephan-hof Date: Tue, 28 Feb 2017 07:56:38 +0100 Subject: [PATCH 2/8] Use the ast module to get all the used names. This gets rid of the dependency to know how the bytecode looks like. --- src/RestrictedPython/Eval.py | 35 +++++++++++++---------------------- 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/RestrictedPython/Eval.py b/src/RestrictedPython/Eval.py index 5051a6d..e4aa142 100644 --- a/src/RestrictedPython/Eval.py +++ b/src/RestrictedPython/Eval.py @@ -12,6 +12,7 @@ ############################################################################## """Restricted Python Expressions.""" +import ast from RestrictedPython.RCompile import compile_restricted_eval from string import strip from string import translate @@ -71,29 +72,19 @@ def prepRestrictedCode(self): def prepUnrestrictedCode(self): if self.ucode is None: - # Use the standard compiler. - co = compile(self.expr, '', 'eval') + exp_node = compile(self.expr, '', 'eval', ast.PyCF_ONLY_AST) + co = compile(exp_node, '', 'eval') + + # Examine the ast to discover which names the expression needs. if self.used is None: - # Examine the code object, discovering which names - # the expression needs. - names = list(co.co_names) - used = {} - i = 0 - code = co.co_code - l = len(code) - LOAD_NAME = 101 - HAVE_ARGUMENT = 90 - while(i < l): - c = ord(code[i]) - if c == LOAD_NAME: - name = names[ord(code[i + 1]) + 256 * ord(code[i + 2])] - used[name] = 1 - i = i + 3 - elif c >= HAVE_ARGUMENT: - i = i + 3 - else: - i = i + 1 - self.used = tuple(used.keys()) + used = set() + for node in ast.walk(exp_node): + if isinstance(node, ast.Name): + if isinstance(node.ctx, ast.Load): + used.add(node.id) + + self.used = tuple(used) + self.ucode = co def eval(self, mapping): From 95fcf825a3c064bce619d1912c5895776c72e854 Mon Sep 17 00:00:00 2001 From: stephan-hof Date: Tue, 28 Feb 2017 08:00:50 +0100 Subject: [PATCH 3/8] Remove not needed code. I guess this particular code was used at the early days of restricted python. To see the speed penaltiy. However I cannot see how this can be utilized by other libraries. --- src/RestrictedPython/Eval.py | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/src/RestrictedPython/Eval.py b/src/RestrictedPython/Eval.py index e4aa142..46fea9f 100644 --- a/src/RestrictedPython/Eval.py +++ b/src/RestrictedPython/Eval.py @@ -30,9 +30,6 @@ def default_guarded_getitem(ob, index): return ob[index] -PROFILE = 0 - - class RestrictionCapableEval(object): """A base class for restricted code.""" @@ -56,15 +53,7 @@ def __init__(self, expr): def prepRestrictedCode(self): if self.rcode is None: - if PROFILE: - from time import clock - start = clock() - co, err, warn, used = compile_restricted_eval( - self.expr, '') - if PROFILE: - end = clock() - print('prepRestrictedCode: %d ms for %s' % ( - (end - start) * 1000, repr(self.expr))) + co, err, warn, used = compile_restricted_eval(self.expr, '') if err: raise SyntaxError(err[0]) self.used = tuple(used.keys()) From 1f20bcf21e7b23ac0f1f18b9366f4cc0aa933fe5 Mon Sep 17 00:00:00 2001 From: stephan-hof Date: Tue, 28 Feb 2017 08:34:50 +0100 Subject: [PATCH 4/8] Port maketrans to python3 and remove other usages of the string module. --- src/RestrictedPython/Eval.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/RestrictedPython/Eval.py b/src/RestrictedPython/Eval.py index 46fea9f..1316da5 100644 --- a/src/RestrictedPython/Eval.py +++ b/src/RestrictedPython/Eval.py @@ -14,13 +14,16 @@ import ast from RestrictedPython.RCompile import compile_restricted_eval -from string import strip -from string import translate -import string +from ._compat import IS_PY2 +if IS_PY2: + from string import maketrans +else: + maketrans = str.maketrans -nltosp = string.maketrans('\r\n', ' ') + +nltosp = maketrans('\r\n', ' ') default_guarded_getattr = getattr # No restrictions. @@ -45,9 +48,9 @@ def __init__(self, expr): expr -- a string containing the expression to be evaluated. """ - expr = strip(expr) + expr = expr.strip() self.__name__ = expr - expr = translate(expr, nltosp) + expr = expr.translate(nltosp) self.expr = expr self.prepUnrestrictedCode() # Catch syntax errors. From 2d0c4423d959bb3709cfb2460becb77f6e588bb4 Mon Sep 17 00:00:00 2001 From: stephan-hof Date: Tue, 28 Feb 2017 08:35:46 +0100 Subject: [PATCH 5/8] Refactor 'eval'. * Using `keys` is not modern anymore. * Using `has_key` is not modern anymore. * Proper variable names * Comments in the same line is not very pep8. --- src/RestrictedPython/Eval.py | 43 ++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/src/RestrictedPython/Eval.py b/src/RestrictedPython/Eval.py index 1316da5..33dc6e4 100644 --- a/src/RestrictedPython/Eval.py +++ b/src/RestrictedPython/Eval.py @@ -25,7 +25,8 @@ nltosp = maketrans('\r\n', ' ') -default_guarded_getattr = getattr # No restrictions. +# No restrictions. +default_guarded_getattr = getattr def default_guarded_getitem(ob, index): @@ -37,8 +38,13 @@ class RestrictionCapableEval(object): """A base class for restricted code.""" globals = {'__builtins__': None} - rcode = None # restricted - ucode = None # unrestricted + # restricted + rcode = None + + # unrestricted + ucode = None + + # Names used by the expression used = None def __init__(self, expr): @@ -52,14 +58,15 @@ def __init__(self, expr): self.__name__ = expr expr = expr.translate(nltosp) self.expr = expr - self.prepUnrestrictedCode() # Catch syntax errors. + # Catch syntax errors. + self.prepUnrestrictedCode() def prepRestrictedCode(self): if self.rcode is None: co, err, warn, used = compile_restricted_eval(self.expr, '') if err: raise SyntaxError(err[0]) - self.used = tuple(used.keys()) + self.used = tuple(used) self.rcode = co def prepUnrestrictedCode(self): @@ -83,21 +90,19 @@ def eval(self, mapping): # This default implementation is probably not very useful. :-( # This is meant to be overridden. self.prepRestrictedCode() - code = self.rcode - d = {'_getattr_': default_guarded_getattr, - '_getitem_': default_guarded_getitem} - d.update(self.globals) - has_key = d.has_key + + global_scope = { + '_getattr_': default_guarded_getattr, + '_getitem_': default_guarded_getitem + } + + global_scope.update(self.globals) + for name in self.used: - try: - if not has_key(name): - d[name] = mapping[name] - except KeyError: - # Swallow KeyErrors since the expression - # might not actually need the name. If it - # does need the name, a NameError will occur. - pass - return eval(code, d) + if (name not in global_scope) and (name in mapping): + global_scope[name] = mapping[name] + + return eval(self.rcode, global_scope) def __call__(self, **kw): return self.eval(kw) From e5817ac147f93259f6f684cbeb1f5d298522e03d Mon Sep 17 00:00:00 2001 From: stephan-hof Date: Tue, 28 Feb 2017 08:46:06 +0100 Subject: [PATCH 6/8] Use 'compile_restricted_eval' from compile.py --- src/RestrictedPython/Eval.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/RestrictedPython/Eval.py b/src/RestrictedPython/Eval.py index 33dc6e4..7c8ab0d 100644 --- a/src/RestrictedPython/Eval.py +++ b/src/RestrictedPython/Eval.py @@ -13,8 +13,8 @@ """Restricted Python Expressions.""" import ast -from RestrictedPython.RCompile import compile_restricted_eval +from .compile import compile_restricted_eval from ._compat import IS_PY2 if IS_PY2: From 6fcc3999f913dc3cc0647413049b3946377946af Mon Sep 17 00:00:00 2001 From: stephan-hof Date: Thu, 2 Mar 2017 07:43:09 +0100 Subject: [PATCH 7/8] Move the tests for 'Eval.py' to the new tests. --- .../tests/testRestrictions.py | 12 ------ tests/test_eval.py | 39 +++++++++++++++++++ 2 files changed, 39 insertions(+), 12 deletions(-) create mode 100644 tests/test_eval.py diff --git a/src/RestrictedPython/tests/testRestrictions.py b/src/RestrictedPython/tests/testRestrictions.py index 369b125..c13e04d 100644 --- a/src/RestrictedPython/tests/testRestrictions.py +++ b/src/RestrictedPython/tests/testRestrictions.py @@ -6,7 +6,6 @@ # here instead. from RestrictedPython import PrintCollector -from RestrictedPython.Eval import RestrictionCapableEval from RestrictedPython.RCompile import compile_restricted from RestrictedPython.RCompile import RFunction from RestrictedPython.RCompile import RModule @@ -307,17 +306,6 @@ def test_NestedScopes1(self): res = self.execFunc('nested_scopes_1') self.assertEqual(res, 2) - def test_UnrestrictedEval(self): - expr = RestrictionCapableEval("{'a':[m.pop()]}['a'] + [m[0]]") - v = [12, 34] - expect = v[:] - expect.reverse() - res = expr.eval({'m': v}) - self.assertEqual(res, expect) - v = [12, 34] - res = expr(m=v) - self.assertEqual(res, expect) - def test_StackSize(self): for k, rfunc in rmodule.items(): if not k.startswith('_') and hasattr(rfunc, 'func_code'): diff --git a/tests/test_eval.py b/tests/test_eval.py new file mode 100644 index 0000000..fd43bba --- /dev/null +++ b/tests/test_eval.py @@ -0,0 +1,39 @@ +import pytest +from RestrictedPython.Eval import RestrictionCapableEval + +exp = """ + {'a':[m.pop()]}['a'] \ + + [m[0]] +""" + +def test_init(): + ob = RestrictionCapableEval(exp) + + assert ob.expr == "{'a':[m.pop()]}['a'] + [m[0]]" + assert ob.used == ('m', ) + assert ob.ucode is not None + assert ob.rcode is None + + +def test_init_with_syntax_error(): + with pytest.raises(SyntaxError): + RestrictionCapableEval("if:") + + +def test_prepRestrictedCode(): + ob = RestrictionCapableEval(exp) + ob.prepRestrictedCode() + assert ob.used == ('m', ) + assert ob.rcode is not None + + +def test_call(): + ob = RestrictionCapableEval(exp) + ret = ob(m=[1, 2]) + assert ret == [2, 1] + + +def test_eval(): + ob = RestrictionCapableEval(exp) + ret = ob.eval({'m': [1, 2]}) + assert ret == [2, 1] From 2f572d29971c29af07697348143531c80ce40875 Mon Sep 17 00:00:00 2001 From: stephan-hof Date: Fri, 3 Mar 2017 08:53:24 +0100 Subject: [PATCH 8/8] Make isort and flake8 happy. --- src/RestrictedPython/Eval.py | 22 ++++++++++++++-------- tests/test_eval.py | 5 ++++- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/RestrictedPython/Eval.py b/src/RestrictedPython/Eval.py index 7c8ab0d..836ea5e 100644 --- a/src/RestrictedPython/Eval.py +++ b/src/RestrictedPython/Eval.py @@ -12,10 +12,11 @@ ############################################################################## """Restricted Python Expressions.""" +from ._compat import IS_PY2 +from .compile import compile_restricted_eval + import ast -from .compile import compile_restricted_eval -from ._compat import IS_PY2 if IS_PY2: from string import maketrans @@ -63,15 +64,20 @@ def __init__(self, expr): def prepRestrictedCode(self): if self.rcode is None: - co, err, warn, used = compile_restricted_eval(self.expr, '') - if err: - raise SyntaxError(err[0]) - self.used = tuple(used) - self.rcode = co + result = compile_restricted_eval(self.expr, '') + if result.errors: + raise SyntaxError(result.errors[0]) + self.used = tuple(result.used_names) + self.rcode = result.code def prepUnrestrictedCode(self): if self.ucode is None: - exp_node = compile(self.expr, '', 'eval', ast.PyCF_ONLY_AST) + exp_node = compile( + self.expr, + '', + 'eval', + ast.PyCF_ONLY_AST) + co = compile(exp_node, '', 'eval') # Examine the ast to discover which names the expression needs. diff --git a/tests/test_eval.py b/tests/test_eval.py index fd43bba..f9046a4 100644 --- a/tests/test_eval.py +++ b/tests/test_eval.py @@ -1,11 +1,14 @@ -import pytest from RestrictedPython.Eval import RestrictionCapableEval +import pytest + + exp = """ {'a':[m.pop()]}['a'] \ + [m[0]] """ + def test_init(): ob = RestrictionCapableEval(exp)