1572290 - Site specific User Agent correction

Status: RESOLVED FIXED

(SeaMonkey :: General, defect)

Description

[Jay]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:57.0) Gecko/20100101 Firefox/57.0

Steps to reproduce:

1. Enable UA override: general.useragent.site_specific_overrides=T
2. Set a UA override: general.useragent.override.mozilla.org=TEST
3. Visit domain and examine recorded UA for all GETs.

Actual results:

Initial UA override worked correctly.
UA for all other resource GETs were not overridden

Expected results:

All GETs should have User Agent overridden as required.

Comment 1

[Jay]

Attachment: Patch to fix UA override implementation

For subresources there should be no test for UA "if it is already set". That is because the UA is ALWAYS already set, having been set in HttpBaseChannel::Init() via nsHttpHandler::AddStandardRequestHeaders().

However, if the intent was to only override the default then one would not test for IsEmpty() but instead would access the deeply buried HeaderVariety enum in nsHttpHeaderArray and check for value "eVarietyRequestDefault". But the methods do not currently lend themselves to this level of access.

In any case, the meaning of the word "override" is to actually override, and it does not mean "to perhaps override but maybe not depending on something obscure". So even if some javascript changes the UA then that should not inhibit the override from actually overriding.

Note that another change would be to never store the top-level UA in nsIRequestContext for subsequent use with subresources, but instead apply independently to every subresource.

Comment 2

[Frank-Rainer Grahl (:frg)]

Jay, this is a change for Gecko code. Is this intended for SeaMonkey?

Usually we pick up the Gecko changes and also backport. You would need to provide a proper patch for current mozilla-central. If you intent to do this you would need to set up phabricator for patch submission and I need to reassign the bug.

Comment 3

[Jay]

I have little interest in Firefox because it is so totally broken in so many ways. So, yes, this is only for SeaMonkey. Besides, Firefox never add a reference to SiteSpecificUserAgent.manifest in chrome.manifest whereas SeaMonkey does.

Comment 4

[Frank-Rainer Grahl (:frg)]

Comment on attachment 9083875 [details] [diff] [review]
Patch to fix UA override implementation

Oki. I will to check out the patch. It is also not in the correct format but I can fix this.

Comment 5

[Frank-Rainer Grahl (:frg)]

Dimitry not sure if this might solve your issue too.

Comment 6

[Frank-Rainer Grahl (:frg)]

Attachment: 1572290-sm-j.patch

Reformatted patch for 2.53 release branch.

Comment 7

[Dmitry Butskoy (:buc)]

Dimitry not sure if this might solve your issue too.

Seems no.

Patch from bug #1331275 completely reverts the (broken) change and restores the "initial" behaviour -- ie. override UA for EACH request (no matter whether it is initial or not), and for EACH request check for the particular site_specific overrides.

For example, this way we can change UA for fonts.googleapis.com, and by showing some early version we cause it to provide fonts in more generic format, undestandable under ancient RHEL6 system. Note "just one of the GETs inside a request is overriden", even not the initial one (from the bar).

You can read bug #1331275 discussion for more info.
The patch itself looks "scary", but it is just a revert of the "spoiled all the things" git commit. It works fine all 3 years, at least in Fedora (where I maintain SM).

Probably restoring the "initial old days" behaviour is the best way to solve all the possible issues with "SM + site_specific overrides" .

Comment 8

[Dmitry Butskoy (:buc)]

Attachment: firefox-52-mozilla-revert-1148544.patch

Restore old behaviour patch from bug #1331275, against SM 2.49.5 .
Works (for years :) ) in Fedora/RHEL

Comment 9

[Dmitry Butskoy (:buc)]

Attachment: firefox-56-mozilla-revert-1148544.patch

