Last Comment Bug 407303 - Getting "Unsafe File Type" error when accessing a site that does not exist.
: Getting "Unsafe File Type" error when accessing a site that does not exist.
Status: RESOLVED FIXED
: fixed1.8.1.12, regression
Product: Core
Classification: Components
Component: Document Navigation (show other bugs)
: Trunk
: All All
: P2 normal (vote)
: ---
Assigned To: Dave Camp (:dcamp)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: jarxss
  Show dependency treegraph
 
Reported: 2007-12-06 23:24 PST by Prasad Sunkari [:prasad]
Modified: 2008-01-31 05:01 PST (History)
14 users (show)
mtschrep: blocking1.9+
dveditz: blocking1.8.1.12+
dveditz: wanted1.8.1.x+
jwalden+bmo: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
v1 (2.79 KB, patch)
2007-12-10 09:44 PST, Dave Camp (:dcamp)
bzbarsky: review+
bzbarsky: superreview+
dveditz: approval1.8.1.12+
Details | Diff | Splinter Review
test case (1.28 KB, patch)
2007-12-10 10:46 PST, Dave Camp (:dcamp)
bzbarsky: review+
bzbarsky: superreview+
Details | Diff | Splinter Review

Description Prasad Sunkari [:prasad] 2007-12-06 23:24:14 PST
When trying to access "jar:http://foo.bar/test.jar!/abc.html, foo.bar is a domain that does not exist and that should be the message shown to the user.  Instead it shows a error saying "Unsafe file type"
Comment 1 Boris Zbarsky [:bz] (still a bit busy) 2007-12-07 22:51:40 PST
Yikes.  We should be checking for an error status in OnDownloadComplete.  Right now we're clobbering error status values.

In particular, we probably shouldn't be looking at some of the data we look at if the status is an error.  It's not guaranteed to have sane values.
Comment 2 Dave Camp (:dcamp) 2007-12-10 09:44:35 PST
Created attachment 292431 [details] [diff] [review]
v1

... and in the case of a redirected jar the status was actually being incorrectly overwritten.

This patch handles redirects outside of NS_SUCCEEDED(status), moves security info fetch and content type check into an NS_SUCCEEDED(status), fixes the status clobbering in redirect, and removes a testing comment.
Comment 3 Boris Zbarsky [:bz] (still a bit busy) 2007-12-10 10:07:04 PST
Comment on attachment 292431 [details] [diff] [review]
v1

Looks great.  Can we add a test somehow?
Comment 4 Dave Camp (:dcamp) 2007-12-10 10:46:22 PST
Created attachment 292445 [details] [diff] [review]
test case

unit testing the redirect case will be a bit tougher, but this tests the simple case.
Comment 5 Mike Shaver (:shaver -- probably not reading bugmail closely) 2007-12-12 03:25:09 PST
I'm wondering if this would also cause netError.xhtml to complain about missing entities, which my wife just started seeing on 2.0.0.11 (not to say that it wasn't there in previous versions, like 2.0.0.10, just that she's on .11 now).  I know that netError runs without privs, and I suspect that there are some redirection-like tricks involved in getting it displayed.  I'm not sure what sort of error she would have been seeing, but she got an XML error about &generic.longDesc not being defined instead.
Comment 6 Myk Melez [:myk] [@mykmelez] 2007-12-12 04:52:30 PST
I don't know all the details, but the "redirection-like trick," as I understand it, is that nsDocShell::DisplayLoadError calls nsDocShell::LoadErrorPage to construct an about:neterror URL that it passes to nsDocShell::InternalLoad, which loads the URL in a way that does not cause the URL bar to change or history to get updated (perhaps via nsIDocShellLoadInfo::loadBypassHistory?).

According to nsAboutRedirector.cpp, about:neterror is chrome://global/content/netError.xhtml , which references chrome://global/locale/netError.dtd , which for Firefox is http://lxr.mozilla.org/mozilla/source/browser/locales/en-US/chrome/overrides/netError.dtd , which does define generic.longDesc, so it's unclear why the app would complain about that entity not being defined.
Comment 7 Dave Camp (:dcamp) 2007-12-13 14:35:39 PST
Checking in nsJARChannel.cpp;
/cvsroot/mozilla/modules/libjar/nsJARChannel.cpp,v  <--  nsJARChannel.cpp
new revision: 1.128; previous revision: 1.127
done
RCS file: /cvsroot/mozilla/modules/libjar/test/unit/test_bug407303.js,v
done
Checking in test/unit/test_bug407303.js;
/cvsroot/mozilla/modules/libjar/test/unit/test_bug407303.js,v  <--  test_bug407303.js
initial revision: 1.1
done
Comment 8 Dave Camp (:dcamp) 2007-12-13 14:47:09 PST
(In reply to comment #5)
> I'm wondering if this would also cause netError.xhtml to complain about missing
> entities, which my wife just started seeing on 2.0.0.11 (not to say that it
> wasn't there in previous versions, like 2.0.0.10, just that she's on .11 now). 
> I know that netError runs without privs, and I suspect that there are some
> redirection-like tricks involved in getting it displayed.  I'm not sure what
> sort of error she would have been seeing, but she got an XML error about
> &generic.longDesc not being defined instead.

This bug would have caused an "Unsafe Content Type" error, but probably wouldn't cause unknown entity errors in netError.

I opened bug 408267 about the unknown entity error.

Comment 9 Jeff Walden [:Waldo] (remove +bmo to email) 2007-12-13 22:28:45 PST
The redirect case still needs to be tested, although the non-existent-site case got hit in the committed test.
Comment 10 Daniel Veditz [:dveditz] 2007-12-17 16:24:22 PST
Comment on attachment 292431 [details] [diff] [review]
v1

approved for 1.8.1.12, a=dveditz for release-drivers
Comment 11 Dave Camp (:dcamp) 2007-12-18 12:46:09 PST
Checking in nsJARChannel.cpp;
/cvsroot/mozilla/modules/libjar/nsJARChannel.cpp,v  <--  nsJARChannel.cpp
new revision: 1.116.2.4; previous revision: 1.116.2.3
done
Comment 12 Al Billings [:abillings] 2008-01-29 16:38:27 PST
In 2.0.0.11, I get an error page stating:

The address isn't valid
The URL is not valid and cannot be loaded.


if I try to load jar:http://foo.bar/test.jar!/abc.html.

Do we have a testcase for verification that I can use in branch? The attached test file is for a unit test.

Note You need to log in before you can comment on or make changes to this bug.