Skip to content

Commit

Permalink
Instead of forbidding underscore directly deprecate it for now
Browse files Browse the repository at this point in the history
  • Loading branch information
loechel committed May 18, 2018
1 parent 21d16a8 commit 8fd2df9
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 23 deletions.
20 changes: 12 additions & 8 deletions src/RestrictedPython/transformer.py
Expand Up @@ -452,14 +452,7 @@ def check_import_names(self, node):
This is a protection against rebinding dunder names like
_getitem_, _write_ via imports.
=> 'from _a import x' is ok, because '_a' is not added to the scope.
"""
if (isinstance(node, ast.ImportFrom)
and not node.module == '__future__'
and any(name.startswith('_') for name in node.module.split('.'))): # NOQA: E501
self.error(node, 'module name starts "_", which is forbidden.')

for name in node.names:
if '*' in name.name:
self.error(node, '"*" imports are not allowed.')
Expand Down Expand Up @@ -1161,7 +1154,18 @@ def visit_Import(self, node):

def visit_ImportFrom(self, node):
"""Allow `import from` statements with restrictions.
See check_import_names."""
See check_import_names.
=> 'from _a import x' is security wise ok,
because '_a' is not added to the scope.
For consistency reasons we should deprecated that and
forbid it in a future versions.
"""
if not node.module == '__future__' and any(name.startswith('_') for name in node.module.split('.')): # NOQA: E501
# self.error(node, 'module name starts "_", which is forbidden.')
self.warn(node, 'module name starts "_", which is deprecated and '
'will be forbidden in RestrictedPython 5.')
return self.check_import_names(node)

def visit_alias(self, node):
Expand Down
42 changes: 27 additions & 15 deletions tests/transformer/test_import.py
Expand Up @@ -46,25 +46,37 @@ def test_RestrictingNodeTransformer__visit_Import__5(c_exec):

@pytest.mark.parametrize(*c_exec)
def test_RestrictingNodeTransformer__visit_Import__6(c_exec):
"""Deny imports from modules staring with `_`."""
"""Check for imports from modules starting with `_`
they will be deprecated in a future version."""
warning_message = 'Line 1: module name starts "_", which is deprecated and will be forbidden in RestrictedPython 5.' # NOQA: E501
result = c_exec('from _a import m')
assert result.errors == (
'Line 1: module name starts "_", which is forbidden.',
)
assert result.warnings == ([warning_message])
result = c_exec('from a._b import m')
assert result.errors == (
'Line 1: module name starts "_", which is forbidden.',
)
assert result.warnings == ([warning_message])
result = c_exec('from _a.b import m')
assert result.errors == (
'Line 1: module name starts "_", which is forbidden.',
)
assert result.warnings == ([warning_message])
result = c_exec('from _a._b import m as x')
assert result.errors == (
'Line 1: module name starts "_", which is forbidden.',
)
result = c_exec('from a import m as _n')
assert result.errors == (import_errmsg % '_n',)
assert result.warnings == ([warning_message])
# Placeholder for future version
# """Deny imports from modules starting with `_`."""
# result = c_exec('from _a import m')
# assert result.errors == (
# 'Line 1: module name starts "_", which is forbidden.',
# )
# result = c_exec('from a._b import m')
# assert result.errors == (
# 'Line 1: module name starts "_", which is forbidden.',
# )
# result = c_exec('from _a.b import m')
# assert result.errors == (
# 'Line 1: module name starts "_", which is forbidden.',
# )
# result = c_exec('from _a._b import m as x')
# assert result.errors == (
# 'Line 1: module name starts "_", which is forbidden.',
# )
# result = c_exec('from a import m as _n')
# assert result.errors == (import_errmsg % '_n',)


@pytest.mark.parametrize(*c_exec)
Expand Down

0 comments on commit 8fd2df9

Please sign in to comment.