572652 - Remove the Accept-Charset header from HTTP requests

Status: VERIFIED FIXED

(Core :: Networking: HTTP, defect)

Description

[Henri Sivonen (:hsivonen)]

Steps to reproduce:
 1) Load http://www.delorie.com:81/some/url.txt

Actual results:
The Accept-Charset header is sent. The value of the header depends on the localization of Firefox, which means configuration-based entropy is exposed and can by used for fingerprinting. See https://panopticlick.eff.org/ The header also bloats each HTTP request, typically by 49 bytes.

Expected results:
Expected no Accept-Charset header to be sent, because the wide support for UTF-8 has obsoleted this HTTP feature.

Additional information:
Internet Explorer (IE8 and IE9 Platform Preview tested) does not send the Accept-Charset header, which suggests that it's not needed for legacy compatibility.

Comment 1

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: WIP

Comment 2

[Henri Sivonen (:hsivonen)]

Anne from Opera QA tells me that they are getting rid of Accept-Charset.

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

FYI, this will (at least as of a year ago) break some sites; see bug 507197 comment 1 (and I think hhaamu had another site as an example, too).  

It doesn't seem like it's a wide number of sites, though, since we'd been inadvertently not sending it in our nightlies for quite some time when that bug was filed, but everyone should expect some breakage if IE8 hasn't cleaned up everything (babelfish, for instance, is still broken).

Comment 4

[philippe (part-time)]

FWIW, Safari 5 + on OS X 10.6 doesn't send it, based on http://browserspy.dk/headers.php

Comment 5

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

Attachment: Patch v1

Comment 6

[Dave Garrett]

I think the current patch is missing some parts. It removes uses of intl.accept_charsets but this pref seems to be obsolete and not in use anymore.
http://mxr.mozilla.org/mozilla-central/search?string=intl.accept_charsets

The actual live pref seems to be intl.charset.default which is used via the INTL_ACCEPT_CHARSET macro. PrepareAcceptCharsets tacks on everything after it. This is the pref you need to remove all of for this bug.
http://mxr.mozilla.org/mozilla-central/search?string=intl.charset.default

You might want to file a separate bug/patch for the obsolete pref because it should probably be removed regardless of the current one that's used here.

Comment 7

[Dave Garrett]

Ms2ger: Are you still available to work on this?

Comment 8

[:Ms2ger (he/him; ⌚ UTC+1/+2)]

I'm not convinced that removing the actual default charset handling is in scope for this bug.

Comment 9

[Dave Garrett]

If you remove the ability to send the accept-charset header, then the menu in prefs->fonts->advanced->encoding where a user sets intl.charset.default for this would not do much of anything anymore. The basic premise of this bug was that this feature was to be removed in favor of just using UTF-8 by default.

Comment 10

[Masatoshi Kimura [:emk]]

