tbodt / v8py

Write Python APIs, then call them from JavaScript using the V8 engine.
GNU Lesser General Public License v3.0
443 stars 29 forks source link

JSClass and JS objects #24

Open armudgal opened 6 years ago

armudgal commented 6 years ago

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 6 years ago

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 6 years ago

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

armudgal commented 6 years ago

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 6 years ago

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

armudgal commented 6 years ago

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

tbodt commented 6 years ago

https://github.com/tbodt/v8py/blob/master/v8py/pyclass.cpp#L204

armudgal commented 6 years ago

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 6 years ago

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.

armudgal commented 6 years ago

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 6 years ago

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...

armudgal commented 6 years ago

Sure, Thank you.

armudgal commented 6 years ago

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

tbodt commented 6 years ago

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.

desertkun commented 6 years ago

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 6 years ago

Oh, interesting.