364412 - Crash when updating in nsIncrementalDownload::OnStartRequest, attempting to allocate a huge amount of memory (was "[Mac] Crash when updating from FF 2->2.0.0.1")

Status: VERIFIED FIXED

(Toolkit :: Application Update, defect)

Description

[Marcia Knous [:marcia]]

Seen during live update testing going from FF 2->2.0.0.1. This is a Mac only bug.

STR:

1. Download the release version of FF 2
2. Check for updates.
3. Update is found, but then things go awry...

Results:

Intel Mac crashes immediately
PPC Mac begins the update process, freezes and then crashes.

My talkback ID on the PPC Mac is TB27542998K.

Comment 1

[Michael Morgan [:morgamic]]

The os string in the update URL was not being received correctly.  "osx" was expected to be "mac".  This was an oversight when migrating the operating systems table from bouncer1 -> bouncer2.

I've since updated the bouncer2 DB to have the correct value and things should be updated once sentry runs its next new files test, which is before the hour (next few minutes).

Comment 2

[J. Paul Reed [:preed]]

This could be all platforms, not just Mac. There's an easy way to test this: we just need to deploy a snippet to a non-existent bouncer URL that will redirect to a webpage; that's what caused the crash.

We can try it on one of the -test channels, and check out all the platforms.

Comment 3

[Mike Schroepfer]

Does the client recover properly now that the server is fixed?

Comment 4

[(not reading, please use [e-mail] instead)]

from marcia's talkback report:

kill()
abort()
__cxxabiv1::__unexpected()  [mozilla/db/sqlite3/src/pragma.c, line 501]
__cxa_rethrow()  [mozilla/db/sqlite3/src/pragma.c, line 1112]
operator()  [mozilla/db/sqlite3/src/pragma.c, line 1112]
operator()  [mozilla/db/sqlite3/src/pragma.c, line 1112]
nsIncrementalDownload::OnStartRequest()  [mozilla/netwerk/base/src/nsIncrementalDownload.cpp, line 51]
nsHttpChannel::CallOnStartRequest()  [mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp, l]
nsHttpChannel::ProcessNormal()  [mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp, line 9]
nsHttpChannel::ProcessResponse()  [mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp, line]
nsInputStreamPump::OnStateStart()  [mozilla/db/sqlite3/src/pragma.c, line 442]
nsInputStreamPump::OnInputStreamReady()  [mozilla/db/sqlite3/src/pragma.c, li]
nsInputStreamReadyEvent::EventHandler()
PL_HandleEvent()  [mozilla/xpcom/threads/plevent.c, line 689]
PL_ProcessPendingEvents()  [mozilla/xpcom/threads/plevent.c, line 623]
CoreFoundation.299.37.0 + 0x4800 (0x901c4800)
CoreFoundation.299.37.0 + 0x20b8 (0x901c20b8)
CoreFoundation.299.37.0 + 0x69e4 (0x901c69e4)
RunCurrentEventLoopInMode()
ReceiveNextEventCommon()
AcquireNextEventInMode()
RunApplicationEventLoop()
nsAppShell::Run()  [mozilla/widget/src/mac/nsAppShell.cpp, line 94]
nsAppStartup::Run()  [mozilla/db/sqlite3/src/pragma.c, line 152]
XRE_main()  [mozilla/toolkit/xre/nsAppRunner.cpp, line 2440]
_start()   start()

Comment 5

[(not reading, please use [e-mail] instead)]

> There's an easy way to test this: we just need to deploy a snippet to a 
> non-existent bouncer URL that will redirect to a webpage; that's what caused the > crash.

preed, do you mean:

1) have app.update.url.override point to a URL that redirects to a webpage (ex: https://aus2.mozilla.org/)
2) in the reachable snippet, have the URL attribute of the patch element point to a page that redirects to a webpage (ex:
<patch type="complete" URL="https://aus2.mozilla.org" hashFunction="SHA1" hashValue="a3ab33bc0dcabd86050013826da8e3a11ab691b2" size="54"/>)

on windows (I need to try mac 10.3, where marcia saw the crash)
for 1, I get the "AUS: Update XML File Not Found (404)" error message in the software update wizard
for 2, I get the "The integrity of the update could not be verified (Contact your Administrator)" error message in the software update wizard

