Summary: | Core Animation plugin layers are incorrectly released | ||
---|---|---|---|
Product: | WebKit | Reporter: | Stuart Morgan <stuartmorgan> |
Component: | Plug-ins | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED WONTFIX | ||
Severity: | Major | CC: | ahmad.saleem792, andersca, ap, rniwa, simon.fraser, taxilian |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Mac | ||
OS: | OS X 10.6 | ||
Bug Depends on: | 61948 | ||
Bug Blocks: |
Description
Stuart Morgan
2011-04-11 15:55:09 PDT
Or maybe we just change the spec. We are currently working around this in FireBreath (http://www.firebreath.org) but it would sure be nice if we could just implement it to spec and it would work. (In reply to comment #1) > Or maybe we just change the spec. I find it interesting that the email addresses of the people on the spec are @apple.com, and it's a spec that has been "Accepted" and published for quite some time -- enough time that two other major browsers have adopted it and added support. I really, *really* hope that you were joking. It's bad enough trying to support all the weird differences in implementations between major browsers without those browsers doing it on purpose... (In reply to comment #1) > Or maybe we just change the spec. Can you guarantee that nobody implemented a plugin that does UA checks, and does the right thing in browsers other than Safari? I'm not excited about the idea of releasing a version of Chrome where plugins that were built to spec suddenly start crashing, and I'm guessing the same would be true of Firefox. You and Kevin were both involved in the discussion on plugin-futures about making that change to the spec; Kevin explicitly agreed that we should make the change, and you didn't object when Josh gave the new language that was going in. You even made the change to the sample code on the wiki IIRC. It's not like this is something that slipped through unnoticed by accident. If you had argued then that your having an implementation already shipped meant the change shouldn't be made, that would be one thing (although it would be nice if pre-spec implementations used some made-up constant to prevent that kind of thing), but now that there are several other shipped implementations that *do* match the spec it's a bit late for that. This is apparently fixed in WebKit2. I want to capture here what I've learned from discussions I've had with various plugin vendors since filing this, to make it clear that this is *very* hard for plugins to work around correctly: - If a plugin follow the spec as written, every single WebKit.framework-based app will crash if it loads that plugin - If a plugin vendor ignores the spec, it will leak CA layer in every compliant browser. - The seemingly logic approach of special-casing browsers that do the wrong thing by doing a UA-check to do the retain is, as far as I can tell, impossible to get right. It's impossible to enumerate the apps that are wrong because every WebKit.framework-based app is wrong, and they could have basically any UA. That leaves whitelisting browsers that *aren't* broken, which is horrible. It means anyone who writes a browser that implements the spec has to get every CA plugin to special-case them, or satisfy the checks done by every plugin (which of course a browser developer won't even know because most plugins are closed-source). And even if we accept that, it's still not clear what check a plugin vendor should actually do. If the plugin looks for "WebKit", it leaks in WebKit2 and in anything Chromium based. So it has to look for WebKit, but not WebKit2, and not Chrome. But any number of apps could be Chromium-based (and at least a couple of other browsers are), and they may not have Chrome in their UA, so then they leak too. And meanwhile any app without WebKit in its UA that uses WebKit.framework will crash. I considered suggesting to plugin vendors that they look for WebKit.framework being loaded in the app--but that doesn't work because there are plugins that themselves load WebKit.framework, and most apps don't have OOP plugins. Apple could make this whole mess go away by shipping a fixed version of WebKit.framework in 10.6.x. Then all a plugin vendor would have to do is check the OS version (or maybe the WebKit version) and not retain for 10.6.x+. The only downside to this is that until plugin vendors updated, things would leak in WebKit.framework apps. That seems a whole lot better than WebKit.framework apps maybe leaking, maybe crashing, and maybe being okay, indefinitely, based on what each individual plugin vendor checks for. Especially given that the number of plugins using CA is still quite small, and evangelizing all of them to add an OS-version-based check would be quite easy. (In reply to comment #6) > - The seemingly logic approach of special-casing browsers that do the wrong thing by doing a UA-check to do the retain is, as far as I can tell, impossible to get right. It's impossible to enumerate the apps that are wrong because every WebKit.framework-based app is wrong, and they could have basically any UA. I had a thought today; I'm not sure why this had never occurred to me before: instead of looking at the UA, a plugin vendor can look for support for the Invalidating Core Animation model. Since any non-Apple implementation of a CA plugin host would want to support ICA for performance reasons, plugin vendors could reasonably assume that a browser that doesn't support ICA must be WebKit.framework-based, and only then violate the spec by leaking the layer. That solves the enumeration problem, and means that it's not fragile to users doing UA spoofing. NPAPI support is removed from Safari 14 onward and it is not supported in Webkit Builds like WebkitGTK as well. I think this can be marked as "RESOLVED WONTFIX". Thanks! Yup, won't fix. |