96784 - crashes at the very end of downloading this file

Status: VERIFIED FIXED

(Core :: Networking, defect)

Description

[Sean Harding]

Not really sure which component this should go under -- sorry.

Four times in a row (talkback incident IDs TB34454760Z, TB34454870W, TB34454920Z
and TB34454980K), mozilla 0.9.3 on linux has crashed at the very end (99-100%)
of downloading the file at
http://mssjus.www.conxion.com/download/winmediaplayer/wmpmac/7.01/macos/en-us/windowsmediainstaller.hqx

Comment 1

[Asa Dotzler [:asa]]

looks like Networking.

Comment 2

[Tom Everingham]

Mac osX and os9 are fine.  Downloading dialog appears and asks where to save the
file.  No crash and nothing launches after saving.

On Linux and Windows, it crashes like the reporter said.  Stack trace is from
windows version 2001-10-01-05-0.9.4

ntdll.dll + 0x497f (0x77f6497f) 
MSVCRT.dll + 0x1383 (0x78001383) 
MSVCRT.dll + 0x12d7 (0x780012d7) 
js_AllocRawStack
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 343]
js_AllocStack                        
[d:\builds\seamonkey\mozilla\js\src\jsinterp.c, line 374]
nsXPCWrappedJSClass::CallMethod 
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappedjsclass.cpp, line 914]
nsXPCWrappedJS::CallMethod 
[d:\builds\seamonkey\mozilla\js\src\xpconnect\src\xpcwrappedjs.cpp, line 430]
PrepareAndDispatch 
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp,
line 102]
SharedStub 
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcstubs.cpp,
line 124]
nsDocLoaderImpl::FireOnStatusChange
[d:\builds\seamonkey\mozilla\uriloader\base\nsDocLoader.cpp, line 1165]
nsDocLoaderImpl::OnStatus 
[d:\builds\seamonkey\mozilla\uriloader\base\nsDocLoader.cpp, line 961]
nsHttpChannel::OnStatus 
[d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpChannel.cpp, line 2269]
XPTC_InvokeByIndex 
[d:\builds\seamonkey\mozilla\xpcom\reflect\xptcall\src\md\win32\xptcinvoke.cpp,
line 139]
EventHandler 
[d:\builds\seamonkey\mozilla\xpcom\proxy\src\nsProxyEvent.cpp, line 510]
PL_HandleEvent 
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 591]
PL_ProcessPendingEvents 
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 524]
_md_EventReceiverProc 
[d:\builds\seamonkey\mozilla\xpcom\threads\plevent.c, line 1072]

Comment 3

[Tom Everingham]

tried downloading files with bogus extensions on local http server.  Not able to
reproduce with that test.

Comment 4

[sairuh (rarely reading bugmail)]

hiya tever --is this perchance a problem with trunk bits as well, or is it
limited to the branch?

Comment 5

[neeti]

darin

Comment 6

[Darin Fisher]

Attachment: stack trace : linux

Comment 7

[Darin Fisher]

Attachment: stack trace : win2k

Comment 8

[Darin Fisher]

the linux and win2k stack traces are very different... looks like something
maybe trashing memory... but what?

Comment 9

[Darin Fisher]

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

Comment 10

[Darin Fisher]

bug 102843 had a stack trace involving the nsBinHexDecoder... i took a closer
look and it appears that this decoder is only included in the non-Mac mozilla
builds.
... probably because the Mac can handle this format natively.  so, my guess
would be that there is something screwy with the nsBinHexDecoder... it is
probably scribbling over some memory that it doesn't own and thus causing all of
these very different looking stack traces.

Comment 11

[Darin Fisher]

OK.. removing nsBinHexDecoder from the build eliminates this crash.

-> mscott (since it looks like nsBinHexDecoder is his baby)

Comment 12

[sairuh (rarely reading bugmail)]

fwiw, while i do see the problem on linux [2001.10.02.04-branch comm] with the
test url, i cannot seem to repro it on just any .hqx file.

eg, the following didn't result in a crash:

http://the.ausmac.net/ftp/Communication-SW/mt-newswatcher-31.hqx - saved the
file successfully.

ftp://the.ausmac.net/pub/mac/Communication-SW/mt-newswatcher-31.hqx - displayed
the file [as text] in the browser, but no crash.

