bugGnash - The GNU Flash player - Bugs: bug #32411, ExternalInterface XML parser fails...

 
 

bug #32411: ExternalInterface XML parser fails with nested tags

Submitted by:  Sandro Santilli <strk>
Submitted on:  Thu 10 Feb 2011 09:16:55 AM UTC  
 
Category: pluginSeverity: 3 - Normal
Release: NoneStatus: Ready For Test
Privacy: PublicAssigned to: Sandro Santilli <strk>
Open/Closed: Open

Add a New Comment (Rich MarkupRich Markup):
   

You are not logged in

Please log in, so followups can be emailed to you.

 

(Jump to the original submission Jump to the original submission)

Thu 03 Mar 2016 09:33:51 AM UTC, comment #27:

patch #8916 committed, looking forward for tests

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 23 Feb 2016 09:28:23 AM UTC, comment #26:

Maybe this would be useful: https://github.com/r3b/phantomjs

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 23 Feb 2016 08:18:46 AM UTC, comment #25:

The current libgnashplugin testing environment is indeed poorer than the standard gnash testing environment. I've been considering for some time to look at webkit but never found the time so I dunno if it would help shaping a more serious environment for testing NPAPI.

BTW, Google announced drop of NPAPI from their browsers, so it might be better to focus on browser support for outputting text to the console, to make the tests transparent to the implementation.

Ideally we'd have some plugin-supporting HTML/javascript interpreter that we could call from the commandline and catch stdout as all the other tests do. Maybe you could do some research on the topic.

Sandro Santilli <strk>
Project MemberIn charge of this item.
Mon 22 Feb 2016 02:27:21 PM UTC, comment #24:

