Be more thorough checking for NULL NPP values

webkit/plugins/npapi/plugin_host.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "webkit/plugins/npapi/plugin_host.h"
 
 #include "app/gfx/gl/gl_context.h"
 #include "app/gfx/gl/gl_implementation.h"
 #include "base/file_util.h"
 #include "base/logging.h"
@@ skipping 675 matching lines @@
   //              plug-in drawing occurs, returns HWND
   //    NPNVjavascriptEnabledBool:  tells whether Javascript is enabled
   //    NPNVasdEnabledBool:  tells whether SmartUpdate is enabled
   //    NPNVOfflineBool: tells whether offline-mode is enabled
 
   NPError rv = NPERR_GENERIC_ERROR;
 
   switch (static_cast<int>(variable)) {
     case NPNVWindowNPObject: {
       scoped_refptr<PluginInstance> plugin(FindInstance(id));
+      if (!plugin.get()) {

[ananta, 2011/03/23 18:39:26]

Should we move this code outside the switch block to reduce the clutter?

[davidben, 2011/03/23 18:51:08]

Some variables don't seem to require a plugin instance. For instance,
NPNVjavascriptEnabledBool and NPNVSupportsWindowless. I don't know if
"officially" you're allowed to query them with a null instance, or if plugins
actually do, but we and Mozilla both accept it, so it's conceivable that someone
does.

(Hence why I'm not very thrilled about doing the check on nspluginwrapper's end,
if it does in fact depend on the particular variable.)

+        NOTREACHED();
+        return NPERR_INVALID_INSTANCE_ERROR;
+      }
       NPObject *np_object = plugin->webplugin()->GetWindowScriptNPObject();
       // Return value is expected to be retained, as
       // described here:
       // <http://www.mozilla.org/projects/plugins/npruntime.html#browseraccess>
       if (np_object) {
         WebBindings::retainObject(np_object);
         void **v = (void **)value;
         *v = np_object;
         rv = NPERR_NO_ERROR;
       } else {
         NOTREACHED();
       }
       break;
     }
     case NPNVPluginElementNPObject: {
       scoped_refptr<PluginInstance> plugin(FindInstance(id));
+      if (!plugin.get()) {
+        NOTREACHED();
+        return NPERR_INVALID_INSTANCE_ERROR;
+      }
       NPObject *np_object = plugin->webplugin()->GetPluginElement();
       // Return value is expected to be retained, as
       // described here:
       // <http://www.mozilla.org/projects/plugins/npruntime.html#browseraccess>
       if (np_object) {
         WebBindings::retainObject(np_object);
         void** v = static_cast<void**>(value);
         *v = np_object;
         rv = NPERR_NO_ERROR;
       } else {
         NOTREACHED();
       }
       break;
     }
   #if !defined(OS_MACOSX)  // OS X doesn't have windowed plugins.
     case NPNVnetscapeWindow: {
       scoped_refptr<PluginInstance> plugin = FindInstance(id);
       if (!plugin.get()) {
         NOTREACHED();
-        return NPERR_GENERIC_ERROR;
+        return NPERR_INVALID_INSTANCE_ERROR;
       }
       gfx::PluginWindowHandle handle = plugin->window_handle();
       *((void**)value) = (void*)handle;
       rv = NPERR_NO_ERROR;
       break;
     }
   #endif
     case NPNVjavascriptEnabledBool: {
       // yes, JS is enabled.
       *((void**)value) = (void*)1;
@@ skipping 14 matching lines @@
   #endif
     case NPNVSupportsWindowless: {
       NPBool* supports_windowless = reinterpret_cast<NPBool*>(value);
       *supports_windowless = true;
       rv = NPERR_NO_ERROR;
       break;
     }
     case NPNVprivateModeBool: {
       NPBool* private_mode = reinterpret_cast<NPBool*>(value);
       scoped_refptr<PluginInstance> plugin(FindInstance(id));
+      if (!plugin.get()) {
+        NOTREACHED();
+        return NPERR_INVALID_INSTANCE_ERROR;
+      }
       *private_mode = plugin->webplugin()->IsOffTheRecord();
       rv = NPERR_NO_ERROR;
       break;
     }
     case webkit::npapi::default_plugin::kMissingPluginStatusStart +
          webkit::npapi::default_plugin::MISSING_PLUGIN_AVAILABLE:
     // fall through
     case webkit::npapi::default_plugin::kMissingPluginStatusStart +
          webkit::npapi::default_plugin::MISSING_PLUGIN_USER_STARTED_DOWNLOAD: {
       // This is a hack for the default plugin to send notification to
       // renderer.  Even though we check if the plugin is the default plugin,
       // we still need to worry about future standard change that may conflict
       // with the variable definition, in order to avoid duplicate case clauses
       // in this big switch statement.
       scoped_refptr<PluginInstance> plugin(FindInstance(id));
+      if (!plugin.get()) {
+        NOTREACHED();
+        return NPERR_INVALID_INSTANCE_ERROR;
+      }
       if (plugin->plugin_lib()->plugin_info().path.value() ==
             webkit::npapi::kDefaultPluginLibraryName) {
         plugin->webplugin()->OnMissingPluginStatus(variable -
             webkit::npapi::default_plugin::kMissingPluginStatusStart);
       }
       break;
     }
   #if defined(OS_MACOSX)
     case NPNVpluginDrawingModel: {
       // return the drawing model that was negotiated when we initialized.
       scoped_refptr<PluginInstance> plugin(FindInstance(id));
+      if (!plugin.get()) {
+        NOTREACHED();
+        return NPERR_INVALID_INSTANCE_ERROR;
+      }
       *reinterpret_cast<int*>(value) = plugin->drawing_model();
       rv = NPERR_NO_ERROR;
       break;
     }
 #ifndef NP_NO_QUICKDRAW
     case NPNVsupportsQuickDrawBool: {
       // We do not admit to supporting the QuickDraw drawing model. The logic
       // here is that our QuickDraw plugin support is so rudimentary that we
       // only want to use it as a fallback to keep plugins from crashing: if a
       // plugin knows enough to ask, we want them to use CoreGraphics.
@@ skipping 60 matching lines @@
       DVLOG(1) << "NPN_GetValue(" << variable << ") is not implemented yet.";
       break;
   }
   return rv;
 }
 
 NPError NPN_SetValue(NPP id, NPPVariable variable, void* value) {
   // Allows the plugin to set various modes
 
   scoped_refptr<PluginInstance> plugin(FindInstance(id));
+  if (!plugin.get()) {
+    NOTREACHED();
+    return NPERR_INVALID_INSTANCE_ERROR;
+  }
   switch(variable) {
     case NPPVpluginWindowBool: {
       // Sets windowless mode for display of the plugin
       // Note: the documentation at
       // http://developer.mozilla.org/en/docs/NPN_SetValue is wrong.  When
       // value is NULL, the mode is set to true.  This is the same way Mozilla
       // works.
       plugin->set_windowless(value == 0);
       return NPERR_NO_ERROR;
     }
@@ skipping 250 matching lines @@
 }
 
 void NPN_URLRedirectResponse(NPP instance, void* notify_data, NPBool allow) {
   scoped_refptr<PluginInstance> plugin(FindInstance(instance));
   if (plugin.get()) {
     plugin->URLRedirectResponse(!!allow, notify_data);
   }
 }
 
 }  // extern "C"