122270 - URL: Remove all conversion from \ to / on windows in nsDefaultURIFixup except when a real filepath is identified

Status: NEW

(Core :: DOM: Navigation, defect)

Description

[Andreas Otte]

This is a spinoff from bug 121839. Following is a example of the inconsistencies
when doing conversion of \ to /:

If I go directly to the image
  http://www.student.utwente.nl/~hardboard/pictures\bordzager.jpg
then the URL is replaced by 
  http://www.student.utwente.nl/~hardboard/pictures/bordzager.jpg
and the image is displayed.

The original page
  http://www.student.utwente.nl/~hardboard/evenementenfotos.html
however does not display the image. It doesn't even display a broken image icon.

I think we should be consistent with conversion from \ to /. I think there is
only one reasonable solution in the light of the discussion on bug 32895: Remove
all conversion from \ to / on windows in nsDefaultURIFixup except when a real
filepath is identified.

Comment 1

[Andreas Otte]

As a sidenote: Doing a conversion that replaces all \ with / in http, https and
ftp urls as it is currently done could cause some damage since \ is also
converted to / outside the directory part of the url. 

Comment 2

[Adam Lock]

Sorry I don't understand what you're asking here. Default URI fixup only occurs
when you type in the URL in the bar. It's an object designed to make URI typing
more convenient and fixup a few common mistakes such as changing back to forward
slashes.

URI fixup does not happen when you click on a link in content. It is expected
that the site designer has their slashes the right way around and if they don't
I suppose something in Mozilla is rejecting the URI as malformed and doing nothing.

Comment 3

[Andreas Otte]

What I'm saying here is that it should not happen on the urlbar to the extent it
happens today. For example (can't check at the moment since I'm on linux) if you
*type* this into the urlbar it should sent you to the noncompliance page:

http://www.undergrad.math.uwaterloo.ca/~dj3vande/rfc\compliance.html

It is also very bad to simply replace all \ with /, since the query part of a
url typed into the urlbar might contain a \ and it should stay the way it is.

The point is: This fixup sometimes does the wrong thing. I can't simply reach
files with names that contain a \ in them on a http server from the client XP_PC
platform. I can do so from linux. That does not make sense. What has the client
platform to do with the http server? This fixup only makes sense locally, when
patching up a local filepath as fileurl (FileURIFixup).
 

Comment 4

[Adam Lock]

CC'ing Darin.

I believe fixup is doing the right thing, correcting what people meant to type. 

Perhaps URI parsing is incorrect though in assuming a backslash is the same as a
forward slash and should escape the character.

In any case, a backslash is an illegal character in a URI (see rfc2396) and must
be escaped. So which is right - rfc2068 dealing with the HTTP protocol (i.e.
transport) or rfc2396 which deals with URIs in general? It is worth noting that
rfc1738 which 2068 refers to for it's definition of an URL is obseleted by rfc2396. 

If you need to embed a backslash then you need to escape it - e.g.

http://www.undergrad.math.uwaterloo.ca/~dj3vande/rfc%5ccompliance.html

Comment 5

[Darin Fisher]

URI parsing does escape '\'

i think that is why andreas reported an inconsistency.  in other words,
mozilla's URL bar being more forgiving results in some inconsistency since the
lower level URL parsing (which sees all URLs processed by mozilla) follows
RFC2396 more rigorously w.r.t. '\' and '/'

i tend to think that this inconsistency isn't so bad.  i kind of like the fact
that the URL bar helps the user... afterall, someone familiar with Windows is
very likely to use '\' instead of '/' ...or to at least get confused about which
is correct.  why not help such users?

of course, website designers should be evangelized if they use '\' when they
really mean '/' and they would never be caught if didn't enforce RFC2396 at the
URL parser level.

i think solving this bug would not violate the mantra of trying to help users.
it's important to distinguish fixup in the filepath to fixup in other parts of
the URI string.  for example, it is perfectly legal to use \ as part of a
password embedded in an URI.

Comment 6

[Andreas Otte]

Attachment: patch to fix the inconsistency

I think this is an easy fix, no need to future it. However it might be
interesting  to run with the patch applied on a windows build for some time and
see if any similar fixup happens anywhere else.

Comment 7

[Andreas Otte]

Attachment: patch to fix the inconsistency

I think this is an easy fix, no need to future it. However it might be
interesting  to run with the patch applied on a windows build for some time and
see if any similar fixup happens anywhere else.

Comment 8

[Andreas Otte]

