166395 - Need ability to override navigator.appName and navigator.appVersion from prefs.js

Status: RESOLVED FIXED

(Core :: Networking: HTTP, enhancement)

Description

[Ian Neal]

Certain sites sniff for which browser is which by looking at their appName and
appVersion and deny access. For Evangelism purposes it would be useful to
overide these variables so that mozilla can get into these sites to test what
else works or doesn't work.

I have looked around on bugzilla and mozilla to see if these prefs already exist
but couldn't find anything. I've tried the obvious ones of
general.useragent.appName, general.useragent.appName.override, general.appName
and general.appName.override but none of them work.

Comment 1

[Olav Vitters]

appVersion is bug 115773

Comment 2

[Brian Nesse (gone)]

Not preferences... over to samir for further triage.

Comment 3

[Ian Neal]

bug 115773 wants to have the appVersion change when the useragent string changes
which is one possible solution but the appName really needs to change too to be
effective. It would mean you'd only need the useragent.override preference as
long as their was a reasonably simple way of mapping the UA string to appName
and appVersion.

Comment 4

[Henry James]

I think the MozDev project UABar just does this job. Also the MultiZilla addon
has a Quick Pref menu with a submenu "HTTP User Agent settings" to fake all
kinds of user agents.

http://uabar.mozdev.org/
http://multizilla.mozdev.org/

Comment 5

[Ian Neal]

The whole point of this enhancement request is that changing the UA string,
which I frequently do for Evaneglism purposes, does not change navigator.appName
and navigator.appVersion.

I'm looking for either a pref that does change these two variables or for these
two variables to be changed when the UA string is changed.

Comment 6

[HJ]

I really wonder how anybody can ask for a preference setting when the code isn't
even implemented. However, the HTTP UserAgent code should change this when the
HTTP UserAgent override is set. 

I don't think we need a new preference setting for this. The user should simply
get what he asked for when he set/changes the HTTP UserAgent. That is just
common sense to me.

Comment 7

[Darin Fisher]

ian: isn't general.useragent.override sufficient?

Comment 8

[Ian Neal]

I suppose I should have been more explict with my summary and said prefs and
backend code (probably why I logged it against prefs backend to start with
though I expect that was the wrong component). 
Tweeking summary, original summary was "Need prefs for overiding
navigator.appName and navigator.appVersion"

general.useragent.override would be fine as long as there was the code to change
what is reported by navigator.appName and navigator.appVersion to match what you
would expect from a browser reporting that UA string.
There would need to be someway of converting UA strings to appName and
appVersion, which I expect would have to be different for IE, Mozilla, Netscape
(various versions), Opera, etc.

Comment 9

[Darin Fisher]

ian: ok, ic what your getting at.  currently, general.useragent.override only
changes what we send on network connections.  it does not effect what our DOM
reports.  perhaps it should.  cc'ing some folks for comment.

Comment 10

[Boris Zbarsky [:bzbarsky]]

timeless, didn't you have some thoughts on this?

Comment 11

[timeless]

yeah, my project to refactor useragent so that each consumer could have its own
forgery would fix this.

Comment 12

[Darin Fisher]

timeless: then you might be interested to know that nsIHttpProtocolHandler is
probably going to be frozen come mozilla 1.2 (see bug 157133).

Comment 13

[HARUNAGA Hirotoshi]

http://www.fujifilm.co.jp/efa/body.html uses scripts as follows.

if (navigator.appName.indexOf("Netscape") >= 0) {
	if (navigator.appVersion.charAt(0) >= 5) {
		var aver = parseFloat(navigator.vendorSub);
		if (navigator.vendorSub.length > 4) {
			aver += parseFloat(navigator.vendorSub.substring(4,5))/100;
		}
	} else {
		var ep = navigator.appVersion.indexOf(" ");
		var aver = parseFloat(navigator.appVersion.substring(0,ep));
	}
	return (nnv != 0 && nnv <= aver);
}

What about navigator.vendorSub?

Comment 14

[John Vance]

Konqueror 3.0 already does this.  Spoofing a different browser changes the user
agent string as well as the navigator object.  It even allows you to spoof
different browsers on a site by site basis.  This would be really useful for
getting around bug 89853.  Capital One checks to see if navigator.appVersion
starts with 4, and if it doesn't you're denied access to online credit card
services.

Comment 15

[Darin Fisher]

-> future

Comment 16

[Bill Sellers]

Darin:

I am requesting that this bug get escalated.  The company that I work for has
implemented checks for appName and appVersion in cold fusion code.  They are
doing this to "enforce" that users use the corporate approved version of
Netscape or IE.  It probably isn't prudent to say what I really think of this
policy, so I won't. 
;)

