If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

LoadPlugins should handle failures more gracefully

VERIFIED FIXED in M17

Status

()

Core
Plug-ins
P3
normal
VERIFIED FIXED
18 years ago
17 years ago

People

(Reporter: sean echevarria, Assigned: av (gone))

Tracking

({testcase})

Trunk
x86
Windows 2000
testcase
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nsbeta3+] [fix in hand])

Attachments

(4 attachments)

(Reporter)

Description

18 years ago
An attempt is made to load a plugin's FileVersionInfo at:
http://lxr.mozilla.org/seamonkey/source/modules/plugin/nglsrc/nsPluginsDirWin.cp
p#287

If a file has no VersionInfo, NS_ERROR_OUT_OF_MEMORY is returned.  In 
http://lxr.mozilla.org/seamonkey/source/modules/plugin/nglsrc/nsPluginHostImpl.c
pp#2376 , nsPluginHostImpl::LoadPlugins() bails out when that gets returned.  

mPluginsLoaded never gets set to PR_TRUE and the plugins that were loaded 
before that do not get removed from the list.  The count of plugins as returned 
by navigator.plugins.length increases everytime the length is queried 
(LoadPlugins is called by GetPluginCount if mPluginsLoaded is FALSE).

LoadPlugins() should handle the situation more gracefully.

Change nsPluginFile::GetPluginInfo() to:
   versionsize = ::GetFileVersionInfoSize((char*)path, &zerome);
+  if (!versionsize)
+      return NS_ERROR_FAILURE;

Change nsPluginHostImpl::LoadPlugins() to:
             // create a tag describing this plugin.
             nsPluginInfo info = { sizeof(info) };
!            nsresult res = pluginFile.GetPluginInfo(info);
+            if (res != NS_OK)
+            {
+                // if not a memory error, just continue with next plugin
+                if (res == NS_ERROR_FAILURE)
+                    continue;
+                return NS_ERROR_FAILURE;
+            }

This change occurs inside two different blocks - the MOZ_LOCAL dir block and 
the 4.x plugins dir block.
(Reporter)

Comment 1

18 years ago
Created attachment 8759 [details] [diff] [review]
proposed fix for nsPluginsHostImpl.cpp
(Reporter)

Comment 2

18 years ago
Created attachment 8760 [details] [diff] [review]
proposed fix for nsPluginsDirWin.cpp
(Reporter)

Updated

18 years ago
Keywords: 4xp, patch
(Reporter)

Comment 3

18 years ago
a testcase would be to create a zero length file with the name npfoo.dll and 
put it in the netscape 4.x plugins dir and then load the testcase that I'll 
upload.
Keywords: testcase
(Reporter)

Comment 4

18 years ago
Created attachment 8761 [details]
testcase html page
(Assignee)

Updated

18 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → M17

Comment 5

18 years ago
*** Bug 42040 has been marked as a duplicate of this bug. ***
(Reporter)

Comment 6

18 years ago
Created attachment 10551 [details] [diff] [review]
new patch - single file change only
(Reporter)

Comment 7

18 years ago
Added an up-to-date patch.  nsPluginsDirWin.cpp does not need to be modified.
(Reporter)

Comment 8

18 years ago
changed the summary - it was really bothering me...
Summary: LoadPlugins should be more graceful → LoadPlugins should handle failures more gracefully
(Reporter)

Comment 9

17 years ago
nominating nsbeta3
Keywords: nsbeta3

Comment 10

17 years ago
This looks easy enough, and the patch looks safe. Andrei, could you please check 
it out and advise if there are risks I do not see? Thanks to sean for the patch!
Whiteboard: [nsbeta3+] [fix in hand]
(Assignee)

Comment 11

17 years ago
Yes, I was just waiting for nsbeta3+ nomination.
(Assignee)

Comment 12

17 years ago
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED

Comment 13

17 years ago
verified that the patch is in.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.