Attachment: patch to fix the inconsistency

I think this is an easy fix, no need to future it. However it might be
interesting  to run with the patch applied on a windows build for some time and
see if any similar fixup happens anywhere else.

Comment 9

[Andreas Otte]

Attachment: patch to fix the inconsistency

I think this is an easy fix, no need to future it. However it might be
interesting  to run with the patch applied on a windows build for some time and
see if any similar fixup happens anywhere else.

Comment 10

[Andreas Otte]

Attachment: patch to fix the inconsistency

I think this is an easy fix, no need to future it. However it might be
interesting  to run with the patch applied on a windows build for some time and
see if any similar fixup happens anywhere else.

Comment 11

[Andreas Otte]

Attachment: patch to fix the inconsistency

I think this is an easy fix, no need to future it. However it might be
interesting  to run with the patch applied on a windows build for some time and
see if any similar fixup happens anywhere else.

Comment 12

[Andreas Otte]

Attachment: patch to fix the inconsistency

I think this is an easy fix, no need to future it. However it might be
interesting  to run with the patch applied on a windows build for some time and
see if any similar fixup happens anywhere else.

Comment 13

[Darin Fisher]

andreas: your patch doesn't solve the problem as stated in the summary.  you're
missing the "except when a real filepath is identified" part.

Comment 14

[Adam Lock]

As stated before, back to forward slash conversion is necessary on the PC
because it is such a common mistake and has to be corrected. Deleting this fixup
for an extremely rare edge case is going to cause a lot more problems than it
solves.

A proper patch is going to be more involved, identifying bits of an URL where
backslashes should be let through and other parts where they should be
converted. Darin has mentioned the only place so far where conversion may not
make sense - in the password.

Comment 15

[Andreas Otte]

Ooops, what happend with the patch ... ? bugzilla hickup?

darin: The patch does not have to, because that part is already in place. At
some point during URIFixup the string is taken as filepath and converted to
file-uri. If that is successfull we go for it.

adam: The problem may be to identify the proper parts of the url without making
assumptions about the meaning of \ in that context which is exactly what we
won't want to do ... that's where the dog bites its tail. Also you can add
param, query and ref to parts of the url where a \ might be okay and should not
be converted. At least for query and ref it might be possible to detect them
properly.

Not really cornercases in my opinion. I agree this might be a common error on
windows, I simply have problems with the fact that some correct urls will be
broken by this fixup and the user has no way to stop it.

Comment 16

[Andreas Otte]

Copyed over from bug 123200, another \ case:

>A compromise might be to have another kind of fixup that tries different version
>of an uri based on from where it is called, something like first try with \, if
>that succeeds all is fine, otherwise replace some of the \ with / and try again

Comment 17

[benc]

qa to me.

Comment 18

[Jacek Piskozub]

I believe this bug tries to reverse the change made by fixing bug 32895. 

BTW, bug 93197, similar to this one,  was WONTFIXed.

Comment 19

[Andreas Otte]

No, this one will take bug 32895 even a step further and remove that conversion
alltogether or at least make it a last resort.

Comment 20

[Clarence Risher]

Another example of this bug, as pertains to links instead of typing into the URL
bar, is when a site specifies a relative image path with \s instead of /s. 
http://www.lanwar.com/registration/Hall.asp tries to reference
http://www.lanwar.com/registration/\images\MML2\MML2HallLayout.jpg in Mozilla.

Comment 21

[OstGote!]

As I see this works now? All remote URLs with "\" won't converted to "/" but the
"\" will be correct encoded as "%5C".

try http://piology.org/ie/a\b.html

Comment 22

[Andreas Otte]

No, it is still possible to trigger this on windows. This stuff in
nsDefaultURIFixup will not go away, but we should have a much smarter way to do
the conversion.

Comment 23

[Noam Tamim]

This bug has some unreported (some are "hidden") duplicates with many comments
in them:
bug 190821, bug 176312, bug 221953

Now, it is pretty clear that sites are not supposed to use backslashes in URLs.
However, IE translates them correctly to "/", and mozilla translates them to
%5c, which is the Right Thing but not the Good (or Smart) Thing to do.
There are problematic sites. I, as a technical user, don't really care about
those bugs - I just understand the problem and correct the URL. But other people
- they are forced to use IE, because "firefox can't display this page". I've
seen it!

This have got to be fixed. I guess it won't be before firefox 1.0, but hopefully
sometime in the near future. You want your product to be used by none-geeks, you
have to support them as much as possible.