Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix/Improve $(eval ...) #2269

Open
wants to merge 3 commits into
base: noetic-devel
Choose a base branch
from
Open

Fix/Improve $(eval ...) #2269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
79f77c5
test access to global symbols in list comprehension
rhaschke Aug 24, 2022
98285fa
Fix symbol access in list comprehension
rhaschke Aug 24, 2022
320ca8f
_eval(): better protection against use of hidden symbols
rhaschke Aug 24, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions tools/roslaunch/src/roslaunch/substitution_args.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"""

import os
import math

try:
from cStringIO import StringIO # Python 2.x
Expand All @@ -47,7 +48,6 @@
import rosgraph.names
import rospkg
from roslaunch.loader import convert_value
import math

_rospack = None

Expand Down Expand Up @@ -302,22 +302,23 @@ def _arg(resolved, a, args, context):
_eval_dict={
'true': True, 'false': False,
'True': True, 'False': False,
'__builtins__': {k: __builtins__[k] for k in ['list', 'dict', 'map', 'str', 'float', 'int']},
'__builtins__': __builtins__,
'env': _eval_env,
'optenv': _eval_optenv,
'find': _eval_find
}
# also define all math symbols and functions
_eval_dict.update(math.__dict__)

class _DictWrapper(object):
class _DictWrapper(dict):
def __init__(self, args, functions):
super().__init__()
super().update(functions)
self._args = args
self._functions = functions

def __getitem__(self, key):
try:
return self._functions[key]
return super().__getitem__(key)
except KeyError:
return convert_value(self._args[key], 'auto')

Expand All @@ -342,9 +343,11 @@ def _eval_dirname_context(): return _eval_dirname(context['filename'])

# ignore values containing double underscores (for safety)
# http://nedbatchelder.com/blog/201206/eval_really_is_dangerous.html
if s.find('__') >= 0:
code = compile(s.strip(), "<expression>", "eval")
invalid_names = [n for n in code.co_names if n.startswith("__")]
if invalid_names:
raise SubstitutionException("$(eval ...) may not contain double underscore expressions")
return str(eval(s, {}, _DictWrapper(context['arg'], functions)))
return str(eval(s, _DictWrapper(context['arg'], functions)))

def resolve_args(arg_str, context=None, resolve_anon=True, filename=None):
"""
Expand Down
9 changes: 7 additions & 2 deletions tools/roslaunch/test/unit/test_substitution_args.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def test_resolve_args():
assert roslaunch_dir

anon_context = {'foo': 'bar'}
arg_context = {'fuga': 'hoge', 'car': 'cdr', 'arg': 'foo', 'True': 'False'}
arg_context = {'fuga': 'hoge', 'car': 'cdr', 'arg': 'foo', 'True': 'False', 'dou__ble': '3.14'}
context = {'anon': anon_context, 'arg': arg_context, 'filename': '/path/to/file.launch'}

tests = [
Expand Down Expand Up @@ -145,6 +145,8 @@ def test_resolve_args():
('$(eval arg("True"))', 'False'),
('$(eval 1==1)', 'True'),
('$(eval [0,1,2][1])', '1'),
('$(eval dou__ble)', '3.14'),
('$(eval "dou__ble")', 'dou__ble'),
# test implicit arg access
('$(eval fuga)', 'hoge'),
('$(eval True)', 'True'),
Expand All @@ -153,6 +155,8 @@ def test_resolve_args():
('$(eval cos(0))', '1.0'),
# str, map
("$(eval ''.join(map(str, [4,2])))", '42'),
# global symbols in range context
("$(eval [arg(var) for var in ['fuga', 'arg']])", "['hoge', 'foo']")
]
for arg, val in tests:
assert val == resolve_args(arg, context=context), arg
Expand All @@ -179,7 +183,8 @@ def test_resolve_args():
'$(anon)',
'$(anon foo bar)',
# Should fail without the supplied context.
'$(dirname)'
'$(dirname)',
'$(eval __import__("os"))',
]
for f in failures:
try:
Expand Down