18148 - missing slash in file: URL parses incorrectly

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[Warren Harris]

If you pass the following string to nsStdURL:
  file://y|mozilla/makefile.win
rather than:
  file://y|/mozilla/makefile.win

nsStdURL parses the hostname as "y|mozilla" and makes the path be
"/makefile.win" whereas it should treat the entire string
"y|mozilla/makefile.win" as the path.

Comment 1

[Andreas Otte]

We really need to agree on what to do with this wrong URIs:

1. Make Parse() in nsStdURL.cpp better (needs protocol specific code)

2. Check in the File-Protocol-Handler if there is a host after parsing, and when
there is, give a MALFORMED_URI error

Comment 2

[Warren Harris]

This URL parsing stuff has been so error prone (I count 29 (!) bugs so far on
this stuff). Maybe the right solution here is to split out nsStdURL into two
classes: nsFileURL and nsNetURL. Then they can have protocol-specific parsing
rules.

Comment 3

[Gagan]

I totally agree that we should pull file specific parsing out into its own.

Comment 4

[Andreas Otte]

I think there a two classes here: URIs with host and without

Comment 5

[leger]

Moving Assignee from gagan to warren since he is away.

Comment 6

[Andreas Otte]

I tried this and I have now three URL classes nsStdURL (like before, but with a
rewritten Parser), nsHostURL (when you are sure you have a URL with a host) and
nsNoHostURL (when there is no host in the URL).

The ProtocolHandler (http, ftp) are using nsHostURL, file is using nsNoHostURL.
This way it is possible to parse certain malformed URLs right according to the
requested protocol.

Comment 7

[Warren Harris]

I think we determined that the bug here was using 2 slashes after file: instead
of 3. 3 works.

Comment 8

[leger]

Bulk move of all Necko (to be deleted component) bugs to new Networking

component.

Comment 9

[Norris Boyd]

I got bit by this bug when I migrated profiles and my 4.x home page (which is a
local file) would no longer display since it was saved as file://e|/...

Comment 10

[Andreas Otte]

assigning this bug to me per Warrens request

Comment 11

[Warren Harris]

Andreas: This was supposed to be fixed by your changes that I just checked in,
but when I tried to fetch the file URL, the browser started spinning in the
parser, steadly eating up all of available memory. Looks like one for harishd,
perhaps, but you should verify.

Comment 12

[Warren Harris]

P.S. If you see this problem too, we probably should open a new bug on that.

Comment 13

[Andreas Otte]

This will not make it into M13

Comment 14

[John Morrison]

I also noticed the same crash as warren with 2000011608 win95, but for URL like
file:///c  or file:///c|  or file:///foobar

(The last one is what you get if you download a file from the net that uses
absolute paths for HREF, and you open the file from the disk in the browser).

Comment 15

[leger]

Putting on PDT+ radar for beta1.

Comment 16

[Andreas Otte]

This one should be fixed now. Two slashes are expanded to three, I see no crash.

Comment 17

[Tom Everingham]

verified:
Linux 2000020708