tbodt/v8py

JSClass and JS objects

armudgal opened this issue · 15 comments

Hi there,
sorry to disturb you, we are facing an issue with port from PyV8 to v8py and thought you could help.

We are setting some attributes for the object of type Window here.
https://github.com/kira0204/thug/blob/c30dd33222ba9424fb306eb40f69d45fcbf67993/thug/DOM/Window.py#L796-L804

Logging self.document seems fine and works perfectly but when using this code to analyze a JS script such as this https://github.com/kira0204/thug/blob/c30dd33222ba9424fb306eb40f69d45fcbf67993/samples/misc/test1.html#L3-L7, we are returned None in both the cases. Somehow the setting of attributes is not reflected in the JS.

Relevant code: https://github.com/kira0204/thug/blob/c30dd33222ba9424fb306eb40f69d45fcbf67993/thug/DOM/JSClass.py#L30-L35

Would like to hear your opinion on this, thanks.
cc @buffer

tbodt commented

The V8 API requires that the list of properties on a JavaScript class be known when the class is created, so you have to use @property to expose properties to javascript.

tbodt commented

Actually that's not true, it looks like you can have dynamic properties by implementing __getitem__ and/or __setitem__.

Please not that this code worked fine with PyV8. I will look into __setitem__ in the morning and revert. Thanks for the quick reply :) Do let me know if you find anything out of place here.

tbodt commented

I'm not sure how PyV8 does it, I'll look into that.

@tbodt
Also, _document property is not being recognised by v8py, whereas navigator works fine.
This is really strange

When I removed the starting underscore from the name, it gave me this error.

Please note that window.navigator is modified to return an object of this class: https://github.com/kira0204/thug/blob/0803b4882dce8633434dd59c280c22aef3629f14/thug/DOM/W3C/Core/DOMImplementation.py#L14

$ thug -v -u win7ie90 -l misc/test1.html 
[2018-05-22 16:05:06] Context1: <thug.DOM.JSContext.JSContext object at 0x7fde99e347a0>
[2018-05-22 16:05:06] <script type="text/javascript">
      alert(window.history);
      alert(window.navigator);
      alert(window._document);
    </script>
[2018-05-22 16:05:06] 
      alert(window.history);
      alert(window.navigator);
      alert(window._document);
    
Context4: <thug.DOM.JSContext.JSContext object at 0x7fde99e347a0>
Context2: <thug.DOM.JSContext.JSContext object at 0x7fde99e347a0>
[2018-05-22 16:05:06] [Window] Alert Text: [object History]
#
# Fatal error in ../src/api.cc, line 1201
# Check failed: !value_obj->IsJSReceiver() || value_obj->IsTemplateInfo().
#
==== C stack trace ===============================
    /home/arushit/Desktop/v8py/_v8py.so(+0xa3680e) [0x7fdeb31d980e]
    /home/arushit/Desktop/v8py/_v8py.so(+0xd18c5) [0x7fdeb28748c5]
    /home/arushit/Desktop/v8py/_v8py.so(+0xa3523d) [0x7fdeb31d823d]
    /home/arushit/Desktop/v8py/_v8py.so(+0xf309c) [0x7fdeb289609c]
    /home/arushit/Desktop/v8py/_v8py.so(add_to_template(_object*, _object*, _object*, v8::Local<v8::FunctionTemplate>)+0x17b) [0x7fdeb286c79b]
    /home/arushit/Desktop/v8py/_v8py.so(add_class_to_template(_object*, v8::Local<v8::FunctionTemplate>)+0x9f) [0x7fdeb286c9bf]
    /home/arushit/Desktop/v8py/_v8py.so(py_class_new(_object*)+0x11f) [0x7fdeb286cb7f]
    /home/arushit/Desktop/v8py/_v8py.so(py_class_to_template(_object*)+0x58) [0x7fdeb286d008]
    /home/arushit/Desktop/v8py/_v8py.so(js_from_py(_object*, v8::Local<v8::Context>)+0x2e4) [0x7fdeb2870e74]
    /home/arushit/Desktop/v8py/_v8py.so(py_class_property_getter(v8::Local<v8::Name>, v8::PropertyCallbackInfo<v8::Value> const&)+0xde) [0x7fdeb28730de]
    /home/arushit/Desktop/v8py/_v8py.so(+0x38ab6c) [0x7fdeb2b2db6c]
    /home/arushit/Desktop/v8py/_v8py.so(+0x3d7836) [0x7fdeb2b7a836]
    /home/arushit/Desktop/v8py/_v8py.so(+0x3d6ee0) [0x7fdeb2b79ee0]
    /home/arushit/Desktop/v8py/_v8py.so(+0x37b63b) [0x7fdeb2b1e63b]
    /home/arushit/Desktop/v8py/_v8py.so(+0x382d0d) [0x7fdeb2b25d0d]
    [0x203e214843fd]
Received signal 4 ILL_ILLOPN 7fdeb31daa1f
Illegal instruction (core dumped)

tbodt commented

I think that's #13.

I'm seeing a lot of issues regarding the impedance mismatch between Python and JavaScript objects. Maybe the whole thing should be redesigned, making property access more dynamic? I'd like to compile a list of all these annoyances and see if I can redesign the thing to avoid them.

I think this would be the minimum repro case:

class Foo:
    def __init__(self):
        self.html_object = bs4.BeautifulSoup("foo.bar")
        context          = v8py.Context(self)

        context.eval("a = foo.bar")

    @property
    def bar(self):
        return self.html_object

    @property
    def foo(self):
        return self

I wonder @desertkun faced issues like this before :) ?

tbodt commented

Here's an even simpler repro case:

import bs4
import v8py
c = v8py.Context()
c.foo = bs4.BeautifulSoup()

BeautifulSoup objects apparently aren't compatible with v8py (#13). I really should redesign the thing...

Sure,
Thank you.

Hi,
curious to know whether there is any progress on this issue.
Thank you

tbodt commented

No, there hasn't. I've got a number of other projects I'm working on, so it's not all that likely that I'll get around to fixing this anytime soon, but if you'd like to take a stab at it you're more than welcome.

I believe #13 has nothing to do with contexts. I rewrote the code to always pass a context even in these cases, and it still crashes.

Found this bug on chromium:

that's intended - you can only set other templates or primitive values on templates, other values will result in broken code, and this CHECK() helps you to catch this earlier.
You can either create an accessor with a getter that returns the array, or a native data property that looks like a value but under the hood also calls an accessor.

So yeah, I guess we would have to resort to SetAccessor on the "otherwise just convert" case within the add_to_template method. Don't have time now to try that though.

OR

We could just skip non-primitive types for templates and it won't crash. The downside of this, obviously, is unexpected behavior for users. Still better than a crash, though.

tbodt commented

Oh, interesting.