Closed Bug 565432 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Networking: FTP, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- beta1+

People

(Reporter: ezh, Assigned: hsivonen)

Details

(Keywords: regression, testcase)

Attachments

(2 files, 2 obsolete files)

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.
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.
Component: General → HTML: Parser
QA Contact: general → parser
Version: unspecified → Trunk
Attached file testcase
Keywords: testcase
Summary: Link does not works in Minefield → <a href="[linebreak]ftp://foo"> link does not works with HTML5 parser
Summary: <a href="[linebreak]ftp://foo"> link does not works with HTML5 parser → <a href="[linebreak]ftp://foo"> link doesn't work with HTML5 parser
HTML5 requires the parser not to remove the line break and makes it the responsibility of the higher layers to ignore the line break.
Component: HTML: Parser → DOM: Core & HTML
QA Contact: parser → general
Summary: <a href="[linebreak]ftp://foo"> link doesn't work with HTML5 parser → [HTML5] <a href="[linebreak]ftp://foo"> link doesn't work with HTML5 parser
blocking2.0: --- → ?
OS: Windows 7 → All
Hardware: x86_64 → All
Assignee: nobody → hsivonen
Status: NEW → ASSIGNED
Component: DOM: Core & HTML → Networking: FTP
QA Contact: general → networking.ftp
Why is removing the unescape ok?
(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.
It turns out that removing the unescape is not OK. Not having the unescape step is a bug in the http: URL impl...
Attachment #445351 - Attachment is obsolete: true
(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.
> 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 <jst@mozilla.jstenback.com> 2004-07-29 19:45
  Fixing bug 250909. Make ftp: URIs containing %00 invalid. r=bzbarsky@mit.edu,
  sr=darin@meer.net

Sadly, that bug number is wrong.
And so far I haven't been able to locate the correct bug.  jst, any idea?
> 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.
Attachment #445673 - Attachment is obsolete: true
Attachment #446186 - Flags: review?(bzbarsky)
Blocking on this regression.
blocking2.0: ? → beta1+
Keywords: regression
(In reply to comment #9)
> Sadly, that bug number is wrong.

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

Yeah, I like this more.
Attachment #446186 - Flags: review?(bzbarsky) → review+
Thanks. Pushed:
http://hg.mozilla.org/mozilla-central/rev/78268499783e
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.