110880 - URL: "::" after scheme should fail (http::/blah)

Status: NEW

(Core :: DOM: Navigation, defect, P4)

Description

[Darin Fisher]

this is an offshoot of bug 89118

http::/blah is being fixed up as http:///blah, when it should either really be
fixed up as http://blah or simply be rejected as invalid.  i think rejecting it
as invalid makes the most sense.

Comment 1

[Adam Lock]

Cut and paste of my analysis of the problem:
---

This one is a bit fiddly to explain, but here goes...

1. The http channel is delegating parsing to nsStandardURL which uses the auth
parser.  It calls ParserURL on it...

2. The base method nsBaseURLParser::ParseURL gets called. This cuts off the
"http:" part and passes the rest (":/sdfsdf") to nsAuthURLParser::ParseAfterScheme. 

3. The ParseAfterScheme method counts the number of consecutive forward slashes
and assumes that the path part is the offset from the start of the string by
that amount, in this case 1 slash means offset by 1 i.e. it thinks the path bit
of the spec is "/sdfsdf" but it's really "sdfsdf".

4. When the URL is reconstructed into a spec, it adds all the bits together as
scheme + "://" + auth + rest, which in this case is "http" + "://" + "/sdfsdf",
or "http:///sdfsdf"

Therefore the bustage is in ParseAfterScheme for not checking the string starts
 with a forward slash or calculating the offset properly. It shouldn't assume
that the string always starts with a number of consecutive slashes.

Comment 2

[Darin Fisher]

-> badami

Comment 3

[Vinay Badami]

Attachment: error out if first character after scheme is ':'

Comment 4

[Darin Fisher]

Comment on attachment 67897 [details] [diff] [review]
error out if first character after scheme is ':'

sr=darin

Comment 5

[Andreas Otte]

Comment on attachment 67897 [details] [diff] [review]
error out if first character after scheme is ':'

looks good to me, r=[e-mail]

Comment 6

[Vinay Badami]

checkd into tip. 

/cvsroot/mozilla/netwerk/base/src/nsURLParsers.cpp,v  <--  nsURLParsers.cpp
new revision: 1.11; previous revision: 1.10

Comment 7

[benc]

UPDATE: Mozilla RC1 Win32
http::/blah seems to map to the current URL.
What is the error message that should be returned? Typing in to the URL bar says
"resolving host http:"

Also, is this http:///stuff/ -> http://stuff/ syntax supported? In my mind, it
should be illagea as well.

Comment 8

[Andreas Otte]

I will take a look after a short vacation.

Comment 9

[Andreas Otte]

http:///host/path -> http://host/path is an uri fixup that we can do because
http urls must have a host, it seems to logical to assume the user just typed
one / to much.

The problem with the missing error message ... hmmm ... this could be a
regression in the frontend. A short while ago there was a similar error with a
link in a page :http://host/path which caused the same page to be loaded that
was displayed. This was solved in the backend by not propagating an error from
the URLParser that was used to figure out if there is a scheme. 

This time we are in the locationbar and urifixup with resolving www.http.com
seems to kick in. I think if this is an error it belongs to docshell.

Comment 10

[benc]

So, move this to docshell and re-assign to default owners?

Comment 11

[Andreas Otte]

moving to docshell ...

Comment 12

[Juan José Mata]

Setting target milestone to 'Future' since 'mozilla1.0' came and went.

Comment 13

[benc]

Just to be clear about the steps:

We are saying "take the URL and paste it into the URL bar" right? If you
mouseover the URL field label and look at the status, you see:

http://bugzilla.mozilla.org/http::/blah

Comment 14

[Andrew Hagen]

Some URI parsing bugs have been fixed in 1.3. Thus, we are handling this
differently now. This particular issue still exists. If you click on http::/blah
with a current build we are reading that as something to append to the current
URL. If you paste in http::/blah to the URL bar, you are redirected to www.http.com.

Maybe we can just check for two or more consecutive colons when we look for
other broken stuff, like invalid protocols. 

Comment 15

[Darin Fisher]

yeah, it sounds like something regressed...

Comment 16

[Baldvin Kovács]

I came here because I mistyped http://www.gnome.org to http:://www.gnome.org
under firefox, and it took me to http://www.microsoft.com... So could
somebody please explain whats going on with this ::?? :)) Or should I say ::))...

Comment 17

[Darin Fisher]

Comment on attachment 67897 [details] [diff] [review]
error out if first character after scheme is ':'

marking patch as obsolete since it has already been checked in.