942356 - Flash reported as out of date (linux Incorrect pluginreg.dat)

Status: VERIFIED FIXED

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

Description

[NoOp]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20100101 Firefox/25.0 SeaMonkey/2.22 (Beta/Release)
Build ID: 20131028185648

Steps to reproduce:

Web site(s) are reporting that I do not have the latest Flash installed. I do have
the most current linux version installed:

SeaMonkey 2.2.0/1:
Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20100101 Firefox/25.0
SeaMonkey/2.22
Shockwave Flash
    File: libflashplayer.so
    Path: /usr/lib/adobe-flashplugin/libflashplayer.so
    Version: 11,2,202,327
    State: Enabled
    Shockwave Flash 11.2 r202

Firefox 25.0.1:
Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20100101 Firefox/25.0
Shockwave Flash
    File: libflashplayer.so
    Path: /usr/lib/adobe-flashplugin/libflashplayer.so
    Version: 11,2,202,327
    State: Enabled
    Shockwave Flash 11.2 r202

The issue occurs in both SeaMonkey 2.2.0 (and 2.2.1) and Firefox 25.1.
However it does not occur in Chromium Version 30.0.1599.114.

The interesting bit is that Chromium is using the exact same
libflashplayer.so as the Mozilla browsers:

Chromium:
Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko)
Ubuntu Chromium/30.0.1599.114 Chrome/30.0.1599.114 Safari/537.36
Adobe Flash Player - Version: 11.2 r202
Shockwave Flash 11.2 r202
Name:	Shockwave Flash
Version:	11.2 r202
Location:	/usr/lib/adobe-flashplugin/libflashplayer.so

If I use <https://www.mozilla.org/en-US/plugincheck/> as a base for
testing, both SeaMonkey and Firefox (both are Mozilla versions & not
distro versions) I get:
Shockwave
FlashShockwave Flash 11.2 r202	vulnerable 'Update Now'

However if I use the same test with Chromium I get:
Shockwave Flash
Shockwave Flash 11.2 r202 	'Up to Date'

Comment 1

[NoOp]

Found the issue: both Firefox and Seamonkey put incorrect plugin information in the pluginreg.dat file. Not that the version information in both files incorrectly places commas, instead of periods, between the version number:
SeaMonkey 2.2.0/1:
Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20100101 Firefox/25.0
SeaMonkey/2.22
Shockwave Flash
    File: libflashplayer.so
    Path: /usr/lib/adobe-flashplugin/libflashplayer.so
    Version: 11,2,202,327

Firefox 25.0.1:
Mozilla/5.0 (X11; Linux x86_64; rv:25.0) Gecko/20100101 Firefox/25.0
Shockwave Flash
    File: libflashplayer.so
    Path: /usr/lib/adobe-flashplugin/libflashplayer.so
    Version: 11,2,202,327

If I modify the pluginreg.dat file to correctly show the version number, the mozilla plugin site recognizes that the Flash version is up-to-date:

[PLUGINS]
libflashplayer.so:$
/usr/lib/adobe-flashplugin/libflashplayer.so:$
11,2,202,327:$

Now I modify:

[PLUGINS]
libflashplayer.so:$
/usr/lib/adobe-flashplugin/libflashplayer.so:$
11.2.202.327:$

and show:
Shockwave Flash
    File: libflashplayer.so
    Path: /usr/lib/adobe-flashplugin/libflashplayer.so
    Version: 11.2.202.327

and the plugin check works correctly.

Comment 2

[Barry Edwin GIlmour]

I too, can confirm that the Bug is still showing commas in my trunk nightly local build (SM2-25/Gecko-28).

Mozilla/5.0 (X11; Linux x86_64; rv:28.0) Gecko/20100101 Firefox/28.0 SeaMonkey/2.25a1 ID:20131120105045 CSet: ffe30ba8cd1b (a local build on SUSE GNU/Linux 3.7.10-1.16 x86_64 gcc version 4.7.2 20130108)

Quoting from my profile's pluginreg.dat file:-

libflashplayer.so:$
/usr/lib64/browser-plugins/libflashplayer.so:$
11,2,202,297:$
1371592303000:0:0:$
Shockwave Flash 11.2 r202:$
Shockwave Flash:$
2
0:application/x-shockwave-flash:Shockwave Flash:swf:$
1:application/futuresplash:FutureSplash Player:spl:$

End quote..

