253570 - Add NPP arguments to some of the new NPP_ npruntime functions

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Johnny Stenback (:jst)]

As per discussion on the plugin-futures mailing list. This is needed to make
things cleaner and to help with defence in depth against security exploits.
Patch coming up.

Comment 1

[Johnny Stenback (:jst)]

Attachment: Add NPP arguments, and make use of them. Also add initial support for exception throwing from plugins.

Comment 2

[Brendan Eich [:brendan]]

Comment on attachment 154668 [details] [diff] [review]
Add NPP arguments, and make use of them. Also add initial support for exception throwing from plugins.

>+void
>+PopException()
>+{
>+  NS_ASSERTION(!gNPPException, "Uh, no NPP exception to pop!");
>+
>+  if (gNPPException) {
>+    free(gNPPException);

Doncha wanna null gNPPException after the free?

>+++ modules/plugin/base/src/ns4xPluginInstance.h	29 Jul 2004 17:31:54 -0000
>@@ -157,18 +157,24 @@ public:
>     virtual ~ns4xPluginInstance(void);
> 
>     // returns the state of mStarted
>     PRBool IsStarted(void);
> 
>     // cache this 4.x plugin like an XPCOM plugin
>     nsresult SetCached(PRBool aCache) { mCached = aCache; return NS_OK; };
> 
>     virtual JSObject *GetJSObject(JSContext *cx);
>+
>+    // Non-refcounting accessor for faster access to the peer...
>+    nsIPluginInstancePeer *GetPeer()
>+    {
>+        return mPeer;
>+    }

If mPeer must be non-null for any fully-constructed instance of
ns4xPluginInstance on which GetPeer might be called, use the new
deCOMtamination name scheme: Peer(), no Get prefix.

> static JSContext *
> GetJSContext(NPP npp)
> {
>+  NS_ENSURE_TRUE(npp, nsnull);
>+
>   ns4xPluginInstance *inst = (ns4xPluginInstance *)npp->ndata;
>   NS_ENSURE_TRUE(inst, nsnull);
> 
>-  nsCOMPtr<nsIPluginInstancePeer> pip;
>-  inst->GetPeer(getter_AddRefs(pip));
>-  nsCOMPtr<nsPIPluginInstancePeer> pp (do_QueryInterface(pip));
>+  nsCOMPtr<nsPIPluginInstancePeer> pp(do_QueryInterface(inst->GetPeer()));

Aha, no null check!  So inst->Peer() wins.

>-  return GetProperty(npjsobj->mCx, npjsobj->mJSObj, identifier, &v) &&
>-    JSValToNPVariant(npjsobj->mNpp, npjsobj->mCx, v, result);
>+  return GetProperty(cx, npjsobj->mJSObj, identifier, &v) &&
>+    JSValToNPVariant(npp, cx, v, result);

Nit: multiline return value always looks more readable to me if the overflow
lines are indented to line up with the return expression's first char.

> PR_STATIC_CALLBACK(PRBool)
>-NPObjWrapperHashMatchEntry(PLDHashTable *table, const PLDHashEntryHdr *entry,
>+JSObjWrapperHashMatchEntry(PLDHashTable *table, const PLDHashEntryHdr *entry,
>                            const void *key)
> {
>+  const nsJSObjWrapperKey *ke = NS_STATIC_CAST(const nsJSObjWrapperKey *, key);

The name |ke| confuses me -- how about |owk| or something longer for
obj-wrapper-key?

Looks good generally, r+sr=me with nits picked.

/be

Comment 3

[Johnny Stenback (:jst)]

Fixed on the trunk.

Comment 4

[Johnny Stenback (:jst)]

Fixed on aviary branch now too.