152275 - [FIX]saved files with GZIP encoding are not ungziped automatically

Status: RESOLVED FIXED

(Core :: Networking: HTTP, defect, P1)

Description

[tomstdenis]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1a) Gecko/20020610
BuildID:    20002061108

When you request a document to be saved to disk Mozilla will specify that GZIP
encoding is allowed but not ungzip the document before saving.  For example,
downloading a ZIP file off my web server will cause the store ZIP file to
actually be a valid GZIP compressed ZIP file.  Mozilla should either not allow
GZIP encoding or it should decompress it automatically.

Reproducible: Always
Steps to Reproduce:
1.run a server that will GZIP pretty much anything that the client allows
2.watch in horror as the download doesn't work!

Actual Results:  You have to rename the ZIP file to .ZIP.GZ then ungzip and
unzip... pain!

Expected Results:  Should be a valid ZIP file (or whatever you saved!)

This is running under a personal webserver I wrote.  I know it works mostly
because WGET will not specify GZIP encoding and the file gets downloaded intact.

Comment 1

[Tuukka Tolvanen (sp3000)]

Confirmed, linux trunk cvs 2002-06-17. maybe dup of bug 119373?

fwiw, here's the headers (tcpflow)

192.168.000.040.33137-024.112.008.087.08080: GET /iahu.zip HTTP/1.1
Host: tomstdenis.dns2go.com:8080
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.1a) Gecko/20020617
Accept:
text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,video/x-mng,image/png,image/jpeg,image/gif;q=0.2,text/css,*/*;q=0.1
Accept-Language: fi, en;q=0.50
Accept-Encoding: gzip, deflate, compress;q=0.9
Accept-Charset: ISO-8859-15, utf-8;q=0.66, *;q=0.66
Keep-Alive: 300
Connection: keep-alive


024.112.008.087.08080-192.168.000.040.33137: HTTP/1.1 200 OK

024.112.008.087.08080-192.168.000.040.33137: Allow: GET, HEAD, POST
Accept-Ranges: bytes
Connection: Keep-Alive
Content-Type: application/octet-stream
Content-Length: 98117
Content-encoding: gzip
Transfer-encoding: gzip

Comment 2

[Tuukka Tolvanen (sp3000)]

->http

Comment 3

[Darin Fisher]

cc'ing bz since i know how much he likes these sorts of bugs ;-)

Comment 4

[Boris Zbarsky [:bzbarsky]]

aaargh.

OK, so the problem is that some servers gzip and set content-encoding:gzip and
then expect you to gunzip.  Others gzip and set content-encoding:gzip and expect
you to _not_ gunzip.  So we're kinda stuck...

Right now we don't decode for certain extensions...  I'm thinking the only way
to "fix" this is to not decode when the extension and the content-encoding map
to the same content-type.  But I need to think that through some more
(suggestions welcome!).

Taking bug, in any case.

Comment 5

[Olav Vitters]

[e-mail]: Bug 145758 is related. Dupe it to this one?

Comment 6

[Malcolm Rowe]

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

Comment 7

[dharana]

Bz, which extensions (content-type) does mozilla actually always decodes?

Comment 8

[Boris Zbarsky [:bzbarsky]]

We never decode .zip or .gz, basically.  See
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAppService.cpp#145
(the nonDecodableTypes and nonDecodableExtensions tables).

The patch in bug  55690 will make things a lot better here once it's done...

Comment 9

[Michael Johnson]

It appears that your server is sending both "Content-Encoding: gzip" and
"Transfer-Encoding: gzip". Unless the file has actually been gzipped twice, then
this is incorrect. Either Content-Encoding or (preferably) Transfer-Encoding
should be used, depending on what the client says it can handle, but not both.

I doubt this is causing the problem, since Mozilla should be ignoring any
Transfer-Encoding that it has not claimed to handle, but you may want to fix
your server.

Comment 10

[Darin Fisher]

mozilla will ignore "Transfer-Encoding: gzip" since it does not send a "TE:
gzip" request header.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Though at some point we will fix that....  and then this case will _really_
break.  ;)

Comment 12

[Boris Zbarsky [:bzbarsky]]

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

Comment 13

[Boris Zbarsky [:bzbarsky]]

Attachment: Patch

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132325 [details] [diff] [review]
Patch

biesi, what do you think?  I decided I couln't think of cases when there would
be no extension and we would want to avoid decoding and all that...

Comment 15

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

Comment on attachment 132325 [details] [diff] [review]
Patch

ok... I like this approach. but some comments:

 {
   NS_PRECONDITION(aExtension, "Null Extension");

please add a precondition for aEncodingType, too
Should you also check for emptyness of aExtension/aEncodingType? The caller in
foundHeaderInfo doesn't seem to check for emptyness of urlExt.

+    nsCOMPtr<nsIURI> channelURI;
+    aChannel->GetURI(getter_AddRefs(channelURI));

Isn't mSourceUrl already initialized at this point?
oh... this is code you just copied... but could you check anyway?

+	 nsCOMPtr<nsISimpleEnumerator> encEnum;
+	 encChannel->GetContentEncodings(getter_AddRefs(encEnum));
totally offtopic question - should this be converted to use
nsIUTF8StringEnumerator? (in another bug of course)

(not marking review+ yet, pending anwers to my questions)

Comment 16

[Boris Zbarsky [:bzbarsky]]

> please add a precondition for aEncodingType, too

Will do.

> Should you also check for emptyness of aExtension/aEncodingType?

Empty just means that it won't match, which is fine.  Null would mean it would
crash....

> Isn't mSourceUrl already initialized at this point?

Looks like it is, since SetUpTempFile runs before this point.  Changed to use that.

> nsIUTF8StringEnumerator

Maybe... depends on what Darin wants.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Attachment: Updated patch

Comment 18

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

I filed bug 220672 for nsIUTF8StringEnumerator in nsIEncodedChannel

Comment 19

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 132352 [details] [diff] [review]
Updated patch

darin, what do you think?

Comment 20

[Boris Zbarsky [:bzbarsky]]

Attachment: Update to tip

biesi, stop changing interfaces out from under me!  ;)

Comment 21

[Darin Fisher]

Comment on attachment 132734 [details] [diff] [review]
Update to tip

sr=darin ... i like this heuristic.

Comment 22

[Boris Zbarsky [:bzbarsky]]

Checked in.

Comment 23

[Boris Zbarsky [:bzbarsky]]

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