<https://www.mozilla.org/en-US/plugincheck/> says my status is "vulnerable" and my action should "Update now",

The same delimiter-error is in SeaMonkey's internal "Add-on Manager", and the "about:plugins" page.
------------
Manually editing the file now says my status is "11.2.202.297" and my action is "Up to Date".
"Add-on Manager", and the "about:plugins" page, now correctly show the period delimiters.
------------
pluginreg-dat carries the Alert: "Generated File. Do not edit.".
But from-where, and when, is it generated?
Is it at the first SeaMonkey installation, or is it generated at every update?
I'll know more at my next successful build.

Comment 3

[Georg Fritzsche [:gfritzsche]]

We get the version string directly from the plugin [1] and don't change it anywhere (at least not in plugin code).
I assume this is the plugin building a locale-dependent string and Chromium fixes it.

(In reply to Barry Edwin GIlmour from comment #2)
> But from-where, and when, is it generated?
> Is it at the first SeaMonkey installation, or is it generated at every
> update?

This is re-generated whenever any of the info in there changes (e.g. plugins are updated or new ones are installed).

[1] http://hg.mozilla.org/mozilla-central/annotate/98aa428a392c/dom/plugins/base/nsPluginsDirUnix.cpp#l333

Comment 4

[Georg Fritzsche [:gfritzsche]]

We should also be able to fix this (by substituting ',' for '.') around [1].
To test it, we could move sPluginVersion to nptest_name.cpp & secondplugin/nptest_name.cpp and make it "1,0,0,0" for the second test plugin. Then a mochitest could verify that navigator.plugins["Second Test Plug-in"] has the right version string.

[1] http://hg.mozilla.org/mozilla-central/annotate/98aa428a392c/dom/plugins/base/nsPluginTags.cpp#l78
[2] http://hg.mozilla.org/mozilla-central/annotate/98aa428a392c/dom/plugins/test/testplugin/nptest.cpp#l64

Comment 5

[»Q«]

This may resolve (or be a dupe of) bug 801329

Comment 6

[Dongie Agnir [:dagnir]]

Hi George, I'm interested in working on this bug.  I will get a patch out in the next day or two.  I may need some guidance in writing the test, as I've never written one before.

Comment 7

[Georg Fritzsche [:gfritzsche]]

(In reply to »Q« from comment #5)
> This may resolve (or be a dupe of) bug 801329

Not a duplicate - Plugincheck is one of the affected sites and could work around it, but we can't rely on every page doing this.

Comment 8

[Georg Fritzsche [:gfritzsche]]

(In reply to Dongie Agnir [:dagnir] from comment #6)
> Hi George, I'm interested in working on this bug.  I will get a patch out in
> the next day or two.  I may need some guidance in writing the test, as I've
> never written one before.

Great! Feel free to contact me directly in IRC or via mail if needed.

For the test, this will be a (plain) Mochitest [1]. The test will live in dom/plugins/test/mochitest/, you can find mochitest-plain examples in there (the test_XXX.html files).

[1] https://developer.mozilla.org/en-US/docs/Mochitest

Comment 9

[Benjamin Smedberg]

We could also just ask Adobe to fix their reported version number: it's likely that they are using some locale-specific formatting function. If we do add a workaround in our client, we should target it as specifically as possible to Adobe Flash.

Comment 10

[Dongie Agnir [:dagnir]]

Attachment: v1 - replace commas with dots in version string

Comment 11

[Dongie Agnir [:dagnir]]

Attachment: test

Comment 12

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8338668 [details] [diff] [review]
v1 - replace commas with dots in version string

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

::: dom/plugins/base/nsPluginTags.cpp
@@ +251,5 @@
>    return NS_OK;
>  #endif
>  }
>  
> +void nsPluginTag::FormatVersion() {

Per comment 9, we should limit this to Flash (and also Linux, as this is not a problem on other platforms).
mIsFlashPlugin is already set at this point, so lets use that.

@@ +253,5 @@
>  }
>  
> +void nsPluginTag::FormatVersion() {
> +  nsCString::char_iterator cur = mVersion.BeginWriting();
> +  nsCString::char_iterator end = mVersion.EndWriting();

nsCString::ReplaceChar() would avoid manually iterating the string here.

::: dom/plugins/base/nsPluginTags.h
@@ +97,5 @@
>                  const char* const* aMimeDescriptions,
>                  const char* const* aExtensions,
>                  uint32_t aVariantCount);
>    nsresult EnsureMembersAreUTF8();
> +  void FormatVersion();

Maybe the name could describe what it's doing better, like say FixupVersion()?

::: dom/plugins/test/mochitest/mochitest.ini
@@ +33,5 @@
>  [test_bug863792.html]
>  [test_cookies.html]
>  [test_defaultValue.html]
>  [test_enumerate.html]
> +[test_formatPluginVersion.html]

I don't think this is supposed to be in this patch :)
Same goes for the changes to nptest.cpp & nptest_name.cpp.

Comment 13

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8338671 [details] [diff] [review]
test

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

The test changes actually look good, thanks for figuring this out :)
However, going with comment 9 and limiting the fixup to Flash means that we can't add a test for this at the moment.

Comment 14

[Dongie Agnir [:dagnir]]

Attachment: v2 - fix up plugin version string for Flash on Linux

Comment 15

[Dongie Agnir [:dagnir]]

Thanks for the feedback Georg!  I've updated the patch with your suggestions.

Comment 16

[Dongie Agnir [:dagnir]]

Attachment: v3 - fix up plugin version string for Flash on Linux

Comment 17

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8339045 [details] [diff] [review]
v3 - fix up plugin version string for Flash on Linux

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

Thanks, this looks good to me apart from the below nit.
Passing it on to Benjamin for review - we can always revert if/when Flash should be fixed.

::: dom/plugins/base/nsPluginTags.cpp
@@ +80,5 @@
>             aPluginInfo->fMimeDescriptionArray,
>             aPluginInfo->fExtensionArray,
>             aPluginInfo->fVariantCount);
>    EnsureMembersAreUTF8();
> +#if defined(XP_LINUX)