or, did you mean something else and I'm confused?

even once morgamic has fixed the server side, I'd want to reproduce this crasher client side and figure out the crash.

Comment 6

[J. Paul Reed [:preed]]

(In reply to comment #5)

> preed, do you mean:
> 
> 1) have app.update.url.override point to a URL that redirects to a webpage (ex:
> https://aus2.mozilla.org/)
> 2) in the reachable snippet, have the URL attribute of the patch element point
> to a page that redirects to a webpage (ex:
> <patch type="complete" URL="https://aus2.mozilla.org" hashFunction="SHA1"
> hashValue="a3ab33bc0dcabd86050013826da8e3a11ab691b2" size="54"/>)
> 
> on windows (I need to try mac 10.3, where marcia saw the crash)
> for 1, I get the "AUS: Update XML File Not Found (404)" error message in the
> software update wizard
> for 2, I get the "The integrity of the update could not be verified (Contact
> your Administrator)" error message in the software update wizard

I meant number 2.

So, the real problem we saw yesterday was a snippet that pointed to a bouncer URL that was something like:

http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US

which bouncer2 redirects to:

http://www.mozilla.org/download.html

The problem may be that the request succeeds, but redirects to a webpage, not a mar, and the updater isn't expecting that? Also, maybe bouncer2 does the redirect slightly differently than bouncer1 (I notice that bouncer1 redirects to www.mozilla.com, not the webpage above.)

> even once morgamic has fixed the server side, I'd want to reproduce this
> crasher client side and figure out the crash.

Most def. If you're around, I can whip up a snippet that should cause a blow up. 

Find me on IRC.

Comment 7

[(not reading, please use [e-mail] instead)]

Attachment: test case that shows the crash (thanks preed for the how to info)

set your app.update.url.override pref to point to that attachment, and boom.

Comment 8

[(not reading, please use [e-mail] instead)]

*** Downloader.onProgress: onProgress: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, 3234/3234
*** UI:DownloadingPage.onProgress
*** Downloader.onProgress: onProgress: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, 6468/3234
*** UI:DownloadingPage.onProgress
*** Downloader.onProgress: onProgress: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, 9702/3234
*** UI:DownloadingPage.onProgress
*** Downloader.onProgress: onProgress: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, 10323/3234
*** UI:DownloadingPage.onProgress
firefox-bin(234,0xa000cfc0) malloc: *** vm_allocate(size=4294963200) failed (error code=3)
firefox-bin(234,0xa000cfc0) malloc: *** error: can't allocate region
firefox-bin(234,0xa000cfc0) malloc: *** set a breakpoint in szone_error to debug
terminate called after throwing an instance of 'std::bad_alloc'
  what():  St9bad_alloc

more details coming soon...

Comment 9

[(not reading, please use [e-mail] instead)]

yikes, allocating 4294963200 bytes is not a good sign.

Comment 10

[Benjamin Smedberg]

We've had this before, when the bouncer config redirects to mozilla.com.

Comment 11

[Marcia Knous [:marcia]]

Just to be clear, during the first iteration of testing last evening we did see this on any other platform but Mac (Intel and PPC).