Comment 13

[Simon Fraser [no longer active]]

I don't see why we couldn't include the BinHex decoder in Mac builds; IE does its 
own BinHex decoding.

Comment 14

[Scott MacGregor]

To answer simon's question: the bin hex decoder isn't included in the Mac
because for it to work on the Mac it needs access to the file we are writing
too. i.e. to set the data fork and the resource fork coming out of the bin hex
decoder. This required some major architectural changes to the way stream
converters work. Right now they are just stream listeners and have no knowledge
about things like a file on disk. So the bin hex stream converter was written so
we had a solution for windows and linux to decode mac bin hex content. Mac
applications like stuffit handle mac bin hex so it wasn't as critical to try to
change how stream converters work to accomodate the mac.

Comment 15

[Darin Fisher]

ok.. so my hunch here is that the binhexdecoder may be sensitive to the content
length, which would explain why some documents cause problems and others don't.

Comment 16

[Scott MacGregor]

is it possible the content length isn't accurate? i.e. not really reflecting the
true length of the incooming data? I doubt it 'cause then lots of other
converters would be having problems. so it must be something inside the bin hex
decoder.

Comment 17

[jon]

ok, i set up 2 test hqx files, one big (11 megs) and one small (400 bytes). 

http://divisionbyzero.com/foo/

the browser doesn't crash on either of them. i then checked to see what mimetype
the server was returning:

[jon@devotchka jon]$ telnet divisionbyzero.com 80
Trying 64.81.68.238...
Connected to divisionbyzero.com.
Escape character is '^]'.
GET /foo/random.hqx HTTP/1.1
Host: www.divisionbyzero.com

HTTP/1.1 200 OK
Date: Wed, 03 Oct 2001 20:10:27 GMT
Server: Apache/1.3.19 (Unix)  (Red-Hat/Linux) mod_ssl/2.8.1 OpenSSL/0.9.6
DAV/1.0.2 PHP/4.0.4pl1 mod_perl/1.24_01
Last-Modified: Wed, 03 Oct 2001 20:06:02 GMT
ETag: "b7a48-b972d2-3bbb6faa"
Accept-Ranges: bytes
Content-Length: 12153554
Content-Type: application/mac-binhex40

and compared that to the original url given:

[jon@devotchka jon]$ telnet mssjus.www.conxion.com 80
Trying 206.204.7.100...
Connected to mssjus.www.conxion.com.
Escape character is '^]'.
GET /download/winmediaplayer/wmpmac/7.01/macos/en-us/windowsmediainstaller.hqx
HTTP/1.1
Host: mssjus.www.conxion.com

HTTP/1.1 200 OK
Server: Microsoft-IIS/5.0
Date: Wed, 03 Oct 2001 19:07:17 GMT
Content-Type: application/mac-binhex40
Accept-Ranges: bytes
Last-Modified: Fri, 29 Jun 2001 20:51:59 GMT
ETag: "80c1f856dd0c11:87b"
Content-Length: 4810252

The content length and mime type were correct from both servers; the only
difference i can see is IIS vs apache. i checked the other links supplied that
didn't cause a crash; they were from apache servers too. can anyone with iis
server access verify this?

Comment 18

[Darin Fisher]

notice that in the crash case, the content-length (=4810252) is a multiple of 4,
but in the non-crash case, the content-length (=12153554) is not a multiple of 4.

i wonder if this is significant.

Comment 19

[Simon Fraser [no longer active]]

BinHex is a tricky format to decode. I would not be surprised if there were bugs 
in our decoder.

Comment 20

[Scott MacGregor]

I had actually just ported over the 4.x bin hex decoder as opposed to writing
one from scratch. It's possible I made a mistake when porting....but I didn't
introduce the complexity of writing a new bin hex decoder.

Comment 21

[Tom Everingham]

another bit of info.  I downloaded the original .hqx file using 4.7 and stored
it on an internal WinNT server.   Using the .0.9.4 client, I did not see the
crash on windows or linux but I cannot download the file.  All that happens is a
progress bar shows activity and I never see a 'save file' dialog.  

Once again, Mac works fine when hitting the winNT server.

Comment 22

[Darin Fisher]