Text output from `js2flash-objret.html` on browser using Gnash with patch #8916:
(Screenshot: file #36424)

You'd see that Object return value and its first-level members are
passed correctly, and the plugin does not crash.

Text output from `js2flash-rtypes.html` (bug #47004 file #36330) on browser using Gnash with patch #8916:
(Screenshot: file #36425)

You'd see that Object return value, including its first-level members
are passed correctly; elements in Array return value are also passed too,
(but disguised as an associative array with numeric key);
and the plugin does not crash.

Gnash: 0.8.11dev (patched against git c12c3cf 22-Feb-2016) NPAPI
Browser: Iceweasel 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

Nutchanon Wetchasit <nachanon>
Mon 22 Feb 2016 02:04:15 PM UTC, comment #23:

I have been looked around Gnash's NPAPI tester for a while, it seems that
making an automated test works for this case might be difficult, since in
current libgnashplugin testing environment:

  • Standalone tester doesn't seem to have access to the `GnashPluginScriptObject` and `NPP` plugin instance.
  • Standalone tester doesn't seem to have access to an JavaScript environment; calling host's `window.Object()` wouldn't work.

I'm not sure how real the NPAPI environment would need to be in order to run
these tests with meaningful result. (A lot of things could be emulated,
but they might not work the same as a real browser).

From current experimental fix `plugin-return-js-object-v2.patch`, one possible
stopgap measure is passing `NULL` as an `GnashPluginScriptObject` parameter
of `ExternalInterface::parseXML()`. This allows non-Object non-Array parsing
tests to continue working, but provide no real support for actually testing
this issue in automated way.

Updated `plugin-return-js-object-v2.patch` with this stopgap measure applied,
is included as patch #8916.

Gnash: 0.8.11dev (patched against git c12c3cf 22-Feb-2016) NPAPI
Browser: Iceweasel 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

Nutchanon Wetchasit <nachanon>
Mon 15 Feb 2016 10:38:31 AM UTC, comment #22:

This bug should be unblocked as patch #8909 was applied

Sandro Santilli <strk>
Project MemberIn charge of this item.
Tue 02 Feb 2016 03:08:46 PM UTC, comment #21:

The previously-posted `plugin-return-js-object.patch` has an error that
caused `null` to be passed to JavaScript instead of real Object, sorry.

The corrected test patch is now attached as `plugin-return-js-object-v2.patch`.

Screenshot from `js2flash-objret.html` under browser using Gnash with this patch:
file #36233

You'd see that the Object return value from ExternalInterface callback
and its members are passed correctly to JavaScript.

Gnash: 0.8.11dev (patched against git e705394 29-Jan-2016) NPAPI
Browser: Iceweasel 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

(file #36234)

Nutchanon Wetchasit <nachanon>
Tue 02 Feb 2016 02:26:05 PM UTC, comment #20:

I have done some trial and error changes on how libgnashplugin allocates NPObject
for returning as Object value from ExternalInterface callback:

  • Original code: crashed at garbage collection.
  • When got object return value from player, parse it, but then return `null` to browser instead: not crashing.
  • Zero-fill entire of new NPObject before use, then proceed normally: crashed at garbage collection.
  • Zero-fill entire of new NPObject before use, set dummy NPClass, then proceed normally: crashed at garbage collection.
  • Use `NPN_CreateObject()` to create object instead of `NPN_MemAlloc()`, with `NULL` as class and NPP instance: crashed at allocation.
  • Use `NPN_CreateObject()` to create object instead of `NPN_MemAlloc()`, with dummy NPClass, and `NULL` NPP instance: crashed at allocation.
  • Use `NPN_CreateObject()` to create object instead of `NPN_MemAlloc()`, with dummy NPClass, and real NPP instance: not crashing.
  • Call browser's `window.Object()` to create object instead of `NPN_MemAlloc()`: not crashing.

Last approach looks the best: apart from not crashing, `NPN_SetProperty()`
is now working, and JavaScript's `typeof()` also returns "object"
(instead of just "function"), first-level Object members are also
get passed to JavaScript properly too.

In case anyone want to test the last approach, see the attached `plugin-return-js-object.patch`.

Warning: This patch is experimental; it does work, but it breaks
libgnashplugin's automated test (compile error) as it hacked other parts of
plugin in order to pass `NPP` structure around, and it blatantly removed the
string conversion code which caused bug #47004 (otherwise it will crash).

Bug #47004 (at least on JavaScript-calls-Flash direction) would need to be
fixed in proper way before a fix for this issue could be carried out.

Gnash: 0.8.11dev (patched against git e705394 29-Jan-2016) NPAPI
Browser: Iceweasel 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

P.S. Removing the string conversion code mentioned in bug #47004
alone won't fix the crash in browser's garbage collection.

(file #36232)

Nutchanon Wetchasit <nachanon>
Tue 02 Feb 2016 06:39:59 AM UTC, comment #19:

For the crash problem, its backtrace looks like that the crash happened on
browser's object destroying (garbage collection?) process. I also found
that if I left the object parsing code in libgnashplugin working
but refrain from giving the resulting object to browser (give `null` instead),
the browser would not crash.

So this part of the issue could be narrowed down to how libgnashplugin
allocate or manipulate the NPObject, rather than about parsing of XML content.

Gnash: 0.8.11dev (git e705394 29-Jan-2016) NPAPI
Browser: Iceweasel 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

P.S. In case anyone want to try reproducing the experiment, see the attached
`plugin-return-null-on-object.patch`. This patch is only for testing
and (intentionally) cause memory leak; apply with care.

(file #36231)

Nutchanon Wetchasit <nachanon>
Tue 02 Feb 2016 05:52:34 AM UTC, comment #18:

According to current code, it seems that libgnashplugin has limited support
for nested XML value sent from Gnash (player); it supported array and object
with primitive-typed elements/members; if there's a nested array/object,
current parsing code would not find the corresponding end tag properly.

Gnash: 0.8.11dev (git e705394 29-Jan-2016) NPAPI
Iceweasel: 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

Nutchanon Wetchasit <nachanon>
Thu 28 Jan 2016 12:38:58 PM UTC, comment #17:

Array-Typed Function Return Value Case

This is an image result from `js2flash-arrret.html` under browser with Flash Player:
http://i.imgur.com/fltGcMx.png

Flash Player's trace output from the run:

You'll see that Flash Player passed ExternalInterface callback's Array return value, length and its sub-elements correctly.

This is an image result from `js2flash-arrjret.html` under browser with Gnash:
http://i.imgur.com/D83Zlmr.png

Gnash's console output shows:

You'll see from screenshot that Gnash did not pass Array-typed return value
from ExternalInterface callback correctly, and XULRunner crashed afterward.

Gnash: 0.8.11dev (git 62cfdfe 16-Jan-2016) NPAPI
Flash Player: 11.2.202.491 NPAPI binary
Iceweasel: 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

Nutchanon Wetchasit <nachanon>
Thu 28 Jan 2016 12:34:29 PM UTC, comment #16:

I also made a test Flash file for checking current ExternalInterface's
support for Array-typed callback argument and return type too (in JavaScript
calls Flash direction); attached as `js2flash-arr.zip`.

Screenshot, logs (verbosity 2 + action error), and relevant GDB backtrace
are also included in the same ZIP file.

Array-Typed Function Argument Case

This is an image result from `js2flash-arrarg.html` under browser with Flash Player:
http://i.imgur.com/1BcnPEL.png

Flash Player's trace output from the run:

You'll see that Flash Player passed an ExternalInterface callback's Array argument, length, and its elements correctly.

This is an image result from `js2flash-arrarg.html` under browser with Gnash:
http://i.imgur.com/7YY6GdA.png

Gnash's console output shows:

You'll see from console output that Gnash passed an Array-typed ExternalInterface callback argument as `undefined`.

According to Gnash's container-player communication log, the problem also
started from libgnashplugin sending an empty `<object>` tag to player,
which is incorrect since this array is supposed to have two elements.

Gnash: 0.8.11dev (git 62cfdfe 16-Jan-2016) NPAPI
Flash Player: 11.2.202.491 NPAPI binary
Iceweasel: 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

(file #36190)

Nutchanon Wetchasit <nachanon>
Mon 25 Jan 2016 01:23:29 PM UTC, comment #15:

YouTube's player used object (array) return type in ExternalInterface callback
causing nested value tag to be sent to plugin; now crashing libgnashplugin
as bug #37223 is fixed.

See Debian bug report: https://bugs.debian.org/811106

Communication log between plugin and player are also included
in Debian bug report.

Gnash: 0.8.11dev (git 62cfdfe 16-Jan-2016) NPAPI
Iceweasel: 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy i386

Nutchanon Wetchasit <nachanon>
Wed 06 Jan 2016 05:32:38 PM UTC, comment #14:

From test results and communication logs, it looks like that nested XML tags
parsing is needed on both side to support Object-typed (and Array-typed)
argument and return value in ExternalInterface like Adobe Flash Player do.

Note: There might be some corner cases on how to deal with looped or
duplicated Object references, but it would take some more testing to see
how Adobe Flash manages (or discards) it.

Nutchanon Wetchasit <nachanon>
Wed 06 Jan 2016 05:29:41 PM UTC, comment #13:

Object-Typed Function Return Value Case

This is an image result from `js2flash-objret.html` under browser with Flash Player:
http://i.imgur.com/KjPAvCU.png

Flash Player's trace output from the run:

You'll see that Flash Player passed ExternalInterface callback's Object return value and its members correctly.

This is an image result from `js2flash-objret.html` under browser with Gnash:
http://i.imgur.com/6nTqrHV.png

Gnash's console output shows:

You'll see from screenshot that Gnash did not pass Object-typed return value
from ExternalInterface callback correctly, and XULRunner crashed afterward.

According to Gnash's container-player communication log, Gnash (player) seems
to marshall the object properly, but libgnashplugin somehow choked on it.

Test SWFs, source code, build script, HTML container, screenshots, debug logs,
container-player communication dump and detailed GDB backtrace of xulrunner's
`plugin-container` are attached as `js2flash-obj.zip`.

Gnash: 0.8.11dev (git 15d61a8 26-Dec-2015) NPAPI
Flash Player: 11.2.202.491 NPAPI binary
Browser: Iceweasel 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy

(file #35935)

Nutchanon Wetchasit <nachanon>
Wed 06 Jan 2016 05:24:27 PM UTC, comment #12:

This is a follow up from bug #37223 comment 4.

I have created dummy SWF files to minimally demonstrate the need for this
functionality. One of SWF file receives an Object argument from JavaScript,
another one tries to pass Object return value to JavaScript.

The Object passed is just a simple Object created from `new Object()`
statement (from both language), then assigned a member named `a` with string
value "Hello", and assigned a member named `b` with string value "World".

Object-Typed Function Argument Case

This is an image result from `js2flash-objarg.html` under browser with Flash Player:
http://i.imgur.com/cfDCgOi.png

Flash Player's trace output from the run:

You'll see that Flash Player passed an ExternalInterface callback's Object argument and its members correctly.

This is an image result from `js2flash-objarg.html` under browser with Gnash:
http://i.imgur.com/rNCDxOu.png

Gnash's console output shows:

Apart from hairline stroke issue (bug #45731), you'll see from console output
that Gnash passed an Object-typed ExternalInterface callback argument as `undefined`.

According to Gnash's container-player communication log, the problem started
from libgnashplugin sending an empty `<object>` tag to player, which is
incorrect since this object is supposed to have two member.

Gnash: 0.8.11dev (git 15d61a8 26-Dec-2015) NPAPI
Flash Player: 11.2.202.491 NPAPI binary
Browser: Iceweasel 10.0.12 (debian)
System: Debian GNU/Linux 7.0 Wheezy

Nutchanon Wetchasit <nachanon>
Sat 05 Mar 2011 08:39:30 PM UTC, comment #11:

The test uses an xml received by our plugin from youtube, as shown by the backtrace attached to this ugly bug here:
https://savannah.gnu.org/bugs/index.php?31766#comment10

Sandro Santilli <strk>
Project MemberIn charge of this item.
Sat 05 Mar 2011 05:35:40 PM UTC, comment #10:

The plugin shouldn't be using any headers from Gnash core. I see only StringPredicates.h and GnashSystemIOHeaders.h, which I guess are safe. AS3 did add more functionality to the API. But the point is I don't believe ExternalInterface is supposed to ever handle nested tags, so this bug report could be bogus, and based on a broken test case.

Rob Savoye <rsavoye>
Project Administrator
Sat 05 Mar 2011 05:30:09 PM UTC, comment #9:

plugin.cpp does in fact use some headers from libbase/.

Anyway, it looks to me as if the ExternalInterface (according to specification) permits controlling actionscript via the XML invoke specfication which we are using, but only in AS3. AS2 just defines the call() and addCallback() methods. Please correct me if I'm wrong.

I think it is safe to resolve this as postponed until we change the plugin and/or when we implement AS3.

Bastiaan Jacques <bjacques>
Project Member
Sat 05 Mar 2011 05:15:17 PM UTC, comment #8:

Currently the plugin uses no headers from the rest of gnash. The debug functions are actually different, just have the same names.
Sharing header only code for XML parsing is fine, it just isn't a big saving in code. You're welcome to implement such a thing if you want, but as ExternalInterface is currently working fine with the new YouTube API, it seems there are more important things to be working on. Maybe it's time for somebody to start on a real plugin. Adobe uses ExternalInterface for the flash debugger, which is the entire reason it's an XML networking protocol.

Rob Savoye <rsavoye>
Project Administrator
Sat 05 Mar 2011 05:06:04 PM UTC, comment #7:

rsavoye, what I was suggesting was putting the XML parsing code in a header file, which can be compiled into plugin code like other headers from libbase/ presently are.

But it seems wasteful to spend time on this if the plugin <-> Gnash communication itself is going to be irrelevant when we put Gnash itself into the browser namespace as a real plugin.

I am certainly not convinced ExternalInterface is a useful way to remotely control Gnash.

Bastiaan Jacques <bjacques>
Project Member
Sat 05 Mar 2011 04:53:11 PM UTC, comment #6:

EsxternalInterface doesn't pass JavaScript at all, only the XML messages as specified by the Adobe spec. Even with a traditional style plugin, we'd still want the optional ability to use a network to remotely control Gnash when running standalone. I don't think ExternalInterface is supposed to deal with nested tags, the test case could be trying to test functionality that may never been required. Before talking about how ExternalInterface works, it'd be easier if people actually researched this first.

Rob Savoye <rsavoye>
Project Administrator
Sat 05 Mar 2011 04:43:52 PM UTC, comment #5:

That's what needs to be done for the ExternalInterface implementation; it would both simplify the code handling it and achieve much better compatibility. The plugin should just be passing XML (and possibly JavaScript; I haven't checked the details recently) to a couple of native functions, which then distribute it to the appropriate built-in AS functions.

But the 'invoke' notion doesn't just handle ExternalInterface, but also some other things like getURL and quitting, which would need to be separated out.

A good time to do this would be when writing a real plugin, because then the fd reading becomes superfluous anyway.

Benjamin Wolsey <bwy>
Project Member
Sat 05 Mar 2011 04:41:38 PM UTC, comment #4:

The plugin shouldn't have any dependencies on core Gnash code unless we ever create a traditional style plugin. While it might be possible to use XML_as.cpp on the Gnash side, the XML used for the ExternalInterface messages in the plugin is much simpler, and deals with NPVariants, instead of ActionScript objects.

Rob Savoye <rsavoye>
Project Administrator
Sat 05 Mar 2011 04:37:39 PM UTC, comment #3:

Thanks.

I'm curious why there is XML parsing logic in external.{h,cpp}; wouldn't it make sense to stick the XML parsing code from XML_as.cpp (or even from some external XML parsing library) in libbase/ and use that to parse the invoke requests?

Bastiaan Jacques <bjacques>
Project Member
Sat 05 Mar 2011 03:38:06 PM UTC, comment #2:

It's plugin/npapi/test.cpp. There are two sets of files required for EsternalInterface to work, the plugin side in plugin/npapi, and the Gnash side in libcore.

Rob Savoye <rsavoye>
Project Administrator
Sat 05 Mar 2011 03:35:50 PM UTC, comment #1:

Does ExternalInterface refer to libcore/ExternalInterface.{h,cpp} or to plugin/npapi/external.{h,cpp}, or both?

Where do I find test.c?

Bastiaan Jacques <bjacques>
Project Member
Thu 10 Feb 2011 09:16:55 AM UTC, original submission:

As shown by test.c, the parseArguments function fails when one of the arguments is an object (ie: contains sub-elements)

Sandro Santilli <strk>
Project MemberIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #36425:  js2flash-rtypes_gnash0.8.11dev-c12c3cf.patched1.png added by nachanon (7kB - image/png - Screenshot of `js2flash-rtypes.html` on browser using Gnash 0.8.11dev git c12c3cf with first patch of patch #8916)
file #36424:  js2flash-objret_gnash0.8.11dev-c12c3cf.patched1.png added by nachanon (2kB - image/png - Screenshot of `js2flash-objret.html` on browser using Gnash 0.8.11dev git c12c3cf with first patch of patch #8916)
file #36234:  plugin-return-js-object-v2.patch added by nachanon (12kB - text/x-patch - Test patch to make libgnashplugin return real JavaScript `Object` (fixed `null` error))
file #36233:  js2flash-objret_gnash0.8.11dev-e705394_plugin-return-js-object-v2.png added by nachanon (2kB - image/png - Screenshot from `js2flash-objret.html` with `plugin-return-js-object-v2.patch`)
file #36232:  plugin-return-js-object.patch added by nachanon (12kB - text/x-patch - Test patch to make libgnashplugin return real JavaScript `Object`)
file #36231:  plugin-return-null-on-object.patch added by nachanon (697B - text/x-patch - Test patch to make libgnashplugin return `null` to JavaScript instead of `Object`)
file #36190:  js2flash-arr.zip added by nachanon (25kB - application/zip - Test Flash files (Array argument/return case), source code, screenshots and logs)
file #35935:  js2flash-obj.zip added by nachanon (24kB - application/zip - Test Flash files (Object argument/return case), source code, screenshots and logs)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by nachanon (Posted a comment)
  • -unavailable- added by bwy (Posted a comment)
  • -unavailable- added by rsavoye (Updated the item)
  • -unavailable- added by bjacques (Posted a comment)
  • -unavailable- added by strk (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 12 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 03 Mar 2016 09:33:51 AM UTCstrkStatusPostponed=>Ready For Test
      Assigned toNone=>strk
    Mon 22 Feb 2016 02:13:07 PM UTCnachanonAttached File-=>Added js2flash-rtypes_gnash0.8.11dev-c12c3cf.patched1.png, #36425
    Mon 22 Feb 2016 02:11:16 PM UTCnachanonAttached File-=>Added js2flash-objret_gnash0.8.11dev-c12c3cf.patched1.png, #36424
    Tue 02 Feb 2016 03:08:46 PM UTCnachanonAttached File-=>Added plugin-return-js-object-v2.patch, #36234
    Tue 02 Feb 2016 03:05:15 PM UTCnachanonAttached File-=>Added js2flash-objret_gnash0.8.11dev-e705394_plugin-return-js-object-v2.png, #36233
    Tue 02 Feb 2016 02:26:05 PM UTCnachanonAttached File-=>Added plugin-return-js-object.patch, #36232
    Tue 02 Feb 2016 06:39:59 AM UTCnachanonAttached File-=>Added plugin-return-null-on-object.patch, #36231
    Thu 28 Jan 2016 12:34:29 PM UTCnachanonAttached File-=>Added js2flash-arr.zip, #36190
    Wed 06 Jan 2016 05:29:41 PM UTCnachanonAttached File-=>Added js2flash-obj.zip, #35935
    Sat 05 Mar 2011 05:30:09 PM UTCbjacquesStatusConfirmed=>Postponed
    Sat 05 Mar 2011 03:38:06 PM UTCrsavoyeAssigned torsavoye=>None

    Back to the top


    Powered by Savane 3.1-cleanup