(In reply to comment #2)
> This could be all platforms, not just Mac. There's an easy way to test this: we
> just need to deploy a snippet to a non-existent bouncer URL that will redirect
> to a webpage; that's what caused the crash.
> 
> We can try it on one of the -test channels, and check out all the platforms.
> 

Comment 12

[(not reading, please use [e-mail] instead)]

here's the real stack:

#0  0x9003d1dc in kill ()
#1  0x9010f2af in raise ()
#2  0x9010de02 in abort ()
#3  0x90b4039c in __gnu_cxx::__verbose_terminate_handler ()
#4  0x90b3e602 in __gxx_personality_v0 ()
#5  0x90b3e640 in std::terminate ()
#6  0x90b3e754 in __cxa_throw ()
#7  0x90b495f3 in operator new ()
#8  0x90b496a5 in operator new[] ()
#9  0x13acb1ac in nsIncrementalDownload::OnStartRequest (this=0x37f03940, request=0x37f2e540, context=0x0) at /Users/sspitzer/Desktop/trunk/mozilla/netwerk/base/src/nsIncrementalDownload.cpp:616
#10 0x13b676a3 in nsHttpChannel::CallOnStartRequest (this=0x37f2e510) at /Users/sspitzer/Desktop/trunk/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp:712
#11 0x13b6c6d7 in nsHttpChannel::ProcessNormal (this=0x37f2e510) at /Users/sspitzer/Desktop/trunk/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp:883
#12 0x13b6d2e6 in nsHttpChannel::ProcessResponse (this=0x37f2e510) at /Users/sspitzer/Desktop/trunk/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp:767
#13 0x13b6d6e1 in nsHttpChannel::OnStartRequest (this=0x37f2e510, request=0x37f2fec0, ctxt=0x0) at /Users/sspitzer/Desktop/trunk/mozilla/netwerk/protocol/http/src/nsHttpChannel.cpp:3943
#14 0x13acc451 in nsInputStreamPump::OnStateStart (this=0x37f2fec0) at /Users/sspitzer/Desktop/trunk/mozilla/netwerk/base/src/nsInputStreamPump.cpp:434
#15 0x13acd077 in nsInputStreamPump::OnInputStreamReady (this=0x37f2fec0, stream=0x37f36bfc) at /Users/sspitzer/Desktop/trunk/mozilla/netwerk/base/src/nsInputStreamPump.cpp:390
#16 0x013aad88 in nsInputStreamReadyEvent::Run (this=0x37f35d90) at /Users/sspitzer/Desktop/trunk/mozilla/xpcom/io/nsStreamUtils.cpp:111
#17 0x0134fda6 in nsThread::ProcessNextEvent (this=0x2111e10, mayWait=1, result=0xbfffd8ac) at /Users/sspitzer/Desktop/trunk/mozilla/xpcom/threads/nsThread.cpp:482
#18 0x012f8ad8 in NS_ProcessNextEvent_P (thread=0x2111e10, mayWait=1) at nsThreadUtils.cpp:225
#19 0x1599855a in nsBaseAppShell::Run (this=0x2125ed0) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:153
#20 0x15982976 in nsAppShell::Run (this=0x2125ed0) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/cocoa/nsAppShell.mm:319
#21 0x15982ce2 in -[AppShellDelegate runAppShell] (self=0x213fe00, _cmd=0x2125910) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/cocoa/nsAppShell.mm:418
#22 0x9260b0c7 in __NSFireDelayedPerform ()
#23 0x90829bc9 in CFRunLoopRunSpecific ()
#24 0x90828eb5 in CFRunLoopRunInMode ()
#25 0x92dcdb90 in RunCurrentEventLoopInMode ()
#26 0x92dcd297 in ReceiveNextEventCommon ()
#27 0x92dcd0ee in BlockUntilNextEventMatchingListInMode ()
#28 0x9326f465 in _DPSNextEvent ()
#29 0x9326f056 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#30 0x1598276d in nsAppShell::ProcessNextNativeEvent (this=0x2125ed0, aMayWait=0) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/cocoa/nsAppShell.mm:273
#31 0x159984cb in nsBaseAppShell::DoProcessNextNativeEvent (this=0x2125ed0, mayWait=0) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:136
#32 0x1599887c in nsBaseAppShell::OnProcessNextEvent (this=0x2125ed0, thr=0x2111e10, mayWait=0, recursionDepth=0) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:209
#33 0x15982a38 in nsAppShell::OnProcessNextEvent (this=0x2125ed0, aThread=0x2111e10, aMayWait=0, aRecursionDepth=0) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/cocoa/nsAppShell.mm:342
#34 0x0134fca2 in nsThread::ProcessNextEvent (this=0x2111e10, mayWait=0, result=0xbfffe7c4) at /Users/sspitzer/Desktop/trunk/mozilla/xpcom/threads/nsThread.cpp:469
#35 0x012f8c39 in NS_ProcessPendingEvents_P (thread=0x2111e10, timeout=20) at nsThreadUtils.cpp:179
#36 0x15998467 in nsBaseAppShell::NativeEventCallback (this=0x2125ed0) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/xpwidgets/nsBaseAppShell.cpp:111
#37 0x159824b7 in nsAppShell::ProcessGeckoEvents (this=0x2125ed0) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/cocoa/nsAppShell.mm:198
#38 0x15982c9f in -[AppShellDelegate handlePortMessage:] (self=0x213fe00, _cmd=0x90aa7c40, aPortMessage=0x33e1d290) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/cocoa/nsAppShell.mm:407
#39 0x92646a4c in __NSFireMachPort ()
#40 0x90839773 in __CFMachPortPerform ()
#41 0x90829a14 in CFRunLoopRunSpecific ()
#42 0x90828eb5 in CFRunLoopRunInMode ()
#43 0x92dcdb90 in RunCurrentEventLoopInMode ()
#44 0x92dcd297 in ReceiveNextEventCommon ()
#45 0x92dcd0ee in BlockUntilNextEventMatchingListInMode ()
#46 0x9326f465 in _DPSNextEvent ()
#47 0x9326f056 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] ()
#48 0x93268ddb in -[NSApplication run] ()
#49 0x1598294a in nsAppShell::Run (this=0x2125ed0) at /Users/sspitzer/Desktop/trunk/mozilla/widget/src/cocoa/nsAppShell.mm:315
#50 0x16cfdfe7 in nsAppStartup::Run (this=0x2146900) at /Users/sspitzer/Desktop/trunk/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:171
#51 0x0020f233 in XRE_main (argc=1, argv=0xbffffa7c, aAppData=0x2040) at /Users/sspitzer/Desktop/trunk/mozilla/toolkit/xre/nsAppRunner.cpp:2520
#52 0x00001f36 in main (argc=1, argv=0xbffffa7c) at /Users/sspitzer/Desktop/trunk/mozilla/browser/app/nsBrowserApp.cpp:61

(from the my trunk build)

note, in nsIncrementalDownload::OnStartRequest, mChunkSize is -7, and so:

mChunk = new char[mChunkSize];

is where we crash.

Comment 13

[(not reading, please use [e-mail] instead)]

from the code:

611       // Adjust mChunkSize accordingly if mCurrentSize is close to mTotalSize.
612       nsInt64 diff = mTotalSize - mCurrentSize;
613       if (diff < nsInt64(mChunkSize))
614         mChunkSize = PRUint32(diff);
615
616       mChunk = new char[mChunkSize];

in our case, mTotalSize is 3234, but mCurrentSize is 10323, so our diff is -7089

continuing to investigate...

Comment 14

[(not reading, please use [e-mail] instead)]

Attachment: screen shot after my wallpaper patch.

Comment 15

[(not reading, please use [e-mail] instead)]

Attachment: wall paper patch, but need to talk to biesi / darin about what might really be going on

Comment 16

[(not reading, please use [e-mail] instead)]

for http://www.mozilla.org/download.html, using the LiveHTTP Headers add on (nice!), I can see this header:  "Content-Length: 3238", and, using page info, the page is also 3238 bytes.

but yet, we keep downloading more bytes.  not sure why yet.

Comment 17

[(not reading, please use [e-mail] instead)]

to reproduce this crash (on mac and windows):

1) set app.update.url.override pref to https://bugzilla.mozilla.org/attachment.cgi?id=249272&action=view
2) do "Help | Check for Updates..."
3) click the "Download and Install"

