638171 - FireFox does not read Java Plugin Info.plist correctly.

Status: RESOLVED FIXED

(Core Graveyard :: Plug-ins, defect)

Description

[Bino George]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_6_6; en-US) AppleWebKit/534.13 (KHTML, like Gecko) Chrome/9.0.597.107 Safari/534.13
Build Identifier: 4.0b11

The Java Plugin2 has an Info.plist that specifies all the mimetypes supported by the plugin. However, only part of this list is read and some parts are ommitted. I found that the the presence of any versions that have three or more digits causes the rest of the plist to not be read correctly :


If you remove all of the x.x.x entries such as :

   <key>application/x-java-applet;version=1.1.1</key>
    <dict>
      <key>WebPluginTypeDescription</key>
      <string>Java applet</string>
    </dict>
    <key>application/x-java-applet;version=1.1.2</key>
    <dict>
      <key>WebPluginTypeDescription</key>
      <string>Java applet</string>
    </dict>
    <key>application/x-java-applet;version=1.1.3</key>
    <dict>
      <key>WebPluginTypeDescription</key>
      <string>Java applet</string>
    </dict>

It works fine and then reads the rest of the list.

Basically, about:plugins should reflect all the versions specified in the Info.plist.

Reproducible: Always

Steps to Reproduce:
type about:plugins in the URL field.

You will see that the list is not the complete list that is specified in the Info.plist :

/System/Library/Java/Support/Deploy.bundle/Contents/Resources/JavaPlugin2_NPAPI.plugin/Contents/Info.plist



Expected Results:  
about:plugins should have the full list of versions from the WebPluginMIMETypes dict of the plist here :

/System/Library/Java/Support/Deploy.bundle/Contents/Resources/JavaPlugin2_NPAPI.plugin/Contents/Info.plist

Comment 1

[Ed Morley [:emorley]]

Does this occur in the latest Fx 4 beta?

Is this broken in 3.6.x as well, or is it an Fx4 regression?

Comment 2

[Bino George]

It is a Fx4 regression, it works fine in FireFox 3 according to my testing.

Comment 3

[Ed Morley [:emorley]]

Would you be able to try and find the regression range:
http://harthur.github.com/mozregression/

Comment 4

[Ed Morley [:emorley]]

Also, before doing the above, can you verify it still exists in the latest nightly (to save unnecessary effort on your part!):
http://nightly.mozilla.org/

Comment 5

[u279076]

Bino, can you please try to reproduce this with Firefox 4.0rc1 or the latest Minefield nightly? If so, please try to find a regression range using the tool in comment 3.  If you need help with any of this please let us know.

Comment 7

[Benjamin Smedberg]

(gdb) p *info.fMimeTypeArray@17
$13 = {0x1207e72f0 "application/x-java-applet;version=1.1.3", 0x1207e7370 "application/x-java-applet", 0x1207e73c0 "application/x-java-applet;jpi-version=1.6.0_22", 0x1207e7450 "application/x-java-applet;version=1.2.2", 0x1207e7500 "application/x-java-applet;version=1.5", 0x1207e75a0 "application/x-java-applet;version=1.3.1", 0x1207e7650 "application/x-java-applet;version=1.3", 0x1207e76f0 "application/x-java-vm", 0x1207e7760 "application/x-java-applet;version=1.1.2", 0x1207e7810 "application/x-java-applet;version=1.1", 0x1207e78b0 "application/x-java-vm-npruntime", 0x1207e7940 "application/x-java-applet;version=1.2.1", 0x1207e79f0 "application/x-java-applet;version=1.6", 0x1207e7a90 "application/x-java-applet;version=1.4.2", 0x1207e7b40 "application/x-java-applet;version=1.4", 0x1207e7be0 "application/x-java-applet;version=1.1.1", 0x1207e7c90 "application/x-java-applet;version=1.2"}

We cut off the list after we see application/x-java-vm-npruntime intentionally:
http://hg.mozilla.org/mozilla-central/annotate/4512b571e48e/modules/plugin/base/src/nsPluginTags.cpp#l138

