525755 - crash [@ nsZipArchive::BuildFileList] using jar: with the file protocol without a '/' for the root of the filesystem

Status: RESOLVED FIXED

(Core :: Networking: JAR, defect)

Description

[Reece H. Dunn]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b2pre) Gecko/20091031 Ubuntu/9.10 (karmic) Namoroka/3.6b2pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2b2pre) Gecko/20091031 Ubuntu/9.10 (karmic) Namoroka/3.6b2pre

Given:
   <p><a href="jar:file:///foo.zip!/default.html">foo</a></td>
   <p><a href="jar:file://bar.zip!/default.html">bar</a></td>
foo does not crash, but bar crashes Firefox (including any other open Firefox windows).

NOTE: Firefox 3.5.3 (Mozilla/5.0 (X11; U; Linux i686; en-GB; rv:1.9.1.3) Gecko/20091020 Ubuntu/9.10 (karmic) Firefox/3.5.3) does not show this behaviour -- it does not crash on the bar link.

Reproducible: Always

Steps to Reproduce:
1. Navigate to jar:file://bar.zip!/default.html
Actual Results:  
Firefox crashes into oblivion!

Expected Results:  
Nothing should happen (same as jar:file:///bar.zip!/default.html).

Comment 1

[Tanner Filip [:tanner]]

Same happens on latest nightly. Confirming. 

03643aed-211c-4ca3-ac20-8d7342091101
There are only 11 of these crashes, now.

Comment 2

[(dormant account)]

Attachment: first stab

Interesting bug. Seems that the url gets parsed into an nsIFile "/" which PR_Read then opens AND PR_Available reports a bogus length on and PR_MemMap fails(instead of PR_CreateFileMap as is normally the case).

I'll add a testcase for this and run it on the tryserver to make sure we handle this correctly on all platforms.

Comment 3

[Reece H. Dunn]

Interesting.

Since you mention nsIFile resolving to "/", I decided to test jar:file:///!/foo.html which also crashes.

Q: Since NS_ERROR_FAILURE is returned in your patch for this, does that result in nothing happening (as per the jar:file:///foo.zip!/default.html behaviour), or does it show a 'file not found' error (as per file:///foo.html)?

In theory this is a separate bug, as 'file not found' should be displayed in this case, in the foo.zip case and in the jar:file:///valid.zip!/unknown.html cases. Should I raise a separate bug for this?

Comment 4

[(dormant account)]

(In reply to comment #3)
> Interesting.
> 
> Since you mention nsIFile resolving to "/", I decided to test
> jar:file:///!/foo.html which also crashes.

Yup.

> 
> Q: Since NS_ERROR_FAILURE is returned in your patch for this, does that result
> in nothing happening (as per the jar:file:///foo.zip!/default.html behaviour),
> or does it show a 'file not found' error (as per file:///foo.html)?
> 
> In theory this is a separate bug, as 'file not found' should be displayed in
> this case, in the foo.zip case and in the jar:file:///valid.zip!/unknown.html
> cases. Should I raise a separate bug for this?

You are right that should be a separate bug.

Comment 5

[Johnny Stenback (:jst)]

Not blocking the release for this, but if this patch gets reviewed and baked in time for the release (like, real soon), we could consider taking this crash fix.

Comment 6

[(dormant account)]

Attachment: patch

Comment 7

[Dão Gottwald [:dao]]

http://hg.mozilla.org/mozilla-central/rev/c0841e95b55d

Comment 8

[(dormant account)]

This is a crashfix, low risk for 192

Comment 9

[Dietrich Ayala (:dietrich)]

Comment on attachment 410109 [details] [diff] [review]
patch

a=me per discussion w/ beltzner et al.

Comment 10

[(dormant account)]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/92475137135c