it should crash.

Comment 18

[(not reading, please use [e-mail] instead)]

Attachment: nspr log (nsHttp:5) for biesi

Comment 19

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

Here's what happens:
- We make the initial request to the server
- in the specific case here, we had the content cached, but this 
  doesn't matter much.
  The server says not modified, or alternatively it replies with 200 
  OK and sends us the file
- The file is gzip compressed. This means: The Content-Length the 
  server sends is the length of the gzipped content, but the data
  nsIncrementalDownload gets is uncompressed
- The first request finishes.
  nsIncrementalDownload::OnStopRequest notices that mTotalSize (the 
  content-length) is not equal to mCurrentSize (the data we got),
  so it attempts to download another chunk.
- The server responds with 200 OK to that new request. But, we
  already have more data than the content-length said, because
  of the compression, so we get a negative difference in the
  line mentioned above.

I think there are two possible solutions:
- disable compression (nsIEncodedChannel::applyConversion) in
  nsIncrementalDownload::onStartRequest
- sspitzer's patch

Comment 20

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

Attachment: alternative patch

or, does this work?

Comment 21

[(not reading, please use [e-mail] instead)]

Attachment: biesi patch + mine wall paper

Comment 22

[(not reading, please use [e-mail] instead)]

Attachment: unified diff

