Closed
Bug 108246
Opened 23 years ago
Closed 23 years ago
A special character in plugin info causes caching mechanism fail
Categories
(Core Graveyard :: Plug-ins, defect)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: pie.fed, Assigned: serhunt)
References
()
Details
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
dp
:
review+
dveditz
:
superreview+
|
Details | Diff | Splinter Review |
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:0.9.5+)
Gecko/20011101
BuildID: 2001110103
I copied plugin files from the Windows Media Player into Mozilla's plugins
directory, and about:plugins fails to work entirely.
Reproducible: Always
Steps to Reproduce:
1. I did this with Windows Media Player 7.01.00.3055
2. Attempt to enable WMP plugin in Mozilla by copying <WMP>\np* into Mozilla's
plugins diretory.
3. go to about:plugins
Actual Results: The page displayed says only:
Installed plug-ins
Find more information about browser plug-ins at Netscape.com.
----- <horizontal line> -----
The JavaScript Console reports:
Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIDOMPlugin.name]" nsresult: "0x80004005
(NS_ERROR_FAILURE)" location: "JS frame ::
jar:resource:///chrome/toolkit.jar!/content/global/plugins.html :: <TOP_LEVEL>
:: line 95" data: no]
Expected Results: I would expect that this exception would be handled, and the
rest of the plugins would be displayed. In fact, it'd be nice if a little
warning block appeared on the about:plugins page saying such-and-such plugin is
invalid.
The offending plugin file is npwmsdrm.dll
When I take it out of the plugins directory, everything is wonderful again.
Comment 1•23 years ago
|
||
confirming with build 2001110103 on Win2k and WM 7.1.
Comment 2•23 years ago
|
||
->plug-ins? punt as needed.
Assignee: pchen → av
Component: XP Apps → Plug-ins
QA Contact: sairuh → shrir
This works for me with yesterday's build on NT40.
Shrirang?
Files I copied are: npdrmv2.dll, npdrmv2.zip, npds.zip, npdsplay.dll.
Comment 4•23 years ago
|
||
will chek this asap today..am having some problem with my NT..
dp, I think this is yours. I need your advice. When we get the plugin tag info
and cache it in the registry your code dose something like
registry->SetStringUTF8(pluginKey, kPluginsNameKey, tag->mName);
The problem here is that this specific plugin name contains a special character
(the trademark -- (R)), and the above line of code fails and writes null in the
registry, I beleive. Then when JavaScripts asks for a name of the plugin it gets
an error and fails to execute.
I can see differents ways to fix this, mostly just bulletproofing which maybe is
a good idea anyway, but the real problem is that the plugin info is _not_ cached
in such a case, and I beleive we should fix that.
Status: NEW → ASSIGNED
Summary: a bad dll causes about:plugins to fail completely → A special character in plugin info causes caching mechanism fail
Comment 7•23 years ago
|
||
Yup. You are right. It is unfortunate that these names can have non-UTF8
characters in them. Which character code is this ?
PluginName and all other cached fields that come from the plugin could use
nsIRegistry::SetBytesUTF8()
instead of SetStringUTF8(). Dan, would that work ?
The other option is to escape/unescape them while writing/reading from registry.
Dan, what is the best approach here.
Comment 8•23 years ago
|
||
The "UTF8" registry APIs expect UTF8, validate the UTF8, and return a specific
error indicating not-UTF8.
If you had Unicode you could use the SetString() API, but then you have to worry
about what charset the plugin is in in order to convert correctly. You can, as
dp suggests, use SetBytes or SetBytesUTF8 (which in this case merely refers to
the character coding of the name arg) to preserve the random bytes passed in,
but that merely moves the problem since at some point to display this string
you're still going to have to figure out what charset it's in and convert to
Unicode.
I did not use string stuff much so far. This patch attempts to address the
presence of spec chars in plugin name and description. And it sort of work.
dp and dveditz, please advise on how to make it better.
Attachment #57869 -
Flags: needs-work+
Assignee | ||
Comment 10•23 years ago
|
||
Please note an extra character just before the trademark symbol. There should
be nothing, not even a space char. When I look at it in the debugger, the '(R)'
symbol value is -82 (negative eighty two).
Comment 11•23 years ago
|
||
*** Bug 110011 has been marked as a duplicate of this bug. ***
Comment 12•23 years ago
|
||
The default three lines of context is pretty useless for people unfamiliar with
code except for the most trivial patches; please use more context for reviews.
The deprecated "WithConversion" string methods or explicit conversions from or
to ASCII are a big red flag -- do you really have ASCII text? (by ascii we mean
the strict 7-bit charset, not the various 8-bit "ascii-plus" national sets
people sometimes call ascii, especially here in the US.) If you did then the
UTF8 registry methods wouldn't have given you any problems since ascii is a
subset of UTF8. Most likely you have Latin-1 data, which means it'll be
something completely different on Asian systems and the ConvertASCIItoUCS2()
functions will *really* mess you up.
Who calls nsPluginsHostImpl::RegisterPlugin()? The only calls I could find were
under module/plugin/samples and in the oji/MRJ directories, neither of which
seemed right. Who calls this will help determine which charset we should
use/expect. Or instead of the ASCII->Unicode conversion just store the text as
bytes and let the conversion errors be dealt with somewhere else.
Chopped off your patch in the RegisterPlugin routine was a for-loop that also
uses SetStringUTF8 for the mime types, descriptions and extensions. The
descriptions in particular might be translated into a non-ascii charset so I
think the UTF8 routines here are problematic.
Assignee | ||
Comment 13•23 years ago
|
||
dveditz, thanks for your thoughts. RegisterPlugin is supposed to be called by
xpcom plugins which are now deprecated. MRJ plugin uses it on Mac. A couple of
commersial products are on the market already, e.g. RealPlayer.
I'll try SetBytes approach and see how it works.
Attachment #57869 -
Attachment is obsolete: true
Attachment #57869 -
Flags: needs-work+
Attachment #57871 -
Attachment is obsolete: true
Assignee | ||
Comment 14•23 years ago
|
||
This patch works with no visible problems so far.
Assignee | ||
Comment 15•23 years ago
|
||
Assignee | ||
Comment 16•23 years ago
|
||
If this way to store string as bytes seems to be acceptable, should I
explicitly terminate the array with null after |GetBytesUTF8(...)| before I cast
it to |const char *|?
Comment 17•23 years ago
|
||
Since you are writing strlen()+1 I dont think you need to try null terminate it.
It should happen automatically. Patch looks good to me.
Assignee | ||
Comment 18•23 years ago
|
||
Formal r=/sr= please?
Comment 19•23 years ago
|
||
You don't need to manually copy in more context, just pass the argument to the
cvs diff command so the result is a valid patch that can be applied and tested.
Thanks for the context though -- much appreciated! -- I'm just noting that the
cvs diff argument is easier and results in a patch that can be used as the main
review attachment so you don't have to make two attachments.
You did not address my concern about the remaining three SetStringUTF8 calls in
nsPluginHostImpl::RegisterPlugin. I'll grant that most likely the MIME type and
extension are 7-bit ascii (maybe even required by the spec?), but I don't trust
the description. You're not going to bail on error, that's good, but what
happens later if there's an uninitialized description?
Other than that though the rest looks good and I'll happily give sr= if you
reassure me about that one point.
Assignee | ||
Comment 20•23 years ago
|
||
I know about cvs diff -uN thing, just thought that including the whole C
function is better. Peter L. told me that there is such cvs option on Mac, but I
am not aware of the Win analogue.
I will gladly change the the remaining three cases along the same line if dp
does not object. dp, do you?
Comment 21•23 years ago
|
||
The other three calls are mimetype, mimetype description and mimetype extension.
I can see description might need it but not the other two.
Comment 22•23 years ago
|
||
dp: isn't that what I said?
I have more concerns than I did about MIME type and extension, though. It looks
like those values are passed in from the plugin itself, and even if legal values
only contain 7-bit ascii (and no-one has asserted that, it's an optimistic guess
on my part) there's no guarantee the calling program won't screw up. These reg
entries won't get created and this routine has no problem with that, but what
happens later when these values aren't found?
av: including the whole function is much appreciated. You can usually jigger the
context value if you have to make sure its included. -u20 would have gotten
nearly all of it (-u30 if you wanted overkill) and has the advantage of 1) being
easier to produce for you than hand editing and 2) results in a valid patch that
can be reviewed and applied
Comment 23•23 years ago
|
||
Dan, yeah. Thats what I get for fastreading...av it is up to you what you wanna
do here.
Assignee | ||
Comment 24•23 years ago
|
||
Please review/superreview. I guess we are fine expecting lower ASCII chars only
for mimetypes and file extensions. If this whenever happens we will fail to
launch a plugin as the worst scenario. Evangelism issue, if any. I'll talk
about it on the next group gathering, perhaps the best way is just to mention
this fact in the plugin documentation.
Attachment #58184 -
Attachment is obsolete: true
Attachment #58185 -
Attachment is obsolete: true
Comment 25•23 years ago
|
||
Comment on attachment 58818 [details] [diff] [review]
patch -- now including mimedescriptions to SetBytesUTF8 business
sr=dveditz, thanks for making these changes
Attachment #58818 -
Flags: superreview+
Updated•23 years ago
|
Attachment #58818 -
Flags: review+
Comment 26•23 years ago
|
||
*** Bug 112046 has been marked as a duplicate of this bug. ***
Comment 27•23 years ago
|
||
*** Bug 112050 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 28•23 years ago
|
||
I checked this in to the trunk. dp, I guess we should also get it into the
branch, should we? What branches will benefit?
Comment 29•23 years ago
|
||
I dont think plugin caching is on the 0.9.4 branch. So this wont matter for that
branch.
Comment 30•23 years ago
|
||
I have a similar problem. Before this patch, about:plugins didn't work at all
(at least recently before this patch). Now it works, but stops before
displaying info on all my plugins. I also have a script that is similar to
about:plugins. It does a loop through the plugins and pipes the output to a new
window. The script does nothing right now because it fails before it can get to
the point where it pipes the data to the new window. Is this going to be fixed
in full soon?
Here is the error in my script:
Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIDOMPlugin.description]" nsresult: "0x80004005
(NS_ERROR_FAILURE)" location: "JS frame ::
http://www.visi.com/~hoju/assets/scripts/browserinfo.js ::
getExtendedBrowserInfo :: line 88" data: no]
To reproduce this, visit:
http://www.visi.com/~hoju/indextest.html
Then, click on "browser info extended browser info (NS only)" on the left side
of the screen. You will see nothing happen. Now check your javascript console
to see the error above.
jake
Comment 31•23 years ago
|
||
*** Bug 112187 has been marked as a duplicate of this bug. ***
Comment 32•23 years ago
|
||
*** Bug 112102 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 33•23 years ago
|
||
Marking fixed. Please file a separate bug for other issues mentioned if needed.
For verification purposes please use npwmsdrm.dll on Windows or plugger on
Linux.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Comment 34•23 years ago
|
||
Sorry, still doesn`t work for me with 20011127 and 20011128 trunk builds, you
should verify the code has actually been checked in or reopen.
Assignee | ||
Comment 35•23 years ago
|
||
Martin, what exactly do you see? The best way to see the original problem (which
is I beleive fixed) is to place npwmsdrm.dll in Mozilla plugins folder (not
Netscape4's) and on about:plugins page see how it looks. Can you please try
this?
Comment 36•23 years ago
|
||
I'm using build 2001112809 on Win2k (SP2).
I just removed all plugins except for npnull.dll from my Netscape4 plugins
directory and and have been testing various plugins on their own in Mozilla's
plugins directory. I haven't found one plugin yet that *doesn't* cause that
error to be thrown. These include:
npwmsdrm.dll (Windows Media Player)
nppl3260.dll (Real Player)
npswf32.dll (flash)
np32dsw.dll (shockwave)
NPAXDLPI.dll (not sure what plugin)
All the Java plugins files
etc...
This build must have the fix that was checked into the trunk. Did it do more
harm than good?
Jake
Comment 37•23 years ago
|
||
See my description in the Bug 110011 I filed (duplicate) for a description of my
problem and the list of plugins I have (I have npwmsdrm.dll and I still see the
same problem as before).
Comment 38•23 years ago
|
||
I don't have netscape 4.x installed too, must I add. Netscape 6.x and Internet
explorer are not on my system too. The only browser I have is mozilla.
Comment 39•23 years ago
|
||
The problem has become worse under Solaris/x86 2.8 using a home-made build
from the latest source. (The source tarball had a 28-Nov-2001 09:52
timestamp, so this is probably 20011128).
about:plugins now simply reports
Installed plug-ins
Find more information about browser plug-ins at Netscape.com.
and the following error appears in the JavaScript console:
Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIDOMPlugin.name]" nsresult: "0x80004005
(NS_ERROR_FAILURE)" location: "JS frame ::
jar:resource:///chrome/toolkit.jar!/content/global/plugins.html :: <TOP_LEVEL>
:: line 95" data: no]
This is with just libnullplugin.so and plugger.so in the plugins
directory.
Comment 40•23 years ago
|
||
*** Bug 112778 has been marked as a duplicate of this bug. ***
Comment 41•23 years ago
|
||
I built Mozilla for Solaris/x86 again from the 29 Nov sources, and this
time about:plugin works perfectly. Either I did something wrong yesterday,
or the bug was finally squashed during the last 24 hours.
Reporter | ||
Comment 42•23 years ago
|
||
Well, the *was* fixed, but I just got Build 2001120303, and now I get the
original symptoms, though in more situations.
Error: uncaught exception: [Exception... "Component returned failure code:
0x80004005 (NS_ERROR_FAILURE) [nsIDOMPlugin.name]" nsresult: "0x80004005
(NS_ERROR_FAILURE)" location: "JS frame ::
jar:resource:///chrome/toolkit.jar!/content/global/plugins.html :: <TOP_LEVEL>
:: line 95" data: no]
So I put only npnul32.dll in the plugins dir, and I get the SAME EXCEPTION, but
at leas a line does display...
So I added the NPJava11.dll, only the default plugin is displayed, the exception
is thrown.
NPJava12.dll, only the default plugin is displayed, the exception is thrown.
NPJava131.dll, only the default plugin is displayed, the exception is thrown.
NPJava32.dll, only the default plugin is displayed, the exception is thrown.
any other plugin dll, now no plugins are displayed, and the exception is thrown.
Zac (original reporter)
Assignee | ||
Comment 43•23 years ago
|
||
*** Bug 113382 has been marked as a duplicate of this bug. ***
Updated•22 years ago
|
Status: RESOLVED → VERIFIED
Comment 44•22 years ago
|
||
.
Updated•2 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•