Comment 17

[noririty]

"navigator.appName" seems defined at here.
http://lxr.mozilla.org/mozilla/source/dom/src/base/nsGlobalWindow.cpp#5881

Comment 18

[Greg Singer]

Attachment: Patch to allow setting navigator.appName and appVersion via about:config

Should apply against latest CVS, NOT 0.7.

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

firstly, that patch lacks the changes to modules/libpref/src/init/all.js
secondly, why do you bother with putting this on nsHttpHandler? that seems
totally not worth the effort.

Comment 20

[Greg Singer]

Ick.  Bluff called.

This was mainly done by copying, pasting, and modifying the code for setting the
user agent, so that's why stuff got shoveled into nsHttpHandler.  I figured
someone must have done something right with that and it would be easier for me
to build off what they'd done rather than trying to figure out the correct way
to do this.  >_<  Since I also seem to be unable to use Bugzilla correctly, I
might as well add that navigator.appName() is set by general.appname, which is
defined in browser/app/profile/all.js (remember, I copied useragent stuff), and
navigator.appVersion() can be overridden by general.appversion.override.

I'm feeling very silly at this point.  Sorry if this is causing anyone any
trouble, since I seem to be spamming twenty or so people every time I post
something to this thread :/.

Comment 21

[Christian :Biesinger (don't email me, ping me on IRC)]

>This was mainly done by copying, pasting, and modifying the code for setting the
>user agent

yeah, the useragent is necko business really, and hence belongs in
nsHttpHandler. navigator.appName however is only of interest to the DOM.

(btw, I think you patch addresses only half of this bug - only appName not
appVersion)

>browser/app/profile/all.js

that's the firebird file...

Comment 22

[Greg Singer]

Attachment: Here's a hopefully less lousy version that seems to work without barfing on more files than it needs to. Patch with -p1.

Note:  both of these patches do cover navigator.appVersion.

Comment 23

[Ian Neal]

Couple of things:
1) Can you not do appname the same way as appversion then you would not need to
alter all.js at all and keeps the behaviour consistent with how setting UA
string works. 
2) Whitespace changes - as far as I can see they all break formatting rather
than fixing any.

Comment 24

[Greg Singer]

Attachment: By popular demand, navigator.appname now set by general.appname.override

all.js is now safe :).	The whitespace was an artifact of something I'd tried
earlier and wasn't intentional, so I fixed it.	Thanks for pointing it out.

Comment 25

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 136130 [details] [diff] [review]
By popular demand, navigator.appname now set by general.appname.override

+  prefBranch->PrefHasUserValue("general.appname.override", &override);

why?

+    if (NS_FAILED(res))
+      return res;

why should this be fatal?

Comment 26

[Greg Singer]

Attachment: It seems my persistence is rivalled by my incompetence.

Comment 27

[Ben Giddings]

An instance where this would really be helpful:

http://personal.fidelity.com/products/funds/mfl_frame.shtml?130347883

var browser = "";

if ( (parseInt(navigator.appVersion) >= 4) &&
	((navigator.appName.indexOf("Microsoft") != -1) ||
(navigator.appName.indexOf("Netscape") != -1)) &&
	((navigator.appVersion.indexOf("Win") != -1) ||
(navigator.appVersion.indexOf("Mac") != -1)) ) {
	browser = "okay";
}
else {
	browser = "other";
}

They don't even try the page when it's Mozilla on Linux, only browsers calling
themselves "Netscape" or "IE" on "Win" or "Mac" are tried.  I threw a filter in
Privoxy to overcome this, after calling their tech support dept. and
complaining.  I still would prefer to be able to set this from Mozilla though.


-----

And even worse:
alert('You are using ' + navigator.appName + ' ' + browser.versionMinor)

where "browser.versionMinor" is reported by "Browser Detect Lite  v2.1.3" as 1.5
and navigator.appName is reported as Netscape.

This makes them think I'm using Netscape 1.5 -- no big shock they don't think
the browser will work.

Comment 28

[Doug Halamay]

Any possibility of getting this patch reviewed/checked in soon?

Trying to sign in to online banking at www.bankofamerica.com is broken because
it checks navigator.appName, which we can't spoof/change until this is fixed.

Comment 29

[Boris Zbarsky [:bzbarsky]]

I assume there is an evangelism bug on that site?

Comment 30

[Hadrien Nilsson]

Btw, I think appName should now return « Mozilla » instead of « Netscape » by
default. Mozilla do not depend on Netscape anymore. It's strange to use Firebird
and get a « Netscape » string when accessing to navigator.appName.

