426273 - [FIX]"Content Encoding Error" page displayed when redownloading a previously cancelled .tar.gz file

Status: RESOLVED FIXED

(Core :: Networking: Cache, defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

STR:
1. Navigate to http://www.w3.org/Graphics/SVG/Test/
2. Click on the first bulleted link, "W3C SVG 1.2 Tiny Test Suite" (http://www.w3.org/Graphics/SVG/Test/20070907/W3C_SVG_12_TinyTestSuite.tar.gz)
3. Click cancel on the save dialog.
4. Click the link again.
5. "Content Encoding Error" page displayed.

To fix things up all you have to do is clear your cache. Note that if you don't cancel but instead continue with the download you will be able to continue retrying successfully forever. It's that first cancel that seems to break everything.

Firefox 2 has worse behavior - it doesn't display the error page but just sits there looking dumb.

Comment 1

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Minusing based on not a regressin; should fix it in a point release..

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I cannot see the error message with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008051306 Minefield/3.0pre ID:2008051306

Bent, is this still valid for you?

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008051404 Minefield/3.0pre

Yes, happens exactly as described in comment 0.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Ok, I wasn't probably seeing this because I was behind a proxy. Now I'm able to reproduce with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008050920 Minefield/3.0pre

Will a HTTP log help here?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Possibly, yes.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: http log

I'm using session restore and I'm able to reproduce this issue when the gz file is set inside the location bar when closing Firefox. After a restart I get directly the Content Encoding Error. I hope it helps.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Presumably this part in ProcessPartialContent is responsible:

1202     // Check if the content-encoding we now got is different from the one we
1203     // got before
1204     if (PL_strcasecmp(mResponseHead->PeekHeader(nsHttp::Content_Encoding),
1205                       mCachedResponseHead->PeekHeader(nsHttp::Content_Encoding))
1206                       != 0) {
1207         Cancel(NS_ERROR_INVALID_CONTENT_ENCODING);
1208         return CallOnStartRequest();
1209     }
1210 

Someone want to check what those two values are in this case?  Note that the sever 206 response _does_ have "Content-Encoding: gzip" as far as I can tell, so presumably the issue is that our cached stuff doesn't?

Comment 8

[Boris Zbarsky [:bzbarsky]]

Looks like mCachedResponseHead has a null pointer for that header, while mResponseHead points to the string "gzip".

Comment 9

[Boris Zbarsky [:bzbarsky]]

Oh, of course.  This is the apache bug where they send 

3804[728640]:   Content-Type: application/gzip; qs=0.001
3804[728640]:   Content-Encoding: gzip

and we clear the content-encoding to work around it.  We just need to do the clearing before we do this check in ProcessPartialContent, I bet.

Ben, want to take a look into doing this?

Comment 10

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

This fixes the bug.  The only issue is that the test fails every second run with this assertion:

  ###!!! ASSERTION: storage allocated for zero data size: '!mBinding->mRecord.DataLocationInitialized()', file /Users/bzbarsky/mozilla/debug/mozilla/netwerk/cache/src/nsDiskCacheStreams.cpp, line 372

in a debug build.  Any idea how I can avoid that?

Comment 11

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

Comment on attachment 331386 [details] [diff] [review]
Fix

+++ b/netwerk/test/unit/test_gzipped_206.js
+const responseBody = ...

can you break this up into multiple lines, to stay below 80 columns?

+  if (doRangeResponse) {

inconsistent indentation in the then and else branches of this if

+           .cancel(Components.results.NS_ERROR_NO_INTERFACE);

hm, interesting choice of error code. considering that other code uses this too, maybe BINDING_ABORTED would be better?

Comment 12

[Boris Zbarsky [:bzbarsky]]

I could do BINDING_ABORTED.  I just wanted an easy hassle-free error code I knew was in Components.results.

The question from the end of comment 10 remains.... Can't really check the test in until that's resolved.  :(

Comment 13

[Dave Camp (:dcamp)]

Filed bug 448712 on the issue from comment 10.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Pushed changeset 1238046c4cce.