38643 - file:///UNC and file:////UNC should not -> file://///UNC

Status: VERIFIED FIXED

(Core :: Networking: File, defect, P3)

Description

[Jesse Ruderman]

Steps to reproduce:  (about 60% reproducability)
1. Save your work so you don't panic if your OS freezes
2. Save URL or attachment to your hard drive
3. Load local copy of attachment file in mozilla
4. Leave the page (click home icon, close mozilla, etc.)

Notes: 

- other apps become unresponsive as well, and ctrl-alt-del takes more than a
  second to show up

- adding more bogus drive names increases crash length, but adding more
  filenames with the same drive name image files doesn't (not sure about this)

- /foo/bar.gif works as well as file:///foo/bar.gif
  (this is why it showed up on local copies of legit pages)

- freeze is most reproducable while leaving page, but can happen if page is
  open and you're working in another application

Comment 1

[Jesse Ruderman]

Attachment: test case

Comment 2

[Jesse Ruderman]

Marking as a crasher and raising severity because with many different bogus 
drive names, this freezes the OS for quite a while.

Comment 3

[Gagan]

->warren

Comment 4

[Warren Harris]

I just use nsIFile.  => dougt

Comment 5

[Doug Turner (:dougt)]

This is a known windows 98 system problem.  I belive that Microsoft has patches 
via WindowsUpdate for this. 

Comment 6

[Jesse Ruderman]

This may be known, and it is probably a bug, but I don't see any patches for 
it.  This bug is not bug 29079, which involved device driver names, whereas 
this one involves bogus drive names.

Comment 7

[Doug Turner (:dougt)]

pam, do you know of any problems loading images from a bad file url on windows 
98?

Comment 8

[pnunn]

DT:

nope. Imglib doesn't care about files. It cares about data streams.
We use file specs for id'ing images, but we deal with data streams.
-P

Comment 9

[Doug Turner (:dougt)]

something must have changed.  works for me.

Comment 10

[Tom Everingham]

verified WORKSFORME
Win98 2000070508

Comment 11

[Jesse Ruderman]

now freezes mozilla but not windows 98.  reopening.

Comment 12

[Jesse Ruderman]

oh, it does sometimes still freeze windows 98.

Comment 13

[Doug Turner (:dougt)]

Still can't reproduce.  QA, can you try/verify this again on a couple different
machines?

Comment 14

[Tom Everingham]

jesse, I'm not seeing any attachment in your 05-08 23:41 comment.  Was this 
basically the URL you saved locally as an html file?

Comment 15

[Jesse Ruderman]

My comment really wasn't clear; thanks for pointing that out.  I was referring 
to what happens when I load a file that looks like

<img SRC="/foo1/bar" alt="1">
<img SRC="/foo2/bar" alt="2">
<img SRC="/foo3/bar" alt="3">
<img SRC="/foo4/bar" alt="4">
<img SRC="/foo5/bar" alt="5">
<img SRC="/foo6/bar" alt="6">
<img SRC="/foo7/bar" alt="7">

and load it in the browser.  (Hint: try it with fewer than 7 first.)

Note that

<img SRC="/foo1/bar1" alt="1">
<img SRC="/foo1/bar2" alt="2">
<img SRC="/foo1/bar3" alt="3">
<img SRC="/foo1/bar4" alt="4">
<img SRC="/foo1/bar5" alt="5">
<img SRC="/foo1/bar6%22 alt="6">
<img SRC="/foo1/bar7" alt="7">

is much less problematic.

Comment 16

[Tom Everingham]

ok, the way I understand this is that you are giving two examples of a lockup 
problem.  The first example uses a copy of the zdnet page stored locally, and 
the second example uses an html test file with the code listed in your last 
comment (with 7 img tags and the alt attribute).  

If that is correct I am still not reproducing this.  Could you possibly 
re-attach your test case?  thx

Comment 17

[Tom Everingham]

I typed that last comment to quickly.  What I can see is a lockup (on win 98) 
when I load a copy of the page from a local drive.  It is ok on win NT.  But I 
cannot narrow it down to the bogus directory using the html you provided.

Comment 18

[Tom Everingham]

Doug, copies of the files I am using are at 
  http://bubblegum/tever/bogus.html
  http://bubblegum/tever/bigpage.html
