mario0alberto1 / gecko-mediaplayer

Automatically exported from code.google.com/p/gecko-mediaplayer
GNU General Public License v2.0
0 stars 0 forks source link

Reduce function table checks to what is really needed #184

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
The attached patch reduced the function table checks to what is really needed. 
The size needs to large enough to hold the last member the plugin wants to 
access. Nothing more is needed.

(This should fix compatibility issues if the plugin has been built against a 
newer Firefox than it is used with and maybe other plugin-capable browsers.)

Original issue reported on code.google.com by s.ramac...@gmail.com on 17 Jun 2013 at 7:31

Attachments:

GoogleCodeExporter commented 8 years ago
I get why you have submitted this patch, but is this a theoretical problem or 
has someone run against this in a real world situation? 

Original comment by kdeko...@gmail.com on 18 Jun 2013 at 2:57

GoogleCodeExporter commented 8 years ago
We have the problem in Debian: 
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=706154#101 and the following 
messages. gecko-mediaplayer is built against Firefox 17 (in unstable) but 
testing still has Firefox 10. I also think this would fix 
https://bugs.launchpad.net/ubuntu/+source/gecko-mediaplayer/+bug/870115 and 
https://bugs.launchpad.net/ubuntu/+source/gecko-mediaplayer/+bug/1069635.

Original comment by s.ramac...@gmail.com on 18 Jun 2013 at 7:54

GoogleCodeExporter commented 8 years ago
I believe I am going to reject this patch on the following reasons

1. That code is from Mozilla, I prefer not to touch it
2. Running a plugin compiled for F17 in F10 is a mistake and I don't think that 
should be supported
3. Chrome has had a horrible time supporting mozilla plugins and so I see no 
reason to work around their problem code (they black listed my plugin for years 
due to problems in their code)

Original comment by kdeko...@gmail.com on 18 Jun 2013 at 2:23

GoogleCodeExporter commented 8 years ago
JFTR: Looking at some of the plugin code shipped in Firefox, this seems to be 
the preferred way now: 
http://sources.debian.net/src/iceweasel/21.0-1/dom/plugins/test/testplugin/nptes
t.cpp#L672

Original comment by s.ramac...@gmail.com on 23 Jun 2013 at 6:21

GoogleCodeExporter commented 8 years ago
I put in some code to address this, see if it works. The patch you provided may 
have generated exceptions in some instances.

Original comment by kdeko...@gmail.com on 23 Jun 2013 at 7:02

GoogleCodeExporter commented 8 years ago
Oh, thanks. Indeed, I missed that setexception comes after getvalue. Thanks! 
I'll let you knew as soon as I get some feedback.

Original comment by s.ramac...@gmail.com on 24 Jun 2013 at 9:51

GoogleCodeExporter commented 8 years ago
I checked the wrong pointer in the first patch. I'm not using 
http://anonscm.debian.org/gitweb/?p=pkg-multimedia/gecko-mediaplayer.git;a=blob;
f=debian/patches/functable-check.patch;h=3b7d9cc0c0af6374dcefad12d3e8ef1d50501e6
7;hb=HEAD

Original comment by s.ramac...@gmail.com on 24 Jun 2013 at 11:02

GoogleCodeExporter commented 8 years ago
I enhanced my patch abit and put it in SVN.

Original comment by kdeko...@gmail.com on 24 Jun 2013 at 12:37

GoogleCodeExporter commented 8 years ago

Original comment by kdeko...@gmail.com on 2 Aug 2013 at 12:51