tever: it's possible that IIS needs to be configured to send the
application/binhex40 Content-Type.  can you verify that it is being
sent?

Comment 23

[Tom Everingham]

Actually, that was Netscape-Enterprise server I checked.  Looks like the correct
content type is there.  Still only seeing busy bar activity and no save dialog.  

HTTP/1.1 200 OK
Server: Netscape-Enterprise/3.0
Date: Thu, 04 Oct 2001 12:29:05 GMT
Content-type: application/mac-binhex40
Etag: "0-49660c-3bbc556d"
Last-modified: Thu, 04 Oct 2001 12:26:21 GMT
Content-length: 4810252
Accept-ranges: bytes
Connection: keep-alive....

Comment 24

[Tom Everingham]

taking jon's advice I saved the original file on an internal IIS server and can
reproduce the crash.  link is http://dogspit/tever/windowsmediainstaller.hqx

  10/4/01  1:43 PM      4810252 windowsmediainstaller.hqx
  
Content-type and Content-Length both look correct. 

HTTP/1.1 200 OK
Server: Microsoft-IIS/4.0
Date: Thu, 04 Oct 2001 22:20:42 GMT
Content-Type: application/mac-binhex40
Accept-Ranges: bytes
Last-Modified: Thu, 04 Oct 2001 20:43:26 GMT
ETag: "18f8d337154dc11:b33"
Content-Length: 4810252
(This file must be converted with BinHex
4.0).:'9GTEQ4[Gh00C@4TB8PZFh4KE'aPFLjcC@%!39"36'&eFh3J!!!dDS%!!C     etc.

Comment 25

[jon]

tever, can you verify that iis is correctly sending the file (ie, md5 sums
match, etc)?

Comment 26

[Tom Everingham]

I am fairly certain the file is being correctly sent because I can download and
install winmediaplayer on my mac using ie5.

Comment 27

[Scott MacGregor]

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

Comment 28

[Scott MacGregor]

Attachment: fixes the crash

Comment 29

[Scott MacGregor]

There were 2 problems in here:
1) In the case of 103740, the content type of the data fork was still
application/macbinhex40. That's because the file name didn't map to an extension
we knew about (foo.a3 in this case). So we fell back to using the file name in
the url for determing the content type in the decoder. Of course this caused a
nice infinite recursion with the stream decoder code. The fix is to force a
content type of unknown if our guessing code for binhex comes back with binhex
again after starting to parse the binhex content.

2) There was also a small problem where we were trying to process the checksum
on the resource fork. The windows/linux bin hex decoder throws away the resource
fork and doesn't use it. So we don't want to be trying to process the checksum
for the resource fork.

Together, these 2 changes fix the crash.

However, for those of you trying to save a binhex file on windows where the data
fork is some mac binary code you aren't going to be happy. You can't then move
the saved file to the Mac and have it work because the resource fork is thrown
away. The decoder just extracts out the data fork so we can try to present it
internally or give it to a helper app if the content type of the data is
something we can understand. For instance eudora frequently sends messages &
attachments in mac binhex. This decoder allows you to read and open these
messages / attachments on windows and linux by stripping out the resource fork.

I'll see if I can scrounge up an r and an sr. We may choose to take this on the
094 branch.

Comment 30

[Darin Fisher]

Comment on attachment 52760 [details] [diff] [review]
fixes the crash

this patch looks good to me -- sr=darin

Comment 31

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

Comment on attachment 52760 [details] [diff] [review]
fixes the crash

r=sspitzer

Comment 32

[Jaime Rodriguez, Jr.]

pls check this into the branch - PDT+

Comment 33

[Scott MacGregor]

fixed on the 094 branch and the trunk.

Comment 34

[Tom Everingham]

verified on branch:   
Linux rh6 and WinNT4 20011011 builds

Comment 35

[jon]

ack. still crashes for me on rh7 with the 20001101202 0.9.5 build. did the
changes go into this branch as well?

Comment 36

[Tom Everingham]

yes this doesn't appear fixed in 0.9.5.  I'm not sure about the reasoning for
not checking everything into this branch.  

It is fixed on both the 094 branch and trunk so it will be carried over to 0.9.6.  

marking verified since this is also working on trunk builds

20011011 tr

Comment 37

[Boris Zbarsky [:bzbarsky]]

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