58037 - Accept-encoding: "identity" in not needed, "compress" should be depreciated

Status: RESOLVED FIXED

(Core :: Networking: HTTP, enhancement, P3)

Description

[Adrian Havill]

the "identity" encoding being transmitted in the accept-encoding is not needed
and
implied according to HTTP/1.1
<URL:http://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html#sec14.3>, unless the
identity is to be refused with a q value of zero. It would be better to remove
it so
the request is less redundant.

Also, shouldn't the LZW based compress, which is not used anywhere, be supported
but
indicated as being "preferred less" than the better-maintained-with-a-future
zlib patent
free based gzip and deflate. Support it, of course, but given a choice between
the two,
Mozilla should request the format that we have a module maintainer for and the
format that
everything inside Mozilla (JAR, etc) is compressed with, so the q values should
be tweaked.

Leave the q value alone if the value is "x-compress", as the client is probably
sending a
request to an older server that doesn't understand q values.

Comment 1

[Adrian Havill]

Attachment: removes "identity", pretty prints (", ") values, prefers zlib-based

Comment 2

[Keyser Sose]

Marking NEW so someone will look at it.

Comment 3

[Dawn Endico]

This bug has a patch which needs to be reviewed. Adding keywords.

Comment 4

[neeti]

This patch incorrectly strips away the q values.

For e.g. if an accept-encoding was set as 
Accept-Encoding: gzip;q=1.0, identity; q=0.5, *;q=0

it changes it to 
Accept-Encoding: gzip,*

Comment 5

[neeti]

We should not be stripping away the "q" values.

Comment 6

[Adrian Havill]

Attachment: revised patch that lowers "compress"'s q without stripping q values

Comment 7

[neeti]

We should be doing all this functionality in SetAcceptEncodings rather than 
GetAcceptEncodings. Also the variable lowQ should be double instead of a float.
Also sscanf should be changed to PR_sscanf.

Neeti

Comment 8

[neeti]

Also strlen and sprintf should be changed to PL_Strlen and PR_snprintf 
respectively.

Neeti

Comment 9

[Gagan]

->neeti

Comment 10

[neeti]

This patch needs some changes

Comment 11

[chris hofmann]

should this bug be targeted for 0.9.1?  is the work going be wrapped up in the
next week or two?

Comment 12

[neeti]

The patch submitted need some changes as mentioned. Changing target milestone to   
mozilla0.9.2

Comment 13

[benc]

Open Networking bugs, qa=tever -> qa to me.

Comment 14

[Adrian Havill]

Need sr= for the following patch.

Comment 15

[Adrian Havill]

Attachment: updated patch incorporating suggestions against latest tree

Comment 16

[Christopher Blizzard (:blizzard)]

sr=blizzard

Comment 17

[Bradley Baetz (:bbaetz)]

Why not just change the default in all.js? This routine is only called for the
pref value, and maybe someone wants to prefer compress, for whatever reason.

I'll happily r= the one line patch for that. 

Comment 18

[Adrian Havill]

Attachment: simpler non-overkill bbaetz fix

Comment 19

[Bradley Baetz (:bbaetz)]

r=bbaetz

Comment 20

[Darin Fisher]

sr=darin

Comment 21

[Darin Fisher]

fix checked in (on the trunk)