Liveconnect bug

[Koos Vriezen]

On Thu, Aug 05, 2004 at 11:09:42AM -0400, George Staikos wrote:
> On Thursday 29 July 2004 16:02, Koos Vriezen wrote:
> > On Wed, Jul 28, 2004 at 05:54:35PM -0400, George Staikos wrote:
> > > There appears to be a bug in the liveconnect integration in KHTML. 
> > > Sometimes it seems that the liveconnect pointer is not found and/or
> > > stored properly by KHTML.  The result is that liveconnect queries come up
> > > empty.  An example site where this happens is http://www.jibjab.com/. 
> > > Start the flash, and a new window will appear.  It will show a flash,
> > > then when it ends, rewrite the document and start a third flash.  The
> > > third one makes a JS call ("javascript:foo") and gets an empty result
> > > back.  Anyone have an idea why?
> >
> > No, but you might add some debug at HTMLObjectBaseElementImpl::put and
> > HTMLObjectBaseElementImpl::setLiveConnect (html/html_objectimpl.cpp) to see
> > if the return value is set to the same liveconnect extension that the
> > plugin has.
> 
>    No help so far.  Anyway this is a pretty major bug as far as KHTML is 
> concerned.  We should entirely disable liveconnect for plugins (and therefore 
> disable flash) if we can't fix this for 3.3.  The number of crashes I'm 
> seeing on "everyday websites" is rather large, though in many cases it 
> doesn't "crash" right away, but it does corrupt itself internally.

I haven't seen any crashes lately with liveconnect and KJAS. Why do you
want to disable it. At least tell where the problem is or what clues you
have that point to liveconnect (valgrind/bug reports/backtraces/test
cases/). This liveconnect code in khtml hasn't changed much since 3.1 and is
unchanged compared to 3.2.3.

The case you mentioned makes for me the nspluginviewer crash. It has the
bt as in bug 81401 (which is likely a dangling pointer in
NSPluginInstance::_streams)...hmm after updating, I don't get that one
anymore though.. But anyway, this cannot corrupt khtml.

I took a quick look at the nsplugin code and noticed that there is no
protection against a script that destroys the calling plugin. I've
attached a patch for that. Also a static_cast -> dynamic_cast change.

So please backup your claims.

Koos
-------------- next part --------------
Index: plugin_part.cpp
===================================================================
RCS file: /home/kde/kdebase/nsplugins/plugin_part.cpp,v
retrieving revision 1.56
diff -u -3 -p -r1.56 plugin_part.cpp
--- plugin_part.cpp	28 Jul 2004 19:55:03 -0000	1.56
+++ plugin_part.cpp	6 Aug 2004 14:52:38 -0000
@@ -29,7 +29,7 @@ public:
 
 
 PluginLiveConnectExtension::PluginLiveConnectExtension(PluginPart* part) 
-: KParts::LiveConnectExtension(part), _part(part) {
+: KParts::LiveConnectExtension(part), _part(part), _retval(0L) {
 }
 
 PluginLiveConnectExtension::~PluginLiveConnectExtension() {
@@ -53,8 +53,8 @@ Q_UNUSED(value);
 
 bool PluginLiveConnectExtension::put( const unsigned long, const QString &field, const QString &value) {
     kdDebug(1432) << "PLUGIN:LiveConnect::put " << field << " " << value << endl;
-    if (field == "__nsplugin") {
-        __nsplugin = value;
+    if (_retval && field == "__nsplugin") {
+        *_retval = value;
         return true;
     } else if (field.lower() == "src") {
         _part->changeSrc(value);
@@ -71,8 +71,10 @@ QString PluginLiveConnectExtension::eval
     jscode.sprintf("this.__nsplugin=eval(\"%s\")",  QString(script).replace('\\', "\\\\").replace('"', "\\\"").latin1());
     //kdDebug(1432) << "String is [" << jscode << "]" << endl;
     args.push_back(qMakePair(KParts::LiveConnectExtension::TypeString, jscode));
+    QString nsplugin("Undefined");
+    _retval = &nsplugin;
     emit partEvent(0, "eval", args);
-    return __nsplugin;
+    return nsplugin;
 }
 
 extern "C"
@@ -179,7 +181,8 @@ KAboutData *PluginFactory::aboutData()
 
 PluginPart::PluginPart(QWidget *parentWidget, const char *widgetName, QObject *parent,
                        const char *name, const QStringList &args)
-    : KParts::ReadOnlyPart(parent, name), _widget(0), _args(args)
+    : KParts::ReadOnlyPart(parent, name), _widget(0), _args(args),
+      _destructed(0L)
 {
     setInstance(PluginFactory::instance());
     kdDebug(1432) << "PluginPart::PluginPart" << endl;
@@ -217,6 +220,8 @@ PluginPart::~PluginPart()
 
     delete _callback;
     _loader->release();
+    if (_destructed)
+        *_destructed = true;;
 }
 
 
@@ -338,10 +343,17 @@ void PluginPart::evalJavaScript(int id, 
 {
     kdDebug(1432) <<"evalJavascript: before widget check"<<endl;
     if (_widget) {
+        bool destructed = false;
+        _destructed = &destructed;
 	kdDebug(1432) <<"evalJavascript: there is a widget" <<endl;	
         QString rc = _liveconnect->evalJavaScript(script);
+        if (destructed)
+            return;
+        _destructed = 0L;
         kdDebug(1432) << "Liveconnect: script [" << script << "] evaluated to [" << rc << "]" << endl;
-        static_cast<NSPluginInstance*>((QWidget*)_widget)->javascriptResult(id, rc);
+        NSPluginInstance *ni = dynamic_cast<NSPluginInstance*>(_widget.operator->());
+        if (ni)
+            ni->javascriptResult(id, rc);
     }
 }
 
Index: plugin_part.h
===================================================================
RCS file: /home/kde/kdebase/nsplugins/plugin_part.h,v
retrieving revision 1.27
diff -u -3 -p -r1.27 plugin_part.h
--- plugin_part.h	27 Jan 2004 04:08:17 -0000	1.27
+++ plugin_part.h	6 Aug 2004 14:52:38 -0000
@@ -108,6 +108,7 @@ private:
   NSPluginCallback *_callback;
   QStringList _args;
   class NSPluginLoader *_loader;
+  bool *_destructed;
 };
 
 
@@ -127,8 +128,8 @@ signals:
     virtual void partEvent( const unsigned long objid, const QString & event, const KParts::LiveConnectExtension::ArgList & args );
 
 private:
-    QString __nsplugin;
     PluginPart *_part;
+    QString *_retval;
 };