565432 - [HTML5] <a href="[linebreak]ftp://foo"> link doesn't work with HTML5 parser

Status: RESOLVED FIXED

(Core Graveyard :: Networking: FTP, defect)

Description

[Eugene Savitsky]

1. Open http://www.kaspersky.ee/static,kav,1.htm
2. Look at the right Kaspersky Anti-Virus box. 
3. Below should be links: 
eesti  	|  	inglise  	|  	vene

In Mozilla/5.0 (Windows; U; Windows NT 6.1; WOW64; en-US; rv:1.9.3a5pre) Gecko/20100511 Minefield/3.7a5pre ( .NET CLR 3.5.30729) they do not work, but work in other browsers including FF 3.6.3.

Comment 1

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.3a5pre) Gecko/20100511 Minefield/3.7a5pre

Confirmed; I see the problem with html5.enable to true. While it works fine with html5.enable to false.

Comment 2

[j.j.]

Attachment: testcase

Comment 3

[Henri Sivonen (:hsivonen)]

HTML5 requires the parser not to remove the line break and makes it the responsibility of the higher layers to ignore the line break.

Comment 4

[Henri Sivonen (:hsivonen)]

Attachment: Copy initializing code from http handler into the ftp handler

Comment 5

[Boris Zbarsky [:bzbarsky]]

Why is removing the unescape ok?

Comment 6

[Henri Sivonen (:hsivonen)]

(In reply to comment #5)
> Why is removing the unescape ok?

Removing it makes the code consistent with http: and gopher: handling. I was assuming that it's safe to assume that http: handling is more correct than ftp: handling where they differ.

Comment 7

[Henri Sivonen (:hsivonen)]

Attachment: Copy initializing code from http handler into the ftp handler, v2

It turns out that removing the unescape is not OK. Not having the unescape step is a bug in the http: URL impl...

Comment 8

[Henri Sivonen (:hsivonen)]

(In reply to comment #7)
> Created an attachment (id=445673) [details]
> Copy initializing code from http handler into the ftp handler, v2
> 
> It turns out that removing the unescape is not OK. Not having the unescape step
> is a bug in the http: URL impl...

But it seems the old ftp: URL impl unescaped at the wrong point. The old ftp: impl. exhibited bug 309671 anyway.

I wish we had a proper URL parsing spec already.

Comment 9

[Boris Zbarsky [:bzbarsky]]

> I was assuming that it's safe to assume that http: handling is more correct
> than ftp:

That's not a safe assumption, because they do very different things with the resulting URI (e.g. HTTP sends the path as-is, URI-escaped, and the server is supposed to deal, while for FTP the path means something quite different and the behavior needs to be different).

So it may well be that "correct" means different behavior for HTTP and FTP.

For what it's worth, I just looked up the CVS blame on that unescape call (something worth doing before removing things), and the checkin comment says:

  1.84 <[e-mail]> 2004-07-29 19:45
  Fixing bug 250909. Make ftp: URIs containing %00 invalid. r=[e-mail],
  sr=[e-mail]

Sadly, that bug number is wrong.

Comment 10

[Boris Zbarsky [:bzbarsky]]

And so far I haven't been able to locate the correct bug.  jst, any idea?

Comment 11

[Henri Sivonen (:hsivonen)]

Attachment: Trim ftp URLs before unescaping and looking for forbidden characters.

> For what it's worth, I just looked up the CVS blame on that unescape call
> (something worth doing before removing things), and the checkin comment says:

Yeah. Sorry.

Here's a better patch.

Comment 12

[Johnny Stenback (:jst)]

Blocking on this regression.

Comment 13

[Johnny Stenback (:jst)]

(In reply to comment #9)
> Sadly, that bug number is wrong.

That would be bug 250906.

Comment 14

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 446186 [details] [diff] [review]
Trim ftp URLs before unescaping and looking for forbidden characters.

Yeah, I like this more.

Comment 15

[Henri Sivonen (:hsivonen)]

Thanks. Pushed:
http://hg.mozilla.org/mozilla-central/rev/78268499783e