Closed Bug 942356 Opened 6 years ago Closed 6 years ago

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

Categories

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

25 Branch
x86_64
Linux
defect

Tracking

()

VERIFIED FIXED
mozilla28
Tracking Status
firefox26 --- wontfix
firefox27 + verified
firefox28 --- verified

People

(Reporter: glgxg, Assigned: dagnir)

References

Details

(Whiteboard: [mentor=gfritzsche][lang=c++][good next bug])

Attachments

(1 file, 4 obsolete files)

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'
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.
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.
Component: General → Plug-ins
Product: SeaMonkey → Core
Version: SeaMonkey 2.22 Branch → 25 Branch
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
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
Priority: -- → P3
Whiteboard: [mentor=gfritzsche][lang=c++][good next bug]
Status: UNCONFIRMED → NEW
Ever confirmed: true
This may resolve (or be a dupe of) bug 801329
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.
(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.
(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
Assignee: nobody → dongie.agnir
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.
Attachment #8338668 - Flags: feedback?(georg.fritzsche)
Attached patch test (obsolete) — Splinter Review
Attachment #8338671 - Flags: feedback?(georg.fritzsche)
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.
Attachment #8338668 - Flags: feedback?(georg.fritzsche)
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.
Attachment #8338671 - Flags: feedback?(georg.fritzsche)
Attachment #8338668 - Attachment is obsolete: true
Attachment #8338671 - Attachment is obsolete: true
Attachment #8338769 - Flags: feedback?(georg.fritzsche)
Thanks for the feedback Georg!  I've updated the patch with your suggestions.
Attachment #8338769 - Attachment is obsolete: true
Attachment #8338769 - Flags: feedback?(georg.fritzsche)
Attachment #8339045 - Flags: feedback?(georg.fritzsche)
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.
Attachment #8339045 - Flags: review?(benjamin)
Attachment #8339045 - Flags: feedback?(georg.fritzsche)
Attachment #8339045 - Flags: feedback+
Attachment #8339045 - Flags: review?(benjamin) → review+
Attachment #8339045 - Attachment is obsolete: true
Attachment #8342016 - Flags: review?(benjamin)
Attachment #8342016 - Flags: review?(benjamin) → review+
Thanks Georg for the feedback and Benjamin for the reviews.  Going to add checkin-needed to this.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/e61b29079c02
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
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:
Attachment #8342016 - Flags: approval-mozilla-beta?
(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
Depends on: 931469
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.
No longer depends on: 931469
Blocks: 801329
(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.
(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.
(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 ?
Flags: needinfo?(wgianopoulos)
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.
Flags: needinfo?(wgianopoulos)
Keywords: verifyme
Attachment #8342016 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified as fixed on latest Aurora 28.0a2 (buildID: 20140109004001) using Ubuntu 12.04 x32 and 13.04 x64.
Verified fixed on Firefox 27 beta 5 using Ubuntu 12.04 x32 and 13.04 x64.
Status: RESOLVED → VERIFIED
Keywords: verifyme
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
$ 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
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
This bug was/is filed against SeaMonkey:
<https://bugzilla.mozilla.org/show_activity.cgi?id=942356>
Is there a fix for the relevent software?
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.
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:$
After deleting and restarting firefox, about:plugins shows the correct flash version.
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.
You need to log in before you can comment on or make changes to this bug.