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)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: pie.fed, Assigned: serhunt)

References

()

Details

Attachments

(1 file, 4 obsolete files)

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.
confirming with build 2001110103 on Win2k and WM 7.1.
Status: UNCONFIRMED → NEW
Ever confirmed: true
->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.
will chek this asap today..am having some problem with my NT..
OK, I see it now. The problematic file is npwmsdrm.dll.
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
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.
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.
Attached patch not fully working patch (obsolete) (deleted) — Splinter Review
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+
Attached image screen shot of how it looks with the patch applied (obsolete) (deleted) —
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).
*** Bug 110011 has been marked as a duplicate of this bug. ***
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.
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
This patch works with no visible problems so far.
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 *|?
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.
Formal r=/sr= please?
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.
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?
The other three calls are mimetype, mimetype description and mimetype extension. I can see description might need it but not the other two.
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
Dan, yeah. Thats what I get for fastreading...av it is up to you what you wanna do here.
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 on attachment 58818 [details] [diff] [review] patch -- now including mimedescriptions to SetBytesUTF8 business sr=dveditz, thanks for making these changes
Attachment #58818 - Flags: superreview+
Attachment #58818 - Flags: review+
*** Bug 112046 has been marked as a duplicate of this bug. ***
*** Bug 112050 has been marked as a duplicate of this bug. ***
I checked this in to the trunk. dp, I guess we should also get it into the branch, should we? What branches will benefit?
I dont think plugin caching is on the 0.9.4 branch. So this wont matter for that branch.
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
*** Bug 112187 has been marked as a duplicate of this bug. ***
*** Bug 112102 has been marked as a duplicate of this bug. ***
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
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.
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?
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
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).
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.
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.
*** Bug 112778 has been marked as a duplicate of this bug. ***
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.
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)
*** Bug 113382 has been marked as a duplicate of this bug. ***
Status: RESOLVED → VERIFIED
.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: