203704 - [AxPlugin] Cleanup hosting flags issues, #define's and build flags

Status: RESOLVED INVALID

(Core Graveyard :: Embedding: APIs, defect)

Description

[Adam Lock]

The plugin should be secure by default in the absence of hosting flags to the
contrary. It should also ask the dispatch support object for the hosting flags,
not grab them from the prefs.

The code has a mess of #ifdef's so it should be made a bit saner if possible.

Comment 1

[Adam Lock]

Attachment: Patch

Patch cleans up the #define's a bit by defining a new
MOZ_ACTIVEX_PLUGIN_COMCONNECT to replace XPC_IDISPATCH_SUPPORT. The patch also
doesn't build PrefObserver.cpp in this mode and always asks the dispatch
support object for the hosting flags. I've moved a couple of functions in the
code around so the patch might look a bit bigger than it really is. Mostly it's
just the proper application of #ifdefs.

I've tested with and without activex.js & nsAxSecurityPolicy.js and it works as
expected.

Patch also builds cleanly in the fixed XPCOM interface
MOZ_ACTIVEX_PLUGIN_XPCONNECT mode, adds some description to the Makefile.in of
the differences and resolves a namespace clash by renaming the MozAxPlugin
namespace as ActiveXPlugin to stop interfering with the MozAxPlugin class
generated by Java when built in MOZ_ACTIVEX_PLUGIN_LIVECONNECT mode.
LiveConnect still has some other JRI issues but I'll deal with them in a
supplemental patch at some point.

Comment 2

[Adam Lock]

Comment on attachment 121929 [details] [diff] [review]
Patch

Requesting r/sr on this patch please. The patch is mostly #ifdef reorganisation
to ensure the plugin uses the dispatch support service in COM connect mode and
other means when built in legacy modes.

Comment 3

[David Bradley]

Is this correct? Specifically the else path just doesn't look like it results in
correct code.

         PRBool isObjectSafe = PR_FALSE;
         if (!cpUnk ||
+#ifdef MOZ_ACTIVEX_PLUGIN_COMCONNECT
             NS_FAILED(dispSupport->IsObjectSafeForScripting(
                 reinterpret_cast<void *>(cpUnk.p), iidDispatch, &isObjectSafe)) ||
+#else
+            !pSite->IsObjectSafeForScripting(__uuidof(IDispatch)))
+            return NPERR_GENERIC_ERROR;
+#endif
             !isObjectSafe)

Comment 4

[Adam Lock]

Yes this should be correct. When the plugin builds in XP or Live but not COM
connect modes, the plugin must ask the site class implementation of the object
if it is safe for scripting since there is no service. The method returns a BOOL
so !pSite->IsObjectSafeForScripting(...) is the equivalent to
NS_FAILED(dispSupport->IsObjectSafeForScripting(...)) in the other part of the
#ifdef.

Comment 5

[David Bradley]

Playing the role of preprocessor, the #else case would generate code like:

         PRBool isObjectSafe = PR_FALSE;
         if (!cpUnk ||
            !pSite->IsObjectSafeForScripting(__uuidof(IDispatch)))
            return NPERR_GENERIC_ERROR;
             !isObjectSafe)

I'm not sure how that's going to parse correctly.

Comment 6

[Adam Lock]

Ugh, I managed to completely not build at all in the backwards compatible mode.

I think I will push this one out since from inspection of how the prefobserver
behaves I believe the plugin should be okay here. It does ask the dispatch
support object for the hosting flags but caches them and resyncs when it is
notified of changes in the prefs. Since the prefs won't change and it doesn't
read them directly it should be fine.

Comment 7

[Adam Lock]

Comment on attachment 121929 [details] [diff] [review]
Patch

Canceling r/sr request

Comment 8

[Adam Lock]

Attachment: Patch 2

A much better patch fixes the silly bustage. I might work on this a bit more
just to see if I can get all modes including LiveConnect to build.

Comment 9

[Adam Lock]

*** Bug 204879 has been marked as a duplicate of this bug. ***

Comment 10

[Adam Lock]

Cleanup nsIMozAxPlugin #include, compile, export mess too.

Comment 11

[Ed Morley [:emorley]]

The ActiveX embedding API was removed in bug 662023 and friends.

-> INVALID

[Filter bugspam on activexinvalid]