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.

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

namespace._copy trips on dict values that are types #126

Open
aaaaalbert opened this issue Aug 11, 2016 · 4 comments
Open

namespace._copy trips on dict values that are types #126

aaaaalbert opened this issue Aug 11, 2016 · 4 comments

Comments

@aaaaalbert
Copy link
Collaborator

A function declared to be available inside the RepyV2 namespace via one of namespace.py's *_WRAPPER_INFO wrapper dicts may list Dict() as (one of) its args processor(s). A call from within the sandbox into such function causes the argument(s) to be _copyed. Now certain dictionary contents cause problems during copying, particularly those that use types as values.

For instance, virtualnamespace.evaluate takes a dict or SafeDict argument. Supplying a dict with a problematic value like so,

v = createvirtualnamespace("", "")
v.evaluate({"abc": str}) # Ouch

breaks as namespace._copy raises Exception (with class 'namespace.NamespaceInternalError'): _copy failed on {'abc': <type 'str'>} with message _copy failed on <type 'str'> with message _copy is not implemented for objects of type <function safe_type at 0x10d0021b8>. The issue is that str is a type, or more accurately a safe.safe_type, and there is no handling for that in _copy.

Funnily enough, the problem is avoided by providing a SafeDict with the same contents, because the SafeDict makes a different clause in _copy match. The difference is that SafeDicts are instances / types.InstanceTypes.

v = createvirtualnamespace("", "")
v.evaluate(SafeDict({"abc": str})) # Success!

The superficial fix for this corner case of a corner case of a bit of funtionality I've never seen anyone use is to add safe.safe_type to the list of types that namespace._copy checks the type of its argument, obj, against. Whether or not dicts should be treated all that differently from SafeDicts as they traverse the namespace boundary is a different question though.


Historical note: I found this problem via the original virtualnamespace context safety unit test, although that test was not supposed the above problem: https://github.com/SeattleTestbed/repy_v2/blob/4db9ecd48a4577d646e4c38cf1af634dd8600810/testsV2/ut_repyv2api_virtualnamespacecontextsafety.py .

aaaaalbert added a commit to aaaaalbert/repy_v2 that referenced this issue Aug 11, 2016
Also fix `ut_repyv2api_virtualnamespacecontextsafety.py` to test the
proper functionality. (Following the namespace-wrap, it exhibited
an issue in in `namespace.py` which we need to discuss separately,
see SeattleTestbed#126)

Details about namespace wrapping:
* In `repy.py`, remove the addition of the non-wrapped `getresources`
  call to `usercontext`. (The properly wrapped definition was implemented
  in `namespace.py`'s `USERCONTEXT_WRAPPER_INFO` already, but `repy.py`
  overrode it.)
* In `namespace.py`, change the `SafeDict*` processors to inherit from
  class `ValueProcessor` rather than `ObjectProcessor`. This follows
  what the `Dict` processor does, and fixes the `virtualnamespace-eval`
  and `virtualnamespacecontextsafety` unit tests that failed with
  unwrapping errors when supplying a `dict` / `SafeDict` to the
  `VirtualNamespace` to be evaluated. The particular unit test
  errors were artifacts of how the `SafeDict*` processors inherited
  from `ObjectProcessor`, and looked like this:

"""
	Running: ut_repyv2api_virtualnamespace-eval.py              [ FAIL ]
--------------------------------------------------------------------------------
Standard error :
..............................Produced..............................
Internal Error
---
Uncaught exception!
---
Following is a full traceback, and a user traceback.
The user traceback excludes non-user modules. The most recent call is displayed last.

Full debugging traceback:
  "/private/tmp/repy_v2/RUNNABLE/namespace.py", line 1194, in wrapped_function
  "/private/tmp/repy_v2/RUNNABLE/namespace.py", line 1095, in _process_args
  "/private/tmp/repy_v2/RUNNABLE/namespace.py", line 283, in unwrap

User traceback:

Exception (with type 'exceptions.AttributeError'): SafeDict instance has no attribute '_wrapped__object'
---

..............................Expected..............................
None
--------------------------------------------------------------------------------
	Running: ut_repyv2api_virtualnamespacecontextsafety.py      [ FAIL ]
--------------------------------------------------------------------------------
Standard error :
..............................Produced..............................
Internal Error
---
Uncaught exception!
---
Following is a full traceback, and a user traceback.
The user traceback excludes non-user modules. The most recent call is displayed last.

Full debugging traceback:
  "/private/tmp/repy_v2/RUNNABLE/namespace.py", line 1194, in wrapped_function
  "/private/tmp/repy_v2/RUNNABLE/namespace.py", line 1095, in _process_args
  "/private/tmp/repy_v2/RUNNABLE/namespace.py", line 283, in unwrap

User traceback:

Exception (with type 'exceptions.AttributeError'): 'dict' object has no attribute '_wrapped__object'
---

..............................Expected..............................
None
--------------------------------------------------------------------------------
"""
@aaaaalbert
Copy link
Collaborator Author

The rough patch thus is

--- ../namespace.py
+++ ./namespace.py
@@ -814,5 +814,6 @@
     if _is_in(type(obj), [str, unicode, int, long, float, complex, bool, frozenset,
                           types.NoneType, types.FunctionType, types.LambdaType,
-                          types.MethodType, types.InstanceType]):
+                          types.MethodType, types.InstanceType,
+                          safe.safe_type]):
       return obj

