Remove the Accept-Charset header from HTTP requests

VERIFIED FIXED in mozilla7

Status

()

Core
Networking: HTTP
VERIFIED FIXED
7 years ago
4 years ago

People

(Reporter: hsivonen, Assigned: Ms2ger)

Tracking

(Blocks: 1 bug, {dev-doc-complete, perf, privacy})

Trunk
mozilla7
dev-doc-complete, perf, privacy
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox5-, firefox7+ wontfix, firefox8+ wontfix, firefox9+ wontfix, blocking2.0 -)

Details

(Whiteboard: [tracking+ because it's new in 6 and regressed yahoo] [qa!] [parity-IE][parity-Opera][parity-Safari] , URL)

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

7 years ago
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.
(Assignee)

Comment 1

7 years ago
Created attachment 451979 [details] [diff] [review]
WIP
Assignee: nobody → Ms2ger
Status: NEW → ASSIGNED
(Reporter)

Comment 2

7 years ago
Anne from Opera QA tells me that they are getting rid of Accept-Charset.
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

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

Updated

7 years ago
Whiteboard: [parity-IE][parity-Opera][parity-Webkit]
(Assignee)

Comment 5

7 years ago
Created attachment 453022 [details] [diff] [review]
Patch v1
Attachment #451979 - Attachment is obsolete: true
Attachment #453022 - Flags: review?(cbiesinger)

Updated

7 years ago
Blocks: 530109

Updated

7 years ago
No longer blocks: 530109

Updated

7 years ago
Whiteboard: [parity-IE][parity-Opera][parity-Webkit] → [parity-IE][parity-Opera][parity-Safari]

Comment 6

7 years ago
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

7 years ago
Ms2ger: Are you still available to work on this?
(Assignee)

Comment 8

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

Comment 9

7 years ago
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.
(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

7 years ago
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.
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.
(Reporter)

Comment 13

7 years ago
(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

7 years ago
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.
blocking2.0: --- → ?
status2.0: --- → ?
At this point I do not want and would not accept patches for this in Firefox 4. Bumping to 5.
blocking2.0: ? → -
status2.0: ? → ---
(Assignee)

Updated

6 years ago
Whiteboard: [parity-IE][parity-Opera][parity-Safari] → [parity-IE][parity-Opera][parity-Safari][needs review]

Updated

6 years ago
Attachment #453022 - Flags: review?(bzbarsky)
Comment on attachment 453022 [details] [diff] [review]
Patch v1

Mmm.... tasty code removal.
Attachment #453022 - Flags: review?(bzbarsky) → review+
(Assignee)

Updated

6 years ago
Whiteboard: [parity-IE][parity-Opera][parity-Safari][needs review] → [parity-IE][parity-Opera][parity-Safari][needs landing]

Updated

6 years ago
tracking-firefox5: --- → ?
tracking-firefox6: --- → ?
http://hg.mozilla.org/mozilla-central/rev/c3081b5db3d1
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Keywords: dev-doc-needed
Resolution: --- → FIXED
Whiteboard: [parity-IE][parity-Opera][parity-Safari][needs landing] → [parity-IE][parity-Opera][parity-Safari]
Target Milestone: --- → mozilla6

Updated

6 years ago
Attachment #453022 - Flags: review?(cbiesinger)
Documentation has been updated.
Keywords: dev-doc-needed → dev-doc-complete

Comment 19

6 years ago
not going to try to rush this into 5.
tracking-firefox5: ? → -
(Assignee)

Updated

6 years ago
Depends on: 655628

Updated

6 years ago
Depends on: 657153

Comment 20

6 years ago
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.

Updated

6 years ago
tracking-firefox6: ? → +

Updated

6 years ago
Blocks: 610998

Updated

6 years ago
Whiteboard: [parity-IE][parity-Opera][parity-Safari] → [parity-IE][parity-Opera][parity-Safari] digesting web compat

Updated

6 years ago
Whiteboard: [parity-IE][parity-Opera][parity-Safari] digesting web compat → [parity-IE][parity-Opera][parity-Safari] [tracking+ because this is new feature in 6]

Updated

6 years ago
Depends on: 664743

Updated

6 years ago
Whiteboard: [parity-IE][parity-Opera][parity-Safari] [tracking+ because this is new feature in 6] → [tracking+ because it's new in 6 and regressed yahoo] [parity-IE][parity-Opera][parity-Safari]

Comment 21

6 years ago
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.
tracking-firefox6: + → ---
tracking-firefox7: --- → +
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
philor points out that bug 655628 (changeset 4c77addce789) should also be backed out.
Backed out bug 655628 (changeset 4c77addce789) from mozilla-beta:
http://hg.mozilla.org/releases/mozilla-beta/rev/a1e3e6a625ce
Target Milestone: mozilla6 → mozilla7

Comment 25

6 years ago
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

6 years ago
(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

6 years ago
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).
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.
Depends on: 684643

Comment 29

6 years ago
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.
status-firefox7: --- → unaffected
tracking-firefox8: --- → +
tracking-firefox9: --- → +

Comment 30

6 years ago
(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
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.
Status: RESOLVED → VERIFIED
Whiteboard: [tracking+ because it's new in 6 and regressed yahoo] [parity-IE][parity-Opera][parity-Safari] → [tracking+ because it's new in 6 and regressed yahoo] [qa!] [parity-IE][parity-Opera][parity-Safari]

Comment 32

6 years ago
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.
(Reporter)

Comment 33

6 years ago
(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?
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

Updated

6 years ago
status-firefox8: --- → unaffected

Updated

6 years ago
status-firefox7: unaffected → wontfix
status-firefox8: unaffected → wontfix
(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.
(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.
(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

6 years ago
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

6 years ago
Backouts for Firefox 9:

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

Updated

6 years ago
status-firefox9: --- → wontfix
tracking-firefox10: --- → +
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 :-) )
status-firefox9: wontfix → ---
tracking-firefox10: + → ---
Keywords: dev-doc-complete → dev-doc-needed

Updated

6 years ago
status-firefox9: --- → wontfix

Comment 41

6 years ago
(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.

Updated

6 years ago
Blocks: 127214
Updated doc:

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

And mentioned on Firefox 10 for developers.
Keywords: dev-doc-needed → dev-doc-complete

Updated

5 years ago
Depends on: 742806

Comment 43

5 years ago
(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

5 years ago
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.
(Reporter)

Comment 45

5 years ago
(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.
You need to log in before you can comment on or make changes to this bug.