Looking through nsPluginHostImpl.cpp, I noticed that in UserAgent() we are using a hard-coded agent string ["Mozilla/5.0 [en] (Windows;I)"] instead of getting the right one for the correct version/platform/localization. It looks like a simple fix because the code is already in place. In fact, the #DEFINE is commented out at the top and it looks like Gagan was working on this at one time.

I may have then, but won't now :) ->av

I think this is something which needs to get fixed for RTM as we will report the wrong UserAgent in the plugin API. Adding that keyword amd cc:ing Eric. http://lxr.mozilla.org/seamonkey/source/modules/plugin/nglsrc/nsPluginHostImpl.c pp#1568

Andrei will investigate. We need to know (1) where this incorrect UA string is being reported to (the plug-in? the server, directly from the browser? the plug-in, and then passed on to the server later?) and what the risks are. It could be very dangerous for our browser to be reporting varying UA strings from time to time. Need to leave on RTM radar until we have severity determined. cc:ing valeski in case Andrei needs advice about how to get the UA string correctly.

oh mama! I don't know the plugin code, but I'm guessing that for all 4.x style plugins that want to get our user agent, we will hand them "Mozilla/5.0 [en] (Windows;I)", regardless of platform (that string is _hardcoded_). Someone who knows plugins better than I do needs to verify what plugins will be affected by this. It's possible that all new style plugins will suffer this fate. I've attatched a patch that retrieves the correct UA string. However, the hardcoded string was static and I generate new memory for the returned stream. The caller of this function (http://lxr.mozilla.org/seamonkey/source/modules/plugin/nglsrc/ns4xPlugin.cpp#1070) is not expecting new memory. Subsequently, as is, my patch will cause a leak of the string. ns4xPlugin should keep track of this memory and delete it when it's done. IMO, this is very serious, and depending on which plugins are affected by it, I'd stop ship for it.

Nick, Liz, Sean, Patrcik -- Severity assessment on this issue please? Any idea how this might affect your plug-ins, if at all?

incorrect platform and localization in userAgent string will not affect Beatnik.

Eric, this string is only reported to the plugin when it asks for it via the Plugin API. I don't think that plugin is interested in platform when it queries for UA, the browser version is probably more important. The way it is now, we will tell plugins that we are Mozilla not 4.x and this should be enough for old-school plugins for now. This can be important later when we fix bugs or say change API (which is unlikely) and a plugin could want to distinguish between different versions of Mozilla. Localization is another issue which could be important, but here we have to ask plugin makers.

Judson, to your patch. Why not to allocate a static string of say 128 byte size, copy UA string there, return it to the caller and then free the memory?

Andrei, please get review and super-review ASAP for this patch and then mark it rtm+ for consideration as a patch for limbo acceptance. Thanks! PDT: The presence of a hardcoded user-agent string within the Plug-in API (that doesn't match the actual user agent string) was completely overlooked. This inconsistency *might* trigger problems that we haven't anticipated. The fix is extremely low risk (it simply copies and returns the correct UA string) so let's please take this along with other limbo fixes if we respin to reduce the risk of unforeseen problems for our plug-in partners in upgrading and cross-browser/cross-point-release support.

r=valeski after the following two items are addressed: 1. rename "retString" to something that differs more from "retstring" 2. we should zero out the "retString" before copying in the new string so we don't wind up w/ overlapping memory.

I talked to Judson and we agreed that #2 in his comment is not actually needed.

r=valeski on the 17:04 patch

Honestly, at this point I think fixing the bug is much more risky than not fixing it. Here's why. You're trading *one week* of QA for the last "n" months where we've been delivering this bogus UA string to plugins. I would have thought that any problems that would've surfaced would have been reported by now, right? If we *change* the UA string we're delivering, isn't it possible that some plugins that were merrily expecting the bogo-string could start to freak out? Anyway, the code changes themeselves look fine, so sr=waterson: this should *definitely* go onto the trunk. However, I'd think long and hard before putting this on the branch.

rtm-, not seeing the benefit being worth any risk at this point.

Checked in into the trunk. It will not go to the branch. Marking fixed.

To verify the fix please place the attached plugin in the Plugins folder, restart and load the attached html source.

*** Bug 61454 has been marked as a duplicate of this bug. ***