But on mac at least, PList dictionaries are not order-sensitive, so it's a bit random where that particular entry would be in the list.

jst added this short-circuit in bug 393928
http://hg.mozilla.org/mozilla-central/rev/bca65549e6b8#l17.128

jst, do you remember why you added that check in the first place, and whether I can just remove it, either for mac or for all platforms?

Nominating for blocking based on the dup bug 640483 which has us breaking java in the wild, and this being a regression from 3.6 because we switched from JEP to JavaPlugin2.

Comment 8

[Benjamin Smedberg]

Attachment: Don't ignore following MIME types, rev. 1

Comment 9

[Johnny Stenback (:jst)]

Comment on attachment 518853 [details] [diff] [review]
Don't ignore following MIME types, rev. 1

+  if (javaSentinelVariant != -1) {
+    if (mVariants > javaSentinelVariant + 1) {
+      memmove(mMimeTypeArray + javaSentinelVariant + 1,
+              mMimeTypeArray + javaSentinelVariant,
+              (mVariants - javaSentinelVariant - 1) * sizeof(mMimeTypeArray[0]));
+
+      if (mExtensionsArray) {
+        memmove(mExtensionsArray + javaSentinelVariant,
+                mExtensionsArray + javaSentinelVariant + 1,
+                (mVariants - javaSentinelVariant - 1) * sizeof(mExtensionsArray[0]));
+      }
+    }
+
+    mMimeTypeArray[mVariants] = NULL;
+    if (mExtensionsArray)
+      mExtensionsArray[mVariants] = NULL;
+
+    mMimeDescriptionArray.RemoveElementAt(javaSentinelVariant);
+    mVariants--;
+  }

This leaks the strings held in mMimeTypeArray[javaSentinelVariant] and 
mExtensionsArray[javaSentinelVariant]. I wonder if this wouldn't be cleaner to have a javaSentinelOffset that starts out as 0 and gets set to -1 in the case where we see the sentinel mimetype, and then add javaSentinelOffset to i in the loop above (and we'd need to 'continue' when we hit the sentinel mimetype as well). That way we wouldn't need to do any memmove's etc.

r- until the leak is fixed, one way or another.

Comment 10

[Benjamin Smedberg]

Attachment: Don't ignore following MIME types, rev. 1.1

I just did the freeing here, and factored this into a helper function.

On trunk it would be great to make these use nsTArray, but that's a bit too much for the immediate term.

Comment 11

[Josh Aas]

Attachment: rev 1.2, w/pluginreg refresh

Looks good to me, this should force a pluginreg refresh but somebody should confirm that it works.

Comment 12

[Bino George]

This bug is the same as 9064651 in Apple bugreporter (https://bugreport.apple.com/cgi-bin/WebObjects/RadarWeb.woa/wa/openProblemDetail?problemID=9064651)  : rdar://problem/9064651

Comment 13

[Benjamin Smedberg]

relnote: "Firefox on Mac OS X running in 64-bit mode does not recognize that Java supports the MIME type 'application/x-java-applet;version=1.6', which may cause some sites, including skandiabanken.no to stop working or ask you to upgrade Java, even though it is already up to date. Workaround: run Firefox in 32-bit mode (link to SUMO article)." I can't actually find the SUMO link, although I've been told there is an article explaning it somewhere.

Comment 14

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/2e161bb743d7

Comment 15

[Daniel Veditz [:dveditz]]

Is it safe to fix this regression on the mozilla2.0 branch for Fx4.0.1? provisionally blocking due to "fc4-ridealong" notation.

Comment 16

[Josh Aas]

I think it's safe enough.

Comment 17

[Daniel Veditz [:dveditz]]

Comment on attachment 519738 [details] [diff] [review]
rev 1.2, w/pluginreg refresh

Approved for the mozilla2.0 repository, a=dveditz for release-drivers

Comment 18

[(no longer active)]

http://hg.mozilla.org/releases/mozilla-2.0/rev/7124cc6d660d