Comment 31

[Matthias Versen [:Matti]]

comment#30 :
That is another bug and it's wontfix for good reasons

Comment 32

[Doug Halamay]

RE: Comment#29 :

I filed bug 230612 regarding the problem.

Comment 33

[David P James]

(In reply to comment #31)
> comment#30 :
> That is another bug and it's wontfix for good reasons

Yes, bug 61071.

But what are the good reasons, especially in 2004? I've probably encountered
*more* problems with sites thinking that appName = Netscape == Netscape4 than
with some other name in the string (the site will serve up NS4 stuff that
Mozilla can't handle whilst Opera and Konqueror will happily display the site
since they use own names and not "Netscape", although Opera does default to IE).

Once this bug is fixed, the default appName should become "Mozilla" or "Mozilla
Firefox" (as the case may be) since a UA spoofing extension could change it back
to Netscape for those sites which may cause a problem.

Comment 34

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 136139 [details] [diff] [review]
It seems my persistence is rivalled by my incompetence.

IMO, this should only be able to be changed by embeddors, not users.  As should
the user agent.  But then again, in an ideal world, this would not matter. 
This whole thing stinks to me, but I'll offer up a review anyway.

>--- dom/src/base/nsGlobalWindow.cpp	2003-11-22 19:03:20.883448800 -0500
>+++ dom/src/base/nsGlobalWindow.cpp.new	2003-11-22 19:03:28.033361848 -0500
>@@ -5758,6 +5758,24 @@
> NavigatorImpl::GetAppVersion(nsAString& aAppVersion)
> {
>   nsresult res;

With your changes, you aren't using this...

>+
>+  nsCOMPtr<nsIPrefBranch> prefBranch(gPrefBranch);
>+  if (!prefBranch) {
>+    prefBranch = do_GetService(NS_PREFSERVICE_CONTRACTID);
>+    NS_ENSURE_STATE(prefBranch);
>+  }
>+
>+  nsXPIDLCString override;
>+

...until here, so declare and initialize all at once.

But while you are here, please rename the nsresult to |rv|.

>+  res = prefBranch->GetCharPref("general.appversion.override",
>+                                getter_Copies(override));
>+
>+  if (NS_SUCCEEDED(res))
>+  {

We use K&R bracing style, not BSD style.

>+    CopyASCIItoUCS2(override, aAppVersion);
>+    return res;
>+  }
>+
>   nsCOMPtr<nsIHttpProtocolHandler>
>     service(do_GetService(kHTTPHandlerCID, &res));
>   if (NS_SUCCEEDED(res) && service) {


The same exact comments apply to your GetAppName changes.

r=caillon I suppose, but only if jst superreviews.

Comment 35

[Greg Singer]

Attachment: Updated patch

Bracing fixed, res changed to rv.  Whatever else that was using gPrefBranch
appears to have gone away in CVS over the past few months, so instead of
bringing it back, I'm just setting prefBranch with do_GetService and rv's
getting declared early anyway.

Comment 36

[Peter Van der Beken [:peterv]]

You want to use GetCharPref or GetStringPref on nsContentUtils.

Comment 37

[Greg Singer]

Attachment: Updated updated patch

Yes, it appears I do.  Simplifies things somewhat.

Comment 38

[Owen Marshall]

(This is possibly beyond the scope of the current bug in name, but not in
spirit, I think, so ...)

To really spoof sites perfectly (such as HSBC's internet banking -
www.ebank.hsbc.com.au - see bug #205631) you need to allow an override to the
navigator.platform string as well.  Adding  

  nsXPIDLCString override =
  nsContentUtils::GetCharPref("general.platform.override");

  if (override) {
    CopyASCIItoUTF16(override, aPlatform);
    return NS_OK;
  }

to NavigatorImpl::GetPlatform(nsAString& aPlatform) solves this, and allows the
HSBC site to work perfectly (and presumably many others that rely on
navigator.platform to check whether the user is running Windows or Mac OS, with
no option for any other OS).  Any chance this could be added in as well?

Comment 39

[Johnny Stenback (:jst)]

Comment on attachment 148434 [details] [diff] [review]
Updated updated patch

Drive-by review comment...

+  nsXPIDLCString override =
+    nsContentUtils::GetCharPref("general.appversion.override");

override should be a const nsAdoptingString& here, the current code will do an
extra malloc+string copy for no reason. Same thing in other places that uses
methods that return nsAdoptingString (except in odd cases that I don't see in
this patch).

   nsCOMPtr<nsIHttpProtocolHandler>
-    service(do_GetService(kHTTPHandlerCID, &res));
-  if (NS_SUCCEEDED(res) && service) {
+    service(do_GetService(kHTTPHandlerCID, &rv));
+  if (NS_SUCCEEDED(rv) && service) {

redundant error checking, either check NS_SUCCEEDED(rv) or that service is
non-null, checking both is a waste of code. Since you need rv here anyways,
check it, and don't bother checking that service is non-null if do_GetService()
succeeded.

+    rv = service->GetAppVersion(str);
+    CopyASCIItoUTF16(str, aAppVersion);

     aAppVersion.Append(NS_LITERAL_STRING(" ("));
-    res = service->GetPlatform(str);
-    if (NS_FAILED(res))
-      return res;
+    rv = service->GetPlatform(str);

rv at the top of this block is overwritten by the last line here, either check
it and return it at the top of this, or just don't assign into it at all...

     aAppVersion.Append(PRUnichar(')'));

Unnecessary cast.

Comment 40

[Greg Singer]

Attachment: Misc. fixes; also covers platform and appCodeName

Update fixes string type and checks rv before reassigning it, as well as
changing the whitespace in GetAppVersion to be a little more logical (lumping
pieces that get appended to aAppVersion together instead of grouping by where
the error checks went).  I also made it possible to override navigator.platform
(as requested) and navigator.appCodeName.  I'm not completely sure those should
be user-configurable, but I guess some sites do check them.  Hopefully, this'll
be useful :/.


>      aAppVersion.Append(PRUnichar(')'));
>
> Unnecessary cast.

If the ')' were assumed to be UTF-16, then it wouldn't be necessary, but on my
system at least, GCC sees

    aAppVersion.Append(')');

as appending an ASCII character to a UTF-16 string.  In addition to not really
being correct, it also doesn't compile (GCC 3.4).  Maybe I'm just
misunderstanding you, but the cast seems necessary to me.

Comment 41

[Greg Singer]

Attachment: Superfluous appCodeName override pulled.

Silly me, navigator.appCodeName is already configurable via
general.useragent.appName... platform appears to be hard-coded.  The override
has been removed, but I left changing |res| to |rv| in GetAppCodeName.	Would
it be worth it to similarly clean up the other functions (it's pretty much
uniformly |res| in this part of the file), or is it better to just leave that
stuff alone?

Comment 42

[Michael van der Kolff]

Was just thinking, can this be put in the next release of mozilla?  It would be
really useful, and I think that the Status Whiteboard is saying that Opera has
already got it; Konqueror definitely does.  How do we go about getting it in the
next beta?

Comment 43

[Steve Chapel]

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

Comment 44

[Greg Singer]

Attachment: Update to apply cleanly against recent SeaMonkey CVS

The code above the patched area appears to have grown by about 80 lines or so;
patch updated for convenience.

Comment 45

[Darin Fisher]

Comment on attachment 153170 [details] [diff] [review]
Update to apply cleanly against recent SeaMonkey CVS

asking jst to review this instead of me.  it's his module! ;-)

Comment 46

[Johnny Stenback (:jst)]

Comment on attachment 153170 [details] [diff] [review]
Update to apply cleanly against recent SeaMonkey CVS

   nsCOMPtr<nsIHttpProtocolHandler>
-    service(do_GetService(kHTTPHandlerCID, &res));
-  if (NS_SUCCEEDED(res) && service) {
+    service(do_GetService(kHTTPHandlerCID, &rv));
+  if (NS_SUCCEEDED(rv) && service) {

I'd suggest replacing all occurances of the above if check with a check that
only tests the service pointer, no need to test both...

r+sr=jst. Let me know if you need help checking this in.

Comment 47

[amano]

It would be nice if this fix could land on the Fx aviary branch as well. So a
completely functional User Agent Switch Extension - possibly developed by Chris
Pederick - would work with Fx 1.0.

Setting the blocking 1.0 flag to get the attention from the main firefox
developers. 

Comment 48

[Johnny Stenback (:jst)]

Fix landed on the trunk (with some other minor related cleanup). Anyone wanna
port this to the aviary branch? I'll gladly check it in, but I'd need the patch
pretty soon to have time to get it in.

Marking bug FIXED.

Comment 49

[Greg Singer]

Attachment: Aviary port

Anything objectionable about this for the aviary branch?

Comment 50

[Johnny Stenback (:jst)]

Not IMO. Landed on the aviary branch.

Comment 51

[Asa Dotzler [:asa]]

Comment on attachment 154016 [details] [diff] [review]
Aviary port

a=asa for 1.7.5.