Comment 23

[(not reading, please use [e-mail] instead)]

biesi's patch works, and my wallpaper does not get hit.

from the console, after biesi's patch:

*** Downloader: downloadUpdate: Downloading from http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US to /Users/sspitzer/Desktop/trunk/debug-build/dist/MinefieldDebug.app/Contents/MacOS/updates/0/update.mar
*** Downloader: onStartRequest: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US
*** UI:DownloadingPage
*** Downloader.onProgress: onProgress: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, 3238/3238
*** UI:DownloadingPage.onProgress
*** Downloader.onProgress: onProgress: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, 6476/3238
*** UI:DownloadingPage.onProgress
*** Downloader.onProgress: onProgress: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, 9714/3238
*** UI:DownloadingPage.onProgress
*** Downloader.onProgress: onProgress: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, 10347/3238
*** UI:DownloadingPage.onProgress
WARNING: server response was unexpected: file /Users/sspitzer/Desktop/trunk/mozilla/netwerk/base/src/nsIncrementalDownload.cpp, line 564
*** Downloader: onStopRequest: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, status = 2147549183
*** Downloader: onStopRequest: Non-verification failure
*** General: Transfer Error: Failed (Unknown Reason), code: 2152398849
*** Downloader: Setting state to: download-failed
*** UI:DownloadingPage
*** Downloader: onStopRequest: Verification of patch failed, downloading complete update
*** General: Reading Status File: /Users/sspitzer/Desktop/trunk/debug-build/dist/MinefieldDebug.app/Contents/MacOS/updates/0/update.status
*** Downloader: found existing patch [state=null]
*** Downloader: no patch to download
*** General: Reading Status File: /Users/sspitzer/Desktop/trunk/debug-build/dist/MinefieldDebug.app/Contents/MacOS/updates/0/update.status

for this warning, will attach a new nspr log:

WARNING: server response was unexpected: file /Users/sspitzer/Desktop/trunk/mozilla/netwerk/base/src/nsIncrementalDownload.cpp, line 564

Comment 24

[(not reading, please use [e-mail] instead)]

Attachment: updated log

Comment 25

[(not reading, please use [e-mail] instead)]

Attachment: newer patch, per biesi

Comment 26

[(not reading, please use [e-mail] instead)]