but you need to copy them locally.

Comment 19

[Tom Everingham]

Added 40 bogus directories to test case per Jesse's recomendation.  Still 
works on win98 for me.  

I do however see a slight pause when the page is loading.  Is it possible you 
are seeing this?   What is the speed / memory of the system you are you using?   

Comment 20

[Doug Turner (:dougt)]

reopening based on comments..

Comment 21

[Tom Everingham]

Attachment: test case

Comment 22

[Jesse Ruderman]

Attachment: Many bogus drives instead of many files on a bogus drive

Comment 23

[Jesse Ruderman]

I think I figured it out: Mozilla is interpreting file:///foo1/bar/ as 
\\foo1\bar\, meaning "ask computer foo1 on your local network for a directory 
called bar".  Windows 98 is very slow about network things (try start-r, 
\\foo115\bar) but apparently only freezes if asked to try to access several 
computers on the local network at once.

IE and Mozilla will both accept file://///watership/mp3/ typed into the 
location bar.  Mozilla will also accept file:///watership/mp3/ as an equivalent 
url for the url above, and will replace it with the 5-slash version if the 3-
slash version is typed into the url bar.  This should only happen when the url 
is typed by the user (see bug 34943 for a similar problem).

So we actually have two problems here:
- url error-corrrection happening when it shouldn't
- a bad crash, that afaict can't be exploited remotely

Netscape 4.75 doesn't accept file:///// urls.

Comment 24

[Jesse Ruderman]

Re-summarizing.

Comment 25

[Andreas Otte]

This file url correction only happens on XP_PC if the stuff after file:/// can
not be identified as a syntactically valid drivename. It is a best guess on what
the user want's and sometimes that's false. This conversion is currently deeply
burried into the urlparser, no way of getting it out there without changing the
whole process of creating fixup URIs, as suggested many times before.

Comment 26

[Jesse Ruderman]

Oops, this crash is remotely exploitable.

<a href="data:text/html,<script>for (i = 0; i < 4; ++i) document.write('<img 
src=file://///bogusdrive' + i + '/foo.jpg alt=' + i + '>');</script>" 
target="_blank">click here to freeze moz for a while</a>

Comment 27

[lchiang]

any opinion on this, mstoltz?  Thanks.

Comment 28

[Doug Turner (:dougt)]

over to you mitch per our conversation.

Comment 29

[Keyser Sose]

All of these testcase now work for me can anyone else verify that its now ok?

Platform: PC
OS: Windows 98
Mozilla Build: 2001022620

Comment 30

[Mitchell Stoltz (not reading bugmail)]

Setting milestone to Moz0.9.

Comment 31

[Chase Tingley]

Using 2001041104 on Win98, all of the test cases seem to work.

When loading a local file that contains:

<img SRC="/foo1/bar" alt="1">
<img SRC="/foo2/bar" alt="2">
<!-- ... -->
<img SRC="/foo9/bar" alt="9">

mozilla "thinks" for several seconds (5-10) but does not freeze.

Comment 32

[Mitchell Stoltz (not reading bugmail)]

Adding CheckLoadURI to IMG tags will keep this from being remotely exploitable,
which IMHO is acceptable. This is supposed to happen by 0.9. After that, if
anyone wants to address this crash/slowdown, that's cool, if not, that aspect of
the bug is a WONTFIX.

Comment 33

[Mitchell Stoltz (not reading bugmail)]

OK, my part is done. It should be generally impossible to reference a file: URL
of any kind from Web content, at least it will be win Pavlov gets his fix in.
Meantime, I'm moving this over to Necko and pushing out the milestone to see if
anyone over there thinks the slowdown caused by asking Windows to search for a
network drive is worth treating as a denial-of-service issue.

Comment 34

[Gagan]

this is an annoying problem and should get fixed for the beta. giving to doug 
since he has been working on file transport related issues. 

Comment 35

[Doug Turner (:dougt)]

RFC 1738 states that the syntax of file:// URL is:

file://<host>/<path>

so, if there is a computer that is accessible via UNC named foo, the URL to that
computer is file://foo/.  Is the suggestion that the host name of a UNC mount in
a URL is always prepended by "//" or "///"?  

Comment 36

