v8py icon indicating copy to clipboard operation
v8py copied to clipboard

JSClass and JS objects

Open armudgal opened this issue 6 years ago • 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

armudgal avatar May 21 '18 19:05 armudgal

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 avatar May 21 '18 19:05 tbodt

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

tbodt avatar May 21 '18 19:05 tbodt

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.

armudgal avatar May 21 '18 20:05 armudgal

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

tbodt avatar May 21 '18 20:05 tbodt

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

armudgal avatar May 22 '18 10:05 armudgal

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

tbodt avatar May 22 '18 16:05 tbodt

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)

armudgal avatar May 22 '18 17:05 armudgal

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.

tbodt avatar May 22 '18 17:05 tbodt

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 :) ?

armudgal avatar May 22 '18 18:05 armudgal

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

tbodt avatar May 22 '18 18:05 tbodt

Sure, Thank you.

armudgal avatar May 23 '18 09:05 armudgal

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

armudgal avatar Jun 11 '18 14:06 armudgal

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.

tbodt avatar Jun 11 '18 18:06 tbodt

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.

desertkun avatar Jun 12 '18 14:06 desertkun

Oh, interesting.

tbodt avatar Jun 12 '18 23:06 tbodt