147914 - Hangs on Links with "|" (pipe) in URL

Status: VERIFIED FIXED

(Camino Graveyard :: Page Layout, defect)

Description

[Chris Petersen]

None of the Discussion links on this Apple "Welcome to Discussions" page will
load.  Hangs at Loading...  Attempts to use the  Tab again for anything are not
successful.

Comment 1

[Chris Petersen]

Verified in the 0.2.8 build.
Clicking on links at http://discussions.info.apple.com/ will not load. 

Comment 2

[saari (gone)]

->pinkerton

Comment 3

[Mike Pinkerton (not reading bugmail)]

wow, whatever this site does, it hoses the entire app.

darin? any ideas what this site is doing that might cause funky networking hiccups?

Comment 4

[Darin Fisher]

WORKSFORME mozilla 1.0 rc3 macosx.

Comment 5

[Mike Pinkerton (not reading bugmail)]

darin, of course it works in mozilla. this bug is for chimera ;)

Comment 6

[Darin Fisher]

arg.. i'm not at all accustomed to looking at the product selection.. will take
another look using chimera or perhaps some other mach-o build.

Comment 7

[Mike Pinkerton (not reading bugmail)]

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

Comment 8

[Winnie Lam]

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

Comment 9

[Winnie Lam]

http://mozdev.org/bugs/show_bug.cgi?id=1160 - Another dupe of this bug.

Comment 10

[Winnie Lam]

Yet another dupe from mozdev - http://mozdev.org/bugs/show_bug.cgi?id=1247

Comment 11

[James]

The character that hangs Chimera is the | character. For an example check here:
http://www.them.ws/chimbugs/

The bug renders all of Chimera useless because it will not respond to any urls
any more, requires a restart of the browser.

Comment 12

[Mike Pinkerton (not reading bugmail)]

ew, wow, perhaps darin knows why. over to bryner to go sit on darin until he
helps us ;) ;)

Comment 13

[Darin Fisher]

WORKSFORME with a debug chimera build pulled on June 15.

Comment 14

[Jørgen Nørgaard]

The nightly from today (20/6) still exhibits the bug.

Try http://anneli.dk/~jnp/test/HTML/std.html, most of the link will render
Chimera useless.  Also mentioned in one of the duplicates.

Not resolved (but I cannot change that!)

Comment 15

[Darin Fisher]

reopening per previous comment.

Comment 16

[Brian Ryner (not reading)]

Ok, it looks like the real problem here is that Cocoa's NSURL class does not
think these URL's are valid.  It probably wants the characters escaped.  The
Apple developer documentation says that the following characters must be escaped
for NSURL:

: / % # ; @

From the discussion here it sounds like '|' may need to be escaped as well.

There are actually a few ways that this can manifest itself.  If you enter one
of these "invalid" url's in the location bar, we fail to create a NSURL, and
pass a null URL string to WebBrowser, and nothing happens.  In the case where
you click a link to an "invalid" url, the problem happens when we get the
location change notification.  We fail to create the NSURL, so when we try to
get the URL spec to put in the location bar, it raises an exception.  We have no
exception handling so we bail out and don't finish processing the current
PLEvent, which causes all sorts of problems.

So, there are a couple of ways I can see to fix this:

- Create a more correct method for converting between nsIURI and NSURL
- Eliminate use of NSURL internally in Chimera (in favor of simple NSStrings)

Pink, Darin, any thoughts on this?  Are there places that we need to use NSURL
to integrate with the OS?

Comment 17

[Jørgen Nørgaard]

Not having seen the documentation, it seems like more characters have to be
escaped: {} as per the above link.

Others, [] and similar used for constructing parameter list in a lot of servers
needs to be tested also.

Comment 18

[Darin Fisher]

beware escaping []... those are used to "escape" IPv6 address literals, e.g.:

  http://[66.218.71.112::]:8080/index.html

the []'s are used to distinguish the port number from the address literal.

Comment 19

[Brian Ryner (not reading)]

Attachment: patch

Ok, this patch removes uses of NSURL except where it's required for other Cocoa
API's (and none of these cases should trigger the escaped characters problem,
since the URL's come from the system).

Comment 20

[Greg K.]

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

Comment 21

[Brian Ryner (not reading)]

Looks like pinkerton's patch for external protocol handling made this more
complicated.  new patch coming up.

Comment 22

[Brian Ryner (not reading)]

Attachment: patch #2

So, pinkerton's patch actually adds a scenario where we need to go from what
was an nsIURI to an NSURL.  In the case where there's an external protocol
specified, and the URL contains one of these characters that NSURL expects to
be escaped, it will fail to create the NSURL.  The real solution for this would
seem to be to figure out exactly what conversion is necessary to go from
nsIURI's escaping conventions to what nsURL expects (this is not trivial). 
I've chosen to ignore that problem in this patch and hope that this situation
won't come up (although it probably will).

Comment 23

[Brian Ryner (not reading)]

Attachment: patch #3

I think this will address the situation I pointed out above.

Comment 24

[Mike Pinkerton (not reading bugmail)]

Comment on attachment 88757 [details] [diff] [review]
patch #3

looks great to me
r=pink. land this puppy.

Comment 25

[Mike Pinkerton (not reading bugmail)]

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

Comment 26

[Brian Ryner (not reading)]

Checked in.

Comment 27

[Brian Ryner (not reading)]

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

Comment 28

[Winnie Lam]

Verified with 06-26 build on OS 10.1.5.  Went to Apple Discussion URL, clicked
through a few categories back and forth.  Did not hang.