@aaaaalbert
Copy link
Collaborator Author

Side note while constructing the unit test: besides str, object also triggers the bug. The workaround is the same.

@aaaaalbert
Copy link
Collaborator Author

...and digging some more (with help(types) as a handy reference) reveals a few more:

unusual_cases = [
    # thing        # type of thing
    DummyClass,    # classobj
    chr,           # builtin_function_or_method (like abs, bool, chr, etc.)
    str, Exception, RepyException, # safe_type (like int, float, set, etc.)
    xrange(1),     # xrange
    slice(1),      # slice
    Ellipsis,      # ellipsis
    ]

As already demonstrated in the current namespace.py code, the types module provides appropriate objects for comparison for these types as well.

@aaaaalbert
Copy link
Collaborator Author

The corresponding patch, including some clean-up:

814,816c814,827
<     if _is_in(type(obj), [str, unicode, int, long, float, complex, bool, frozenset,
<                           types.NoneType, types.FunctionType, types.LambdaType,
<                           types.MethodType, types.InstanceType]):
---
>     if _is_in(type(obj), [
>         str, unicode, int, long, float, complex, bool, frozenset,
>         types.NoneType,     # for None
>         types.FunctionType, # for functions
>         types.BuiltinFunctionType, # for builtin functions like chr
>         types.LambdaType,   # XXX should not happen
>         types.MethodType,   # for methods of objects
>         types.InstanceType, # for instantiated objects
>         types.ClassType,    # for classes of objects
>         types.SliceType,    # for slices
>         types.XRangeType,   # for xrange
>         types.EllipsisType, # for Ellipsis
>         safe.safe_type,     # for wrapped types like str, RepyExcpetion
>         ]):

I don't understand why exactly we allow lambda here when safe.py actually forbids it (and there is no way to construct a lambda expression in RepyV2 thus).

aaaaalbert added a commit to aaaaalbert/repy_v2 that referenced this issue Sep 13, 2017
As documented by SeattleTestbed#126, namespace wrapping and
unwrapping for `dict`s is incomplete and breaks if a `dict` that
contains a value of certain types like `safe_type` or `classobj`
traverses the namespace in a RepyV2 API call. (At the moment, the
`evaluate(context)` method of VirtualNamespace is the only API
call that takes a `dict` as the parameter.)

This commit patches the relevant `_copy` method in `namespace.py`
to enable proper copying of said types. Furthermore, it adds a
unit test that checks many "easy", natural and often-encountered
types, but also checks "unusual" cases (like those above) to
verify that the patch really fixes the problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant