193477 - URL: colon ":" in URI is escaped in request (not scheme or port delimiter)

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Scott Gilbert]

User-Agent:       Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.3b) Gecko/20030210
Build Identifier: Mozilla/5.0 (Windows; U; Win 9x 4.90; en-US; rv:1.3b) Gecko/20030210

This function should dispaly the next 20 items and I recieve an errror message.
It does work in Netscape 4.7 and IE but not in Mozilla or Netscape6.0+. The
shopping cart program is Interchange 4.9.6 running on a Linux server.

Reproducible: Always

Steps to Reproduce:
1.Go to http://www.pillagedvillage.com/pvonline/index.html (catalog page)
2.choose category (trim) or category in which more than 20 items exist.
3.click on any page listed below the displayed items or the nextpage or last page.

Actual Results:  
Sorry, no matches for

Errors:
No search was found


Expected Results:  
displayed the next 20 items.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Over to networking.  We escape the ':' in the URI, whereas netscape 4 does not.
 Not being able to deal with that is a server bug, of course.  But should we try
not to do it?

Comment 2

[Doug Turner (:dougt)]

andreas, can you comment?

Comment 3

[Andreas Otte]

I think we can remove that colon-escape in the file-part of urls, the reason we
had this no longer applies, but I am not sure. Needs careful testing but we
should try.

Comment 4

[Andreas Otte]

Attachment: Patch to remove the colon from escaping in file-part of url

Comment 5

[Darin Fisher]

Comment on attachment 114893 [details] [diff] [review]
Patch to remove the colon from escaping in file-part of url

yeah, this should be fine.  sr=darin

Comment 6

[Andreas Otte]

Stop, not so fast. First I want to see a lot of people running with this patch
applied for some weeks and having no regressions (for example in directory
listings from http, ftp and file, clicking on filenames or directorys containing
the colon).

Comment 7

[Darin Fisher]

andreas: how does the 1.4 alpha trunk sound to you?  i'm sure i could run with
this patch forever and never encounter a site that has trouble with it.

Comment 8

[Andreas Otte]

Sounds good, I also don't see normal pages as problem, I see potential problems
with those directory listings of different protocols when those filenames
contain a colon.   

Comment 9

[benc]

updated URL w/ sample badness.

I'm surprised this hasn't been a problem before? Is this a related to recent
changes in URL pasing?

Was this escaping affecting ftp: URLs as well?

Comment 10

[Andreas Otte]

This is in since forever (M9?) and it is no problem. We can escape a colon in a
filename whenever we want. It is a failure of the server not to unescape
properly. However we can try to help, but I just did a little bit of testing:

file protocol using HTML directory:
klicking on directory containing colon give unknown protocol error

file protocol using XUL directory:
klicking on directory containing colon gives assertions but seem to work

Comment 11

[Andreas Otte]

The assertions also happen if the patch is not in. That leaves the problem with
the HTML directory when the patch is in. The reason is clear. We handle the
filename when clicking on it as relative url, but the unescaped colon marks a
scheme => most of the time an absolute url => unknown protocol. That's the
reason  we escaped the colon in a filename in the first place. 

So the problem is still there. If we remove the colon escape from the filename
component of the url we need another mask (directory-entry) that still does and
use it in the directory representations.

Comment 12

[Andreas Otte]

Comment on attachment 114893 [details] [diff] [review]
Patch to remove the colon from escaping in file-part of url

removing review request

Comment 13

[Darin Fisher]

Comment on attachment 114893 [details] [diff] [review]
Patch to remove the colon from escaping in file-part of url

andreas: thanks for doing the leg work to test this out.  care to own this bug?
;-)

Comment 14

[Andreas Otte]

Taking ...

Comment 15

[Andreas Otte]

Attachment: better patch that takes care of the directory listings

This patch introduces another mask esc_Colon which forces the escape of the
colon even when not in the matrix for that part of the url.

Comment 16

[Andreas Otte]

Attachment: Throwing in an additional esc_Forced

Throwing in an additional esc_Forced to deal with filenames like
abc%20def.html. This will fix the file-part on bug 191638

Comment 17

[Boris Zbarsky [:bzbarsky]]

I won't be able to get to this review till 2-3 weeks from now, most likely.

Comment 18

[Andreas Otte]

Attachment: Adding a new mask to unblock the fix for bug 191638

Added a new mask to allow minimal escaping, this will be used by the fix for
bug 191638.

Comment 19

[Andreas Otte]

Comment on attachment 115043 [details] [diff] [review]
Throwing in an additional esc_Forced

removing Requests, thanks Boris for the info

Comment 20

[Darin Fisher]

Comment on attachment 115228 [details] [diff] [review]
Adding a new mask to unblock the fix for bug 191638

sr=darin

Comment 21

[Andreas Otte]

Attachment: patch without the additional changes for bug 191638 which already got checked in

Comment 22

[Andreas Otte]

fix checked in