patchGnash - The GNU Flash player - Patches: patch #8909, Patches to fix exception handling...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #8909: Patches to fix exception handling in Gnash's ExternalInterface implementation

Submitter:  Nutchanon Wetchasit <nachanon>
Submitted:  Thu 11 Feb 2016 02:26:19 PM UTC
   
 
Category:  plugin Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  strk Open/Closed:  Closed

Jump to the original submission

Wed 17 Feb 2016 11:20:31 AM UTC, comment #8: 

The persistent error seems to be a CGit error (some internal cache) -- I guess it should be reported upstream

Sandro Santilli <strk>
Group Member
Wed 17 Feb 2016 11:05:37 AM UTC, comment #7: 

Ccommits are now visible on git repository, thanks:


Anyway, CGit link of the first commit somehow persistently showing
"_Bad commit reference_" error, not sure why. Other three are displayed
properly. Nonetheless, `git pull` downloaded all of them correctly.

Nutchanon Wetchasit <nachanon>
Mon 15 Feb 2016 05:45:22 PM UTC, comment #6: 

Sorry, I forgot to push. Can you confirm you see them now ?

Sandro Santilli <strk>
Group Member
Mon 15 Feb 2016 05:04:44 PM UTC, comment #5: 

Is there any problem in submitted patches? They didn't seem to be visible on
Gnash's Savannah Git repository:

http://git.savannah.gnu.org/cgit/gnash.git/commit/?id=87b25aeebc1cc2ba9037f23db37d41b51965a40a
http://git.savannah.gnu.org/cgit/gnash.git/commit/?id=051aa9c34b69e1a0a1d2d75e3cf1db07fcea4006

Both URLs just gave the "Bad commit reference" error.

Commit from patch #8908 (GetVariable() path notation fix) is visible though.

Nutchanon Wetchasit <nachanon>
Mon 15 Feb 2016 10:35:20 AM UTC, comment #4: 

87b25aeebc1cc2ba9037f23db37d41b51965a40a^..051aa9c34b69e1a0a1d2d75e3cf1db07fcea4006

Sandro Santilli <strk>
Group Member
Thu 11 Feb 2016 03:20:07 PM UTC, comment #3: 

Patch 4 of 4:
See the attached `0004_remove-inband-player-externalinterface-return-value-check.patch`.

Current Gnash's implementation of `ExternalInterface.call()` treats string
return value "Error" and "SecurityError" as in-band indication of JavaScript error.
This causes legitimate string return value of these messages to be passed
to ActionScript as `undefined`.

This patch removes the check, allowing string "Error" and "SecurityError"
to be returned to ActionScript properly; completing the fix of bug #47004.

Once this patch is applied, `flash2js-errorstring.html` test from bug #47004
should show a correct result.

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


(file #36316)

Nutchanon Wetchasit <nachanon>
Thu 11 Feb 2016 03:13:11 PM UTC, comment #2: 

Patch 3 of 4:
See the attached `0003_send-javascript-error-to-player-as-undefined.patch`.

Current NPAPI libgnashplugin's JavaScript invocation code sends string-based
in-band failure notification to player when `NPN_Invoke()` returned failure.
Doing so makes ordinary string return value "Error", "SecurityError" got
treated as error status on player side, and is discarded.

This patch removes the check, allowing the default `undefined` value
to be passed back to the player in case of exception, which is a correct behavior;
partially fixing bug #47004.

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


(file #36315)

Nutchanon Wetchasit <nachanon>
Thu 11 Feb 2016 02:32:19 PM UTC, comment #1: 

Patch 2 of 4:
See the attached `0002_use-void-for-undefined-in-player-externalinterface-output.patch`.

Current code in Gnash (player)'s ExternalInterface data marshalling function
used "<undefined/>" string to represent `undefined` ActionScript value,
whereas others used "<void/>". This caused the `undefined` value returned
from ExternalInterface callback to be received by JavaScript as `null` instead.

This patch fixes the disagreeing `undefined` value representation, and
adjusts the associated container-emulated tests to follow the new representation.

Once this patch is applied, `js2flash-exception.html` test from bug #47004
should show a correct result.

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


(file #36314)

Nutchanon Wetchasit <nachanon>
Thu 11 Feb 2016 02:26:19 PM UTC, original submission:  

Current Gnash's ExternalInterface uses a string-based in-band signalling to
indicate failure on calling JavaScript or ActionScript function, by sending
"<string>Error</string>" or "<string>SecurityError</string>" as a return value
back to the calling party. This interferes with the legitimate String return
value "Error" and "SecurityError"; and also caused libgnashplugin's crash on
special return value a.k.a. bug #47004.

This series of patches address the issue by dropping the string-based in-band
signalling in libgnashplugin and Gnash player, and use a correct return value
(`undefined`) in case of exception or other failures.


Patch 1 of 4:
See the attached `0001_remove-bogus-plugin-externalinterface-return-value-check.patch`.

Original code in libgnashplugin tries to convert `NPVariant` return value
to string and check for magic error string. This fails when ActionScript code
actually return String "Error", "SecurityError"; and crashes when
ActionScript's return value is non-null and non-string, e.g. `NPObject` and `void`.

The patch removes the check, allowing real "Error", and "SecurityError"
string return value to reach JavaScript, and prevents libgnashplugin from
crashing when non-string return value is encountered;
partially fixing bug #47004, unblocking bug #32411.

Once this patch is applied, `js2flash-errorstring.html` test from bug #47004
should show a correct result.

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

Nutchanon Wetchasit <nachanon>

 

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

Attached Files
file #36316:  0004_remove-inband-player-externalinterface-return-value-check.patch added by nachanon (2KiB - text/x-diff - Patch to remove in-band error check of return value in `ExternalInterface.call()`)
file #36315:  0003_send-javascript-error-to-player-as-undefined.patch added by nachanon (2KiB - text/x-diff - Patch to make plugin send JavaScript error as `undefined` value)
file #36314:  0002_use-void-for-undefined-in-player-externalinterface-output.patch added by nachanon (3KiB - text/x-diff - Patch to fix ExternalInterface communication's representation of `undefined` value)
file #36313:  0001_remove-bogus-plugin-externalinterface-return-value-check.patch added by nachanon (1KiB - text/x-diff - Patch to remove plugin's bogus in-band error check on ExternalInterface callback return value)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by strk (Posted a comment)
  • -email is unavailable- added by nachanon (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-02-15 strk StatusNone Done
        Assigned toNone strk
        Open/ClosedOpen Closed
    2016-02-11 nachanon Attached File- Added 0004_remove-inband-player-externalinterface-return-value-check.patch, #36316
    2016-02-11 nachanon Attached File- Added 0003_send-javascript-error-to-player-as-undefined.patch, #36315
    2016-02-11 nachanon Attached File- Added 0002_use-void-for-undefined-in-player-externalinterface-output.patch, #36314
    2016-02-11 nachanon Attached File- Added 0001_remove-bogus-plugin-externalinterface-return-value-check.patch, #36313

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code