249282 - Permit mixing of \ and / in file:// URLs under Win32

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Darin Fisher]

We should fixup any \'s in file:// URLs under Win32 builds since we know that
path segments cannot contain \'s under Win32.  This will help workaround a
problem with DB2 UDB 7.1 (see bug 13607) and will likely help with other Win32
specific apps that utilize HTML documentation, etc.

It's unfortunate to have to do this, but I think it is safe to do.

mkaply: I can do this on OS/2 as well if you think it should be done there.  The
changes will be confined to nsURLHelperWin.cpp and nsURLHelperOS2.cpp is mostly
a copy of nsURLHelperWin.cpp.

Comment 1

[Darin Fisher]

comments from andreas otte via email:

> Fine with me, but if we do it, that code (also looking for %5C) should
> be placed into the functions that convert file-urls into paths. They
> have no place in the url-parser.

Comment 2

[Christian :Biesinger (don't email me, ping me on IRC)]

*** Bug 252538 has been marked as a duplicate of this bug. ***

Comment 3

[Darin Fisher]

Attachment: v1 patch

Comment 4

[Darin Fisher]

Comment on attachment 154704 [details] [diff] [review]
v1 patch

I noticed quite a lot of rough edges in the handling of non-ASCII file:// URLs.
 I doubt they work very well... I plan on filing some bugs on various issues
that I spotted.

Comment 5

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 154704 [details] [diff] [review]
v1 patch

base/src/nsURLHelper.cpp

+    for (const char *s = begin; *s; ++s)

that assumes the string is null-terminated. couldn't this be a substring?

base/src/nsURLHelper.h

you should mention that aResultBuf will be unmodified if the url was already
normalized and that the string should be empty initially.

Comment 6

[Darin Fisher]

> +    for (const char *s = begin; *s; ++s)
> 
> that assumes the string is null-terminated. couldn't this be a substring?

yup.. good catch!!


> you should mention that aResultBuf will be unmodified if the url was already
> normalized and that the string should be empty initially.

will do, thanks!

Comment 7

[Darin Fisher]

Attachment: v1.1 patch

revised with correction to end-of-string determination.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 155148 [details] [diff] [review]
v1.1 patch

>Index: base/src/nsURLHelper.h

>+ * @returns false if aURL is already normalized.  Otherwise, returns true.

Make it clear that when aURL is already normalized aResultBuf will be left
empty (or that its value shouldn't be relied on, or whatever)...

sr=bzbarsky with that.

Comment 9

[Darin Fisher]

fixed-on-trunk

Comment 10

[David G King]

I'm getting the following error with my Win32/MinGW/cygwin build of Mozilla.
Reopening bug.

e:/mozilla/source/mozilla/netwerk/base/src/nsURLHelper.cpp: In function `PRBool
net_NormalizeFileURL(const nsACString&, nsCString&)':
e:/mozilla/source/mozilla/netwerk/base/src/nsURLHelper.cpp:602: error: name look
up of `s' changed for new ISO `for' scoping
e:/mozilla/source/mozilla/netwerk/base/src/nsURLHelper.cpp:591: error:   using o
bsolete binding at `s'
make[5]: *** [nsURLHelper.o] Error 1
make[5]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/netwerk/base/src'
make[4]: *** [libs] Error 2
make[4]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/netwerk/base'
make[3]: *** [libs] Error 2
make[3]: Leaving directory `/cygdrive/e/mozilla/source/mozilla/netwerk'
make[2]: *** [libs] Error 2
make[2]: Leaving directory `/cygdrive/e/mozilla/source/mozilla'
make[1]: *** [alldep] Error 2
make[1]: Leaving directory `/cygdrive/e/mozilla/source/mozilla'
make: *** [alldep] Error 2

Comment 11

[Darin Fisher]

fix checked in for MingW bustage.

Comment 12

[David G King]

Thanks, that fixed by build problems. Resolving.

Comment 13

[Mike Kaply [:mkaply]]

Comment on attachment 155148 [details] [diff] [review]
v1.1 patch

I want this in 1.7.

And it really should have gone into Firefox 1.0. 

Once the FF branch opens for 1.0X stuff, we really should get this in. Ugly bug

Comment 14

[Mike Kaply [:mkaply]]

Comment on attachment 155148 [details] [diff] [review]
v1.1 patch

There is no post 1.0 aviary and I don't think this would be needed for
Thunderbird - darin?

Comment 15

[Mike Kaply [:mkaply]]

Comment on attachment 155148 [details] [diff] [review]
v1.1 patch

There is no post 1.0 aviary and I don't think this would be needed for
Thunderbird - darin?

Comment 16

[Darin Fisher]

yeah, i doubt thunderbird has any need for this patch.