Maybe it's enough to #ifdef in FixupVersion() only? This should get optimized away and litters the code less.

@@ +256,5 @@
>  #endif
>  }
>  
> +#if defined(XP_LINUX)
> +void nsPluginTag::FixupVersion() {

Nit: Opening braces for functions should be on a new line.

Comment 18

[Dongie Agnir [:dagnir]]

Attachment: v4 - fix up plugin version string for Flash on Linux

Comment 19

[Dongie Agnir [:dagnir]]

Thanks Georg for the feedback and Benjamin for the reviews.  Going to add checkin-needed to this.

Comment 20

[Ryan VanderMeulen [:RyanVM]]

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

Comment 21

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/e61b29079c02

Comment 22

[Bill Gianopoulos [:WG9s]]

Comment on attachment 8342016 [details] [diff] [review]
v4 - fix up plugin version string for Flash on Linux

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 
User impact if declined: 
Testing completed (on m-c, etc.): 
Risk to taking this patch (and alternatives if risky): 
String or IDL/UUID changes made by this patch:

Comment 23

[Bill Gianopoulos [:WG9s]]

(In reply to Bill Gianopoulos [:WG9s] from comment #22)
> Comment on attachment 8342016 [details] [diff] [review]
> v4 - fix up plugin version string for Flash on Linux

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Actually caused by a feature but regression form so long ago I can;t identify it, but it did used to work.
> User impact if declined: Linux users will continue to distrust the results of the plugincheck system.
> Testing completed (on m-c, etc.): This has been on central for a few weeks and is also on Aurora
> Risk to taking this patch (and alternatives if risky): This is a very minimal low-risk patch.
> String or IDL/UUID changes made by this patch: none

Comment 24

[Bill Gianopoulos [:WG9s]]

I took a different approach and nominated the already implemented, tested and verified fix for bug 931469 for uplifting to beta so it will land sooner.

Comment 25

[Georg Fritzsche [:gfritzsche]]

(In reply to Bill Gianopoulos [:WG9s] from comment #23)
> [Approval Request Comment]
[...]
> > User impact if declined: Linux users will continue to distrust the results of the plugincheck system.

This also potentially affects other websites that check for the Flash version.

Comment 26

[Bill Gianopoulos [:WG9s]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #25)
> (In reply to Bill Gianopoulos [:WG9s] from comment #23)
> > [Approval Request Comment]
> [...]
> > > User impact if declined: Linux users will continue to distrust the results of the plugincheck system.
> 
> This also potentially affects other websites that check for the Flash
> version.

Not really.  I would expect other websites to correctly use the comma delimiter in version numbers that the flash plugin actually uses.

Comment 27

[bhavana bajaj [:bajaj]]

(In reply to Bill Gianopoulos [:WG9s] from comment #24)
> I took a different approach and nominated the already implemented, tested
> and verified fix for bug 931469 for uplifting to beta so it will land sooner.

NI on Bill to confirm if the attachment on this bug is still needed on Fx 27 ?

Comment 28

[Benjamin Smedberg]

Yes, if we want plugincheck to work. Bill is confusing this bug with his personal wishlist, as far as I can tell. The Linux Flash plugin did not report any version at all until very recently, and it only uses commas in some specific locales, so it is unlikely that any website will be prepared to deal with commas.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5f40681b58fe

Comment 30

[Bogdan Maris, Desktop Test Engineering]

Verified as fixed on latest Aurora 28.0a2 (buildID: 20140109004001) using Ubuntu 12.04 x32 and 13.04 x64.

Comment 31

[Bogdan Maris, Desktop Test Engineering]

Verified fixed on Firefox 27 beta 5 using Ubuntu 12.04 x32 and 13.04 x64.

Comment 32

[NoOp]

Sorry folks, but this bug is NOT fixed:

Mozilla/5.0 (X11; Linux x86_64; rv:26.0) Gecko/20100101 Firefox/26.0 SeaMonkey/2.23

https://www.mozilla.org/en-US/plugincheck/
Shockwave FlashShockwave Flash 11.2 r202	vulnerable	Update Now

about:plugins
Shockwave Flash

    File: libflashplayer.so
    Path: /usr/lib/adobe-flashplugin/libflashplayer.so
    Version: 11,2,202,335
    State: Enabled
    Shockwave Flash 11.2 r202

adobe-flashplugin:
  Installed: 11.2.202.335-0precise1
  Candidate: 11.2.202.335-0precise1
  Version table:
 *** 11.2.202.335-0precise1 0
        500 http://archive.canonical.com/ubuntu/ precise/partner amd64 Packages
        100 /var/lib/dpkg/status

http://www.adobe.com/software/flash/about/
You have version 11,2,202,335 installed

Comment 33

[NoOp]

$ strings /usr/lib/adobe-flashplugin/libflashplayer.so | grep 202
FlashPlayer_11_2_202_335_FlashPlayer
,iso-2022-jp
,csiso2022jp
1iso-2022-kr
1csiso2022kr
LNX 11,2,202,335
Shockwave Flash 11.2 r202
iso-2022-jp
iso-2022-kr
csiso2022jp
csiso2022kr
11.2.202.335

Comment 34

[Georg Fritzsche [:gfritzsche]]

This is fixed in Firefox 27, you are using 26.
You can try the fix e.g. in Firefox Beta:
http://www.mozilla.org/en-US/firefox/channel/#beta

Comment 35

[NoOp]

This bug was/is filed against SeaMonkey:
<https://bugzilla.mozilla.org/show_activity.cgi?id=942356>
Is there a fix for the relevent software?

Comment 36

[Georg Fritzsche [:gfritzsche]]

From my understanding, Seamonkey 2.24 will be based on the Firefox 27 code, so it will be fixed there.
I don't see a SM 2.24 Beta build, but you can try SM Aurora or Nightly.

Comment 37

[Steeve McCauley]

Seeing this again in Firefox 49 on Fedora 24, where about:plugins is reporting that flash is out of date (11.2.202.632) even though it has been updated to 11.2.202.635 (STATE_VULNERABLE_UPDATE_AVAILABLE).

$ rpm -qf /usr/lib64/flash-plugin/libflashplayer.so
flash-plugin-11.2.202.635-release.x86_64
$ strings /usr/lib64/flash-plugin/libflashplayer.so | grep 635
FlashPlayer_11_2_202_635_FlashPlayer
LNX 11,2,202,635
11.2.202.635
drm/%s/%s/%s/11.2.202.635%s

$ grep 11.2.202.635 pluginreg.dat
$ grep 11.2.202.632 pluginreg.dat
11.2.202.632:$

Comment 38

[Steeve McCauley]

After deleting and restarting firefox, about:plugins shows the correct flash version.

Comment 39

[Steeve McCauley]

Happened again on Fedora 25 with Shockwave Flash 24.0 r0, but about plugins was reporting 11.2.202.644 (the version contained in pluginreg.dat).  Deleting the pluginreg.dat file fixed the problem.