206659 - Plugin not found, because MIME type has upper-case letters

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect, P5)

Description

[Lukasz Jazgar]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; pl-PL; rv:1.3.1) Gecko/20030425
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; pl-PL; rv:1.3.1) Gecko/20030425

If plugin is registered with MIME type, which have uppercase letter, window to
making choice between saving file and running application is shown instead of
launching plugin. 
This happens only if we directly go to resource on server and this resource has
type which should be handled by this plugin.

Reproducible: Always

Steps to Reproduce:
1. Install any plugin with MIME type with upper-case letters inside, e.g.
application/x-Mrowkojad.
2. Refer to resource which has unrecognizable extension for this plugin and
which has MIME type "application/x-Mrowkojad" 
3.

Actual Results:  
Window with information, that type of file is "application/x-mrowkojad"
(lowercase) and proposition of saving file or running application is shown.

Expected Results:  
Plugin should be used.
Comparing Mime types should not be case-sensitive in this case

This happens only if we directly go to resource on server and this resource has
type which should be handled by this plugin.
If resource is loaded by <embed> tag, it works correct. Similarly if resource
has extension handled by this plugin.
I have 2 plugins which make such problems. If you request, I can send you one of
these plugins and jsp page generating data for this plugin.

Comment 1

[Matthias Versen [:Matti]]

see bug 87290

Comment 2

[Peter Lubczynski]

I just tried this with my debug Win32 build on a fresh trunk pull and it WFM.

Here's steps to repro on Linux (mdk9.1) /w Apache:
1. su - 
2. gedit /etc/httpd/apache-mime.types &
3. Change the case of application/x-shockwave-flash
4. Save, then: service httpd restart
5. Put a test.swf to test with in /var/www/html
6. Validate your server sends the content-type header with mixed case by doing a:
wget http://localhost/test.swf

In mozilla, you can with with a -console switch and NSPR_LOG_MODULES=nsHTTP:8 to
see the HTTP headers. Be sure you clear you cache before testing so that you'll
get a HTTP 200 response (instead of the HTTP 304 Not Modified).

Reporter: please validate this bug happens in a daily trunk build. Thanks!

Comment 3

[Boris Zbarsky [:bzbarsky]]

Um.. This bug is about plugins that report a type with an uppercase letter to
Mozilla (see step 1 in comment 0)

Comment 4

[Lukasz Jazgar]

Exactly.
It doesn't matter is server response content-type upper or lower case. 

Comment 5

[Grahame Jordan]