Attachment: screen shot of software update error (post biesi's last suggested fix)

Comment 27

[(not reading, please use [e-mail] instead)]

from the console now:

*** General: Reading Status File: /Users/sspitzer/Desktop/trunk/debug-build/dist/MinefieldDebug.app/Contents/MacOS/updates/0/update.status
*** Downloader: downloadUpdate: Downloading from http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US to /Users/sspitzer/Desktop/trunk/debug-build/dist/MinefieldDebug.app/Contents/MacOS/updates/0/update.mar
*** Downloader: onStartRequest: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US
*** UI:DownloadingPage
*** Downloader.onProgress: onProgress: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, 10347/10347
*** UI:DownloadingPage.onProgress
*** Downloader: onStopRequest: http://download2.mozilla.org/?product=firefox-2.0.0.1-partial-2.0&os=garbage&lang=en-US, status = 0
*** Downloader: onStopRequest: download verification failed
*** General: Transfer Error: The integrity of the update could not be verified (Contact your Administrator), code: verification_failed
*** Downloader: Setting state to: download-failed
*** UI:DownloadingPage
*** Downloader: onStopRequest: Verification of patch failed, downloading complete update
*** General: Reading Status File: /Users/sspitzer/Desktop/trunk/debug-build/dist/MinefieldDebug.app/Contents/MacOS/updates/0/update.status
*** Downloader: found existing patch [state=null]
*** Downloader: no patch to download
*** General: Reading Status File: /Users/sspitzer/Desktop/trunk/debug-build/dist/MinefieldDebug.app/Contents/MacOS/updates/0/update.status

no more warning from nsIncrementalDownloader.cpp, thanks biesi

Comment 28

[(not reading, please use [e-mail] instead)]

Attachment: adding a NS_WARNING, per biesi

Comment 29

[(not reading, please use [e-mail] instead)]

this could happen on the MOZILLA_1_8_0_BRANCH, the MOZILLA_1_8_BRANCH and the trunk.

there is a reproducable test case (see comment #17), but it involves software update.  perhaps we could write a test case purely for the necko unit tests?

Comment 30

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

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

Comment 31

[Jay Patel [:jay]]

Looks like we've made some progress, so let's get those reviews and request for patch approval.

Comment 32

[Darin Fisher]

Comment on attachment 249314 [details] [diff] [review]
adding a NS_WARNING, per biesi

r=darin

It might be nice to move the code that clears the Accept-Encoding header into a helper function.

Comment 33

[(not reading, please use [e-mail] instead)]

>  It might be nice to move the code that clears the Accept-Encoding header into a
helper function.

I'll do that and attach a final patch.  thanks for the review over the holidays, darin.

Comment 34

[(not reading, please use [e-mail] instead)]

Attachment: patch per darin's comment (added a helper function)

Comment 35

[(not reading, please use [e-mail] instead)]

fixed on the trunk.  will seek approval for the branches.

Checking in netwerk/base/src/nsIncrementalDownload.cpp;
/cvsroot/mozilla/netwerk/base/src/nsIncrementalDownload.cpp,v  <--  nsIncrementa
lDownload.cpp
new revision: 1.11; previous revision: 1.10
done

Comment 36

[(not reading, please use [e-mail] instead)]

Comment on attachment 249788 [details] [diff] [review]
patch per darin's comment (added a helper function)

carrying over darin's review.

seeking approval for the branches.

Comment 37

[(not reading, please use [e-mail] instead)]

I created a litmus test case for this issue, see http://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=2900

Comment 38

[Jay Patel [:jay]]

Comment on attachment 249788 [details] [diff] [review]
patch per darin's comment (added a helper function)

Approved for both branches, a=jay for drivers.

Comment 39

[(not reading, please use [e-mail] instead)]

fixed landed on the MOZILLA_1_8_BRANCH and the MOZILLA_1_8_0_BRANCH branches.

Comment 40

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

Comment on attachment 249788 [details] [diff] [review]
patch per darin's comment (added a helper function)

I'd have put the header name into ClearRequestHeader as well (and the comment). That shares a bit more code, and in case it should become necessary to clear further headers, that just requires changing the helper function.

Comment 41

[(not reading, please use [e-mail] instead)]

I'll make christian's suggested improvements to the trunk (but leave the branches alone).

Comment 42

[Mike Schroepfer]

Does disabling the encoding types affect bandwidth usage at all during the updates?  Are updates gzip encoded usually at the server?  Did we test updates both if the server is encoding and not?

Comment 43

[(not reading, please use [e-mail] instead)]

schrep, sorry for the delay.  

> Does disabling the encoding types affect bandwidth usage at all during the
> updates? 

No, I don't think it does.  (see below.)

> Are updates gzip encoded usually at the server?  

No, they aren't.  For example http://ftp-mozilla.netscape.com/pub/mozilla.org/firefox/releases/2.0.0.1/update/win32/en-US/firefox-2.0-2.0.0.1.partial.mar is just "data" (according to "file firefox-2.0-2.0.0.1.partial.mar")

> Did we test updates both if the server is encoding and not?

I did not test that.

here is some info, thanks to the "Live HTTP Headers" add-on.

Before the fix:

GET /?product=firefox-2.0.0.1-partial-2.0&os=win&lang=en-US HTTP/1.1
Host: download.mozilla.org
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5
Accept-Language: en-US,en;q=0.8,he-IL;q=0.7,he;q=0.5,en-us;q=0.3,en;q=0.2
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Connection: keep-alive

HTTP/1.x 302 Found
Date: Thu, 28 Dec 2006 22:51:53 GMT
Server: Apache/2.0.52 (Red Hat)
Set-Cookie: dmo=206.80.94.45.1167346313557458; path=/; expires=Fri, 28-Dec-07 22:51:53 GMT
X-Powered-By: PHP/4.3.9
Location: http://ftp-mozilla.netscape.com/pub/mozilla.org/firefox/releases/2.0.0.1/update/win32/en-US/firefox-2.0-2.0.0.1.partial.mar
Content-Length: 0
Connection: close
Content-Type: text/html; charset=UTF-8
----------------------------------------------------------
http://ftp-mozilla.netscape.com/pub/mozilla.org/firefox/releases/2.0.0.1/update/win32/en-US/firefox-2.0-2.0.0.1.partial.mar

GET /pub/mozilla.org/firefox/releases/2.0.0.1/update/win32/en-US/firefox-2.0-2.0.0.1.partial.mar HTTP/1.1
Host: ftp-mozilla.netscape.com
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061010 Firefox/2.0
Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5
Accept-Language: en-US,en;q=0.8,he-IL;q=0.7,he;q=0.5,en-us;q=0.3,en;q=0.2
Accept-Encoding: gzip,deflate
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Connection: keep-alive

HTTP/1.x 200 OK
Date: Thu, 28 Dec 2006 22:51:53 GMT
Server: Apache
Last-Modified: Thu, 14 Dec 2006 23:50:07 GMT
Etag: "28fc00d-c3e4e-2ccbe9c0"
Accept-Ranges: bytes
Content-Length: 802382
Connection: close
Content-Type: application/octet-stream
----------------------------------------------------------

Note, that we have the "Accept-Encoding: gzip,deflate" header.

after the fix:

GET /pub/mozilla.org/firefox/releases/2.0.0.1/update/win32/en-US/firefox-2.0-2.0.0.1.partial.mar HTTP/1.1
Host: laotzu.acc.umu.se
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20061228 BonEcho/2.0.0.2pre
Accept: text/xml,application/xml,application/xhtml+xml,text/html;q=0.9,text/plain;q=0.8,image/png,*/*;q=0.5
Accept-Language: en-US,en;q=0.8,he-IL;q=0.7,he;q=0.5,en-us;q=0.3,en;q=0.2
Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7
Keep-Alive: 300
Connection: keep-alive

HTTP/1.x 200 OK
Date: Thu, 28 Dec 2006 23:12:38 GMT
Server: Apache/2.2.3 (Unix)
Last-Modified: Thu, 14 Dec 2006 23:50:07 GMT
Etag: "1253817-c3e4e-424992ccbe9c0"
Accept-Ranges: bytes
Content-Length: 802382
Expires: Fri, 29 Dec 2006 22:22:18 GMT
Age: 3019
Keep-Alive: timeout=5, max=1000
Connection: Keep-Alive
Content-Type: application/octet-stream
----------------------------------------------------------
 
Note, that we do not have the "Accept-Encoding: gzip,deflate" header.

In both cases, we are getting 802382 bytes.

Comment 44

[(not reading, please use [e-mail] instead)]

for more about .mar file format, see http://lxr.mozilla.org/seamonkey/source/toolkit/mozapps/update/src/updater/bspatch.cpp
and http://www.daemonology.net/bsdiff/

Comment 45

[(not reading, please use [e-mail] instead)]

discussing it with biesi in irc:  bsdiff and bspatch use bzip2, and so we server it as binary data.  we don't want the server to gzip it again.

Comment 46

[(not reading, please use [e-mail] instead)]

Attachment: code cleanup, per biesi (trunk only)

Comment 47

[(not reading, please use [e-mail] instead)]

supplimental patch landed on the trunk.

Checking in netwerk/base/src/nsIncrementalDownload.cpp;
/cvsroot/mozilla/netwerk/base/src/nsIncrementalDownload.cpp,v  <--  nsIncrementa
lDownload.cpp
new revision: 1.12; previous revision: 1.11
done

Comment 48

[Carsten Book [:Tomcat]]

I can verify fixed this for 1.8.1.2. and 1.8.0.10 for Windows and Linux

Builds: Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.0.10pre) Gecko/20070109 Firefox/1.5.0.10pre ID: 2007010909
Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.2pre) Gecko/20070110 BonEcho/2.0.0.2pre ID:2007011004 on Windows XP x64 and tested also on Fedora FC 6 for 1.8.1.2 and 1.8.0.10 

Maybe someone can also verify this fixed for mac 

Comment 49

[Tony Chung [:tchung]]

need more information on testing this.   This was originally found during live testing so reproducing it on the builds.

Comment 50

[(not reading, please use [e-mail] instead)]

tony, for a test case, see http://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=2900

note, I think juan has started to verify this.

Comment 51

[juan becerra [:juanb]]

Verified according to instructions in comment #17 using: Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.10pre) Gecko/20070112 Firefox/1.5.0.10pre, as well as 2002pre build, and trunk (mac).