The same reverting patch (bug #1331275), re-edited against FF 56 (tag FIREFOX_RELEASE_56_END).
Simple changes, should work.

Could somebody test it with SM-2.53? Ie. whether it still works as expected, and which of two alternate solutions from here is more useful in practice?

Comment 10

[Frank-Rainer Grahl (:frg)]

I am putting the first one soon into the unoffical build. We can then try the second one later so that we have 2 builds to compare. I am a bit concerned about performance myself here but something needs to be done.

Comment 11

[Frank-Rainer Grahl (:frg)]

Comment on attachment 9085895 [details] [diff] [review]
1572290-sm-j.patch

This version is now in Bills latest 2.53 build: http://www.wg9s.com/comm-253/

Would like some feedback and then try the second one too.

Comment 13

[Dmitry Butskoy (:buc)]

Attachment: seamonkey-2.53.1-mozilla-revert-1148544.patch

Patch against 2.53.1

Comment 14

[Dmitry Butskoy (:buc)]

It seems attachment 9085895 [details] [diff] [review] is not gitlab source? Whether it was tested?

Comment 15

[Frank-Rainer Grahl (:frg)]

Whether it was tested?

It is in the unofficial 2.53.1/2:
http://www.wg9s.com/comm-253/

There was no feedback here so the patch did not go into the official 2.53.1 yet. Should I replace it with yours in the unofficial 2.53.2?

Comment 16

[Frank-Rainer Grahl (:frg)]

Upps forgot to set NI

Should I put your patch into the unofficial 2.53.2 for testing? 2.57 too if you want to see if it works there.

Comment 17

[Dmitry Butskoy (:buc)]

(In reply to Frank-Rainer Grahl (:frg) from comment #16)

Should I put your patch into the unofficial 2.53.2 for testing? 2.57 too if you want to see if it works there.
Sure.

I've just tested that it works as expected -- ie. changes UA for the domain (and subdomains) specified, considering the change for EACH http request in the session (NOT for whole session's requests just by what specified in urlbar). It is the traditiobal FF behavior (before it was spoiled for Android optimisations as I mentioned somewhere above).

BTW, it is a chance that various sites which dislike our UA string can use some common subrequest to check it (ie. for some script at some *.googleapic.com etc.). Then it is possible to override UA "once" for such a subrequest, rather than collect a lot of sites who dislike our UA.

Comment 18

[Frank-Rainer Grahl (:frg)]

Attachment: WIP-1572290-revert-1148544-253.patch

I added a standard header and reformatted the patch. I will check it out but please take a look at it. I didn't look at the surrounding code yet. Shouldn't the observer http-on-useragent-request still be called or is http-on-modify-request used also in current SeaMonkey. If not this might break some add-ons.

Comment 19

[Dmitry Butskoy (:buc)]

(In reply to Frank-Rainer Grahl (:frg) from comment #18)

Created attachment 9120293 [details] [diff] [review]
WIP-1572290-revert-1148544-253.patch

I didn't look at the surrounding code yet. Shouldn't the observer http-on-useragent-request still be called or is http-on-modify-request used also in current SeaMonkey. If not this might break some add-ons.
No idea.
I just reverted the correspond commit, then fixed it from time to time because of code changes.

Comment 20

[Dmitry Butskoy (:buc)]

Lightning seems to spoil dramatically UA overriding.

By default, "Advertise Lightning Installation" is checked ON at Edit->Preferences->Advanced->HTTP Networking (the correspond preference parameter is "calendar.useragent.extra").
This extra UA part is added anyway, regardless of general.useragent.override common or site-specific values.

Moreover, it is not shown in "about:", and can only be detected by looking into network traffic (in Browser Console etc.)

When people, who have Lightning installed/activated, try to set some site-specific UA overrides to be "exact FF" for compatibility, actually it is "FF + Lightning", and tests are broken.

IMHO "calendar.useragent.extra" should be set to empty by default.

BTW, why the advertising of Lightning should appear in web-browser<-->web-site communication in general? It looks like violation of privacy and kinda telemetry reporting...

Comment 21

[Frank-Rainer Grahl (:frg)]

2.53.1 final
https://gitlab.com/seamonkey-project/seamonkey-2.53-mozilla/-/commit/fcb0e7f5db06165a592967782fe8853f01d9cfc7
Bug 1572290 - Site specific User Agent correction. Revert Bug 1148544 for SeaMonkey

Comment 22

[Frank-Rainer Grahl (:frg)]

Attachment: 1572290-revert-1148544-253.patch

Landed patch