[Andreas Otte]

The host in the file-url has nothing to do the host in an UNC filepath. As the
name suggests, all of UNC is happening inside the path component of the URL. All
the urlparser can do is guess and add the appropiate number of slashes as
necessary. So to really be an UNC filepath it has to be file://///foo to work on
every OS. Two slashes for the protocol, one to delimit the non existing host and
then two to start the UNC filepath.

The file protocol is by definition a local protocol, even if the syntax might
suggest otherwise. The only possible host can be localhost, nothing else makes
really sense because there is no generic way to access a file a on remote
computer.

The problem during parsing is to distinguish a hostname from a directory. This
is easy on XP_PC where drives can be easily detected at the beginning of a path
and eyerything else *could* be a network name. Currently the urlparser doeas
just that. On unix/linux there is no way to syntactically distinguish a hostname
from a directory, so there is much less fixup possible.

Comment 37

[Doug Turner (:dougt)]

As near as I can tell, the bug is that we assue that if a file url does not have 
a drive specifier, it is a UNC path.  This, however, is incorrect.  We should be 
defaulting to the drive where the application is running from.

The code that check for UNC paths is here:

http://lxr.mozilla.org/seamonkey/source/netwerk/base/src/nsNoAuthURLParser.cpp#2
72

So, I think one way of handling this is for the check in nsNoAuthURLParser.cpp 
to go away.  Then in nsStdUrl::GetFile(), where we convert the url path to 
native path, we have some logic to determine if it is a UNC path or if it is a 
path missing a drive spec.

I will attach a patch which addresses this.  I tested the path against 
the testcases and it performs well.  I also loaded file files and such, but I am 
not sure I am covering all bases.  

Andreas, could you tell me what I am missing?  

Comment 38

[Doug Turner (:dougt)]

Attachment: fix for aggressively accessing network volumes

Comment 39

[Andreas Otte]

As far as I know there is no defaulting to the current drive with file urls. A
file url is either absolute and contains a full path (which includes a drive on
some OS) or is relative and maybe contains no drive but the base url has to (on
some OS).

But if this url guessing is to agressive, we should try and switch it off/apply
the patch. I currently have no working win32 build (totally rotten away), so I
can't check against my modified urltest-version. tever has the diffs. Maybe
someone else has time to test the patch.

Comment 40

[Doug Turner (:dougt)]

gagan, darin, please review.  

Comment 41

[Gagan]

r=gagan

Comment 42

[Darin Fisher]

sr=darin (but make sure you get enough testing before landing this)

Comment 43

[Doug Turner (:dougt)]

Fix checked in.

Checking in nsNoAuthURLParser.cpp;
/cvsroot/mozilla/netwerk/base/src/nsNoAuthURLParser.cpp,v  <--  
nsNoAuthURLParser.cpp
new revision: 1.13; previous revision: 1.12
done
Checking in nsStdURL.cpp;
/cvsroot/mozilla/netwerk/base/src/nsStdURL.cpp,v  <--  nsStdURL.cpp
new revision: 1.72; previous revision: 1.71
done

Comment 44

[benc]

-> file

So, is the fix that we do not take file:///<path> as UNC, and require file:///// 
paths now?

Comment 45

[Andreas Otte]

Yes, we no longer stuff the url with / when we can't detect a drivename in file
urls and it is not a UNC-PATH. Instead we let it default to the current(?)
drive. This may be wrong as it is to always assume UNC-Paths if no drive is
present. But I think this may be the best compromise. Time will tell ....

Comment 46

[benc]

Thanks for the clairification.

File URL's will be getting more attention in the next milestone release notes.

We'll find out one way or the other...

Comment 47

[benc]

VERIFIED:
Win98, Mozilla 0.9.4.

STEPS:
1- access file:///UNC
Viewed file.
2- attempted to access shorter file URLs by progressively removing slashes

file:///UNC and file:///UNC are non-existent files and treated as such. 
(correct)
file://UNC -> file://servername/path (correct)
file:/UNC -> file:///UNC, which is non-existent. (correct)

Comment 48

[benc]

I can't count...
file://///UNC (5) works.

"file:///UNC and file:///UNC" should have been:
file:///UNC (3) and file:////UNC (4)"

Fixed summary too.