228321 - content-encoding: gzip page gets saved gzipped when no extension present in the url

Status: RESOLVED FIXED

(Core Graveyard :: File Handling, defect)

Description

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

quoting bz,
"The problem here is that the change for bug 213877 added a check for the
extension before calling applyDecodingForExtension().  That's bad, since that
code is defaulting to "don't apply" and no extension means that that's what it
will do.

The correct fix is to restore the code to the way it used to be (remove the
check for a non-empty extension). "

Comment 1

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

this causes, e.g., http://slashdot.org to be saved as a gzip file, not simply as
.html.

Comment 2

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

Attachment: patch

I suppose we should remove the NS_PRECONDITION(aExtension) in
ApplyDecodingForExtension...

Comment 3

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

Attachment: alternative patch

er, actually, I wanted to upload this patch, not the other one...

Anyway, either fixes the bug.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Slightly more context for comment 0:

L. David Baron wrote:
>[14:59:20] <dbaron> so I just went to http://slashdot.org/
>[14:59:26] <dbaron> and did "File | Save Page"
>[14:59:29] <dbaron> (chose HTML only)
>[14:59:40] <dbaron> and the saved page was a gzip file
>[14:59:45] <dbaron> 1) is that supposed to happen?
                                                                                
No.
                                                                                
>[14:59:48] <dbaron> 2) is that a regression?
                                                                                
Yes.
                                                                                
>[14:59:59] * dbaron assumes it's content-encoding: gzip
                                                                                
Yep.
                                                                                
The problem here is that the change for bug 213877 added a check for the
extension before calling applyDecodingForExtension().  That's bad, since
that code is defaulting to "don't apply" and no extension means that
that's what it will do.
                                                                                
The correct fix is to restore the code to the way it used to be (remove
the check for a non-empty extension).

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 137335 [details] [diff] [review]
alternative patch

We want this one.  If we can't get an exthandler here, it's better to leave
slashdot gzipped than to decompress .tar.gz files...

I don't think the assert needs removing, since this should be passing "", not
null.

Comment 6

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

Comment on attachment 137335 [details] [diff] [review]
alternative patch

darin: this is a very short patch, and would be nice to get for 1.6.

Comment 7

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

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

Comment 8

[Jerry Baker]

Can we remove the "with no extension present" from the summary since my
duplicate displayed this problem even when the file name to save was specified
with an extension?

Comment 9

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

err, really? what was the exact url?

Comment 10

[Jerry Baker]

https://www.attwireless.com/ocs/billing/eBills/AWSUser?[long query string]

I was not aware that you meant no extension in the URL. I read that as no
extension in the saved file name.

Comment 11

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

ah, sorry for not making that clear.

Comment 12

[Darin Fisher]

Comment on attachment 137335 [details] [diff] [review]
alternative patch

sr=darin

Comment 13

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

Comment on attachment 137335 [details] [diff] [review]
alternative patch

fixes the problem that some html pages (which the server sent gzipped) were not
ungzipped during download, low risk.

Comment 14

[chris hofmann]

Comment on attachment 137335 [details] [diff] [review]
alternative patch

a=chofmann for 1.6

Comment 15

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

Checking in resources/content/contentAreaUtils.js;
/cvsroot/mozilla/xpfe/communicator/resources/content/contentAreaUtils.js,v  <--
 contentAreaUtils.js
new revision: 1.99; previous revision: 1.98
done

Comment 16

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

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

Comment 17

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

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