(In reply to comment #9)
Although other browsers do not send Accept-Charset, they select a fallback charset based on the preference. Many websites will be broken if the actual default charset handling is removed.
> The basic premise of this bug was
> that this feature was to be removed in favor of just using UTF-8 by default.
I thought the purpose of this bug was to prevent fingerprinting. Why this feature needs to be removed only to stop sending Accept-Charset?

Comment 11

[Dave Garrett]

I'm not suggesting that detection be removed, but is this setting still really needed for it to work? If I'm wrong on that then sorry for the confusion. intl.accept_charsets does appear abandoned, in any case.

Comment 12

[Masatoshi Kimura [:emk]]

intl.charset.default is still used in some places (including charset detection).
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsDocumentViewer.cpp#2971
http://mxr.mozilla.org/mozilla-central/source/toolkit/components/search/nsSearchService.js#518
http://mxr.mozilla.org/mozilla-central/source/content/html/document/src/nsHTMLDocument.cpp#884
You can remove the pref from nsHttpHandler.cpp, but please do not nuke it blindly.

Comment 13

[Henri Sivonen (:hsivonen)]

(In reply to comment #8)
> I'm not convinced that removing the actual default charset handling is in scope
> for this bug.

Indeed, I did not intend this bug to cover any change to the fallback encoding used for decoding text/html content with no explicit encoding information is given (no HTTP, meta or BOM).

Comment 14

[Dave Garrett]

Ok, sorry for inadvertently attempting to expand the scope of this bug.

In any case, requesting betaN blocking or at least wanted status so this can get some beta testing to make sure it doesn't break too much. Hopefully the reviewer will have the time to get to this one soon, though obviously there are a lot of other patches waiting in line.

Comment 15

[Benjamin Smedberg]

At this point I do not want and would not accept patches for this in Firefox 4. Bumping to 5.

Comment 16

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 453022 [details] [diff] [review]
Patch v1

Mmm.... tasty code removal.

Comment 17

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/c3081b5db3d1

Comment 18

[Eric Shepherd [:sheppy]]

Documentation has been updated.

Comment 19

[Asa Dotzler [:asa]]

not going to try to rush this into 5.

Comment 20

[Kev Needham [:kev]]

For QA, are we testing against popular sites with a non-latin charset? Per bug 657153, it looks like some sites are potentially relying on/expecting it to be sent with Firefox. If this is an issue, it'd be great to understand the scope.

Comment 21

[LegNeato]

I backed this out of beta6:

    http://hg.mozilla.org/releases/mozilla-beta/rev/07c59e8e900a

due to bug 657153. We could use another 6 weeks of exploratory / proactive QA to see what else breaks and get more info in bug 657153.

Comment 22

[Nick Thomas [:nthomas] (UTC+12)]

The backout is burning across all platforms, eg
 /usr/bin/ccache /tools/gcc-4.5/bin/g++ -o nsHttpChannel.o -c ... /builds/slave/m-beta-lnx-dbg/build/netwerk/protocol/http/nsHttpChannel.cpp
../../../../netwerk/protocol/http/nsHttpHandler.cpp: In member function 'nsresult nsHttpHandler::AddStandardRequestHeaders(nsHttpHeaderArray*, PRUint8, PRBool)':
../../../../netwerk/protocol/http/nsHttpHandler.cpp:380:29: error: 'Accept_Charset' is not a member of 'nsHttp'

See http://tbpl.mozilla.org/?tree=Mozilla-Beta&rev=07c59e8e900a

Comment 23

[Nick Thomas [:nthomas] (UTC+12)]

philor points out that bug 655628 (changeset 4c77addce789) should also be backed out.

Comment 24

[Matt Brubeck (:mbrubeck)]

Backed out bug 655628 (changeset 4c77addce789) from mozilla-beta:
http://hg.mozilla.org/releases/mozilla-beta/rev/a1e3e6a625ce

Comment 25

[George Carstoiu]

Mozilla/5.0 (X11; Linux i686; rv:6.0) Gecko/20100101 Firefox/6.0 - beta 5

Going to http://www.delorie.com:81/some/url.txt displays, amongst others, the following line (across OS's):

Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7

This means that the back-out worked, but in this case what should happen to the status of this bug? It can't be set to Verified Fixed because it contradicts the main reason of this issue. Should it be Reopened?

Comment 26

[Dave Garrett]

(In reply to George Carstoiu from comment #25)
It was backed out of beta only; it's still in Aurora and Trunk. It's still FIXED, just for one later release now. (and as such, the milestone was changed above)

Comment 27

[George Carstoiu]

Mozilla/5.0 (X11; Linux i686; rv:8.0a1) Gecko/20110805 Firefox/8.0a1

I can confirm that this wasn't backed-out on Nightly and Aurora. Not marking as Verified until it reaches the release channel (Firefox 7).

Comment 28

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20100101 Firefox/7.0

The Accept Charset string is still present in the current beta 1. 

However, it is removed in the current Mozilla aurora and nightly.

Comment 29

[LegNeato]

On 7 this looks fine (backed out/sends the header) via code inspection. I'm not sure we want to do anything here.

Someone please check me / check the behavior rather than code inspection.

Comment 30

[Daniel C]

(In reply to Christian Legnitto [:LegNeato] from comment #29)
> On 7 this looks fine (backed out/sends the header) via code inspection. I'm
> not sure we want to do anything here.
> 
> Someone please check me / check the behavior rather than code inspection.

Load this page in Aurora and Beta and compare the results: http://software.hixie.ch/utilities/cgi/test-tools/echo

Comment 31

[Virgil Dicu [:virgil] [QA]]

Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (X11; Linux x86_64; rv:9.0a2) Gecko/20111003 Firefox/9.0a2
Mozilla/5.0 (X11; Linux x86_64; rv:10.0a1) Gecko/20111002 Firefox/10.0a1

Verified on Ubuntu 11.04, Mac OS 10.6, Windows XP, Windows 7 in Firefox 8 Beta1, Firefox 9, Firefox 10 using the URL in comment 0. The Accept Charset header is no longer sent on the specified branches.

Comment 32

[LegNeato]

I backed this out of mozilla-beta for Firefox 8 as well:

http://hg.mozilla.org/releases/mozilla-beta/rev/12d87aaf8960

We need to figure out what version we are going to keep this in and put pressure on partners / widely announce the change.

Comment 33

[Henri Sivonen (:hsivonen)]

(In reply to Christian Legnitto [:LegNeato] from comment #32)
> We need to figure out what version we are going to keep this in and put
> pressure on partners / widely announce the change.

Is Yahoo! the only partner holding us back on this one? Maybe we need more effective lines of contact to Y! than the lines of contact used so far? Or maybe this is one of those things they won't fix until they see it's real in a release?

Comment 34

[Simona Badau, Desktop QA]

Verified that this fix is backed out from Firefox 8 beta 6. 
Going to http://www.delorie.com:81/some/url.txt - displayes "Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7" as one of the lines.

Tested on Windows XP, Windows 7, Ubuntu 10.10 and Mac OS X 10.6.
Mozilla/5.0 (Windows NT 5.1; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Windows NT 6.1; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (X11; Linux x86_64; rv:8.0) Gecko/20100101 Firefox/8.0
Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:8.0) Gecko/20100101 Firefox/8.0

Comment 35

[Alex Keybl [:akeybl]]

(In reply to Christian Legnitto [:LegNeato] from comment #32)
> I backed this out of mozilla-beta for Firefox 8 as well:
> 
> http://hg.mozilla.org/releases/mozilla-beta/rev/12d87aaf8960
> 
> We need to figure out what version we are going to keep this in and put
> pressure on partners / widely announce the change.

[Triage Comment]
Nothing has changed here since this was backed out on FF8 beta. Let's do the same for 9beta/10aurora/m-c, and come to a consensus as to what needs to be communicated before this can land again.

Comment 36

[Dão Gottwald [:dao]]

(In reply to Alex Keybl [:akeybl] from comment #35)
> (In reply to Christian Legnitto [:LegNeato] from comment #32)
> > I backed this out of mozilla-beta for Firefox 8 as well:
> > 
> > http://hg.mozilla.org/releases/mozilla-beta/rev/12d87aaf8960
> > 
> > We need to figure out what version we are going to keep this in and put
> > pressure on partners / widely announce the change.
> 
> [Triage Comment]
> Nothing has changed here since this was backed out on FF8 beta.

Actually, bug 657153 has been resolved.

Comment 37

[Alex Keybl [:akeybl]]

(In reply to Dão Gottwald [:dao] from comment #36)
> (In reply to Alex Keybl [:akeybl] from comment #35)
> > (In reply to Christian Legnitto [:LegNeato] from comment #32)
> > > I backed this out of mozilla-beta for Firefox 8 as well:
> > > 
> > > http://hg.mozilla.org/releases/mozilla-beta/rev/12d87aaf8960
> > > 
> > > We need to figure out what version we are going to keep this in and put
> > > pressure on partners / widely announce the change.
> > 
> > [Triage Comment]
> > Nothing has changed here since this was backed out on FF8 beta.
> 
> Actually, bug 657153 has been resolved.

Thanks - you're right. Are we confident that yahoo has resolved across the board at this point? bug 664743 is still open currently.

Comment 38

[LegNeato]

Let's back this out for Firefox 9 and keep it in for 10 (ESR). We'll try to put some pressure on Yahoo and message for 10.

Comment 39

[LegNeato]

Backouts for Firefox 9:

http://hg.mozilla.org/releases/mozilla-beta/rev/7018a8ca5a3a
http://hg.mozilla.org/releases/mozilla-beta/rev/cba021dbebc9

Comment 40

[Jean-Yves Perrier [:teoli]]

I've added dev-doc-needed as we need to update the doc regarding the version change.

(Christian, can you help us by switching/adding dev-doc-needed when things got "backed-out", or approved for beta/aurora? It would help us a great deal for the documentation :-) )

Comment 41

[Jochen Roderburg]

(In reply to Christian Legnitto [:LegNeato] from comment #38)
> Let's back this out for Firefox 9 and keep it in for 10 (ESR). We'll try to
> put some pressure on Yahoo and message for 10.

Great, now this has crept into the new released versions (Firefox 10 and SeaMonkey 2.7) ansd things like bug 664743 (Yahoo Babelfish translation) still don't work.

Comment 42

[Eric Shepherd [:sheppy]]

Updated doc:

https://developer.mozilla.org/en/HTTP/Content_negotiation#The_Accept-Charset:_header

And mentioned on Firefox 10 for developers.

Comment 43

[Jochen Roderburg]

(In reply to Jochen Roderburg from comment #41)
> (In reply to Christian Legnitto [:LegNeato] from comment #38)
> > Let's back this out for Firefox 9 and keep it in for 10 (ESR). We'll try to
> > put some pressure on Yahoo and message for 10.
> 
> Great, now this has crept into the new released versions (Firefox 10 and
> SeaMonkey 2.7) ansd things like bug 664743 (Yahoo Babelfish translation)
> still don't work.

This problem has now resolved itself, because Yahoo Babelfish has been morphed into Bing Translator with a totally different and working webinterface.

Comment 44

[Dan Dart]

This being missing appears to break PHP's charset detection.
In Chrome where this header is sent, for example with UTF-8, Mongo saves data successfully, but in Firefox it doesn't know what to encode it from, and Mongo exceptions out. I believe it's still important to keep since this can break a few things.

Comment 45

[Henri Sivonen (:hsivonen)]

(In reply to Dan Dart from comment #44)
> This being missing appears to break PHP's charset detection.

Can you, please, provide a reference to said detection?

> In Chrome where this header is sent, for example with UTF-8, Mongo saves
> data successfully, but in Firefox it doesn't know what to encode it from,
> and Mongo exceptions out. I believe it's still important to keep since this
> can break a few things.

Chrome is the *only* major browser left that sends it. Firefox doesn’t. IE doesn’t. Safari doesn’t. Opera doesn’t. Any site that relies on Accept-Charset is simply broken.