(In reply to comment #2)
This is also a problem on Linux.
To solve the problem on the client:
sudo vi /etc/mime-types
add JPG to mime type.
Restart Thunderbird and attachement opens.

This is silly. What if I have jpG or jPg ...
A simple tolower() in the comparison will fix this?

Comment 6

[Georg Fritzsche [:gfritzsche]]

Per the description this would be relatively easy to fix by converting the mimetype to lowercase when the plugin tag stores it in nsPluginTag::InitMime() [1].

To test this we could just introduce upper-case to the mimetype entries for the second test plugin [2], "application/x-second-test".

[1] http://hg.mozilla.org/mozilla-central/diff/c4f9959cc9ac/dom/plugins/base/nsPluginTags.cpp
[2] http://mxr.mozilla.org/mozilla-central/source/dom/plugins/test/testplugin/secondplugin/

Comment 7

[Srinath N [:srinath]]

Hi gfritzsche,

I'm interested in working on this bug. Are you available for mentoring?

Comment 8

[Georg Fritzsche [:gfritzsche]]

Hi Srinath, that's great.
Yes, i'm available - feel free to contact me directly via mail or IRC if needed.
A good first step here should be to confirm the issue by:
* first confirming that the following test works fine: dom/plugins/test/mochitest/test_secondPlugin.html
* change the mimetype at [1] to mixed-case, say x-Second-Test
* build dom/plugins
* run the test again - it should now fail

[1] http://hg.mozilla.org/mozilla-central/annotate/5a49762ee832/dom/plugins/test/testplugin/secondplugin/nptest_name.cpp#l7

Comment 9

[Georg Fritzsche [:gfritzsche]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #8)
> * change the mimetype at [1] to mixed-case, say x-Second-Test

Also in these 2 locations:
http://mxr.mozilla.org/mozilla-central/source/dom/plugins/test/testplugin/secondplugin/Info.plist#27
http://mxr.mozilla.org/mozilla-central/source/dom/plugins/test/testplugin/secondplugin/nptest.rc#32

Comment 10

[Georg Fritzsche [:gfritzsche]]

Attachment: Test mixed-case mime behavior.

Srinath found that the test-scenario i described actually wasn't the right one, so here is a patch containing a proper test.

After applying the patch, run the test using:
./mach mochitest-plain dom/plugins/test/mochitest/test_mixed_case_mime.html

Comment 11

[Biru [:poiru]]

Attachment: Disregard case of MIME types in nsPluginTag::InitMime

Went ahead and took this since Srinath does not seem to be working on it any longer.

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=fa7f6595e476

Comment 12

[Biru [:poiru]]

Another push to try to test on OS X and Windows: https://tbpl.mozilla.org/?tree=Try&rev=a5204c63c845

Comment 13

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8333959 [details] [diff] [review]
Disregard case of MIME types in nsPluginTag::InitMime

Review of attachment 8333959 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me and try runs are green - thanks!
Passing on review to Benjamin though as i'm no peer (and shouldn't review my own test-code either).

Comment 14

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8333959 [details] [diff] [review]
Disregard case of MIME types in nsPluginTag::InitMime

Review of attachment 8333959 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/plugins/base/nsPluginTags.cpp
@@ +133,5 @@
> +    if (!aMimeTypes[i]) {
> +      continue;
> +    }
> +
> +    nsCString mimeType(aMimeTypes[i]);

Minor note: per [1], this looks like a good use-case for nsAutoCString.

[1] https://developer.mozilla.org/de/docs/Mozilla_internal_string_guide#The_Concrete_Classes_-_which_classes_to_use_when

Comment 15

[Biru [:poiru]]

Attachment: Disregard case of MIME types in nsPluginTag::InitMime

(In reply to Georg Fritzsche [:gfritzsche] from comment #14)
> Minor note: per [1], this looks like a good use-case for nsAutoCString.

Fixed, thank you.

Comment 16

[Benjamin Smedberg]

Why are we taking this patch instead of just asking people to fix their plugins? What real-life plugin does this affect?

Comment 17

[Georg Fritzsche [:gfritzsche]]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #16)
> Why are we taking this patch instead of just asking people to fix their
> plugins? What real-life plugin does this affect?

Without collecting data i'm not sure if there is a live plugin that this affects. However, unless i'm mixing up what applies here, we should be case-insensitive anyway. Identical text is found in e.g. [1] & [2]:
"The type, subtype, and parameter names are not case sensitive. For example, TEXT, Text, and TeXt are all equivalent top-level media types."

[1] http://tools.ietf.org/html/rfc2045#page-13
[2] http://www.w3.org/Protocols/rfc1341/4_Content-Type.html

Comment 18

[Benjamin Smedberg]

Comment on attachment 8334685 [details] [diff] [review]
Disregard case of MIME types in nsPluginTag::InitMime

ok

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ad0d9f62c29c

Comment 20

[Ryan VanderMeulen [:RyanVM]]

Backed out for B2G desktop mochitest orange. Probably just needs to be skipped on B2G, but please test that on Try first.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2e586efc6160

https://tbpl.mozilla.org/php/getParsedLog.php?id=31465057&tree=Mozilla-Inbound

Comment 21

[Georg Fritzsche [:gfritzsche]]

That definitely shouldn't run on any B2G test-suite. I'm confused though - why is this test run and apparently not the other tests in dom/plugins/test/mochitest/mochitest.ini (those would fail too)?

Comment 22

[Georg Fritzsche [:gfritzsche]]

So, turns out that we need to exclude the tests specifically for b2g-desktop around here:
http://hg.mozilla.org/mozilla-central/annotate/118234ab24ed/testing/mochitest/b2g-desktop.json#l594

Let's do that and a full try run, then this should be good to go.

Comment 23

[Biru [:poiru]]

Attachment: Disregard case of MIME types in nsPluginTag::InitMime

(In reply to Georg Fritzsche [:gfritzsche] from comment #22)
> So, turns out that we need to exclude the tests specifically for b2g-desktop
> around here:
> http://hg.mozilla.org/mozilla-central/annotate/118234ab24ed/testing/
> mochitest/b2g-desktop.json#l594
> 
> Let's do that and a full try run, then this should be good to go.

Done: https://tbpl.mozilla.org/?tree=Try&rev=1632ba075c8e

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/11e0720